805112 - Crash [@ srtp_protect_rtcp ]

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, defect, P1)

Description

[Florian Quèze [:florian]]

I unfortunately don't have steps to reproduce this crash (https://crash-stats.mozilla.com/report/index/bp-fa2b2f7d-1fb4-4797-853f-301632121024)

It happened while I was playing with/debugging https://github.com/anantn/socialapi-demo

I think I restarted the browser while I had an ongoing video stream displayed in a social API chat window.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

0 	XUL 	srtp_protect_rtcp 	srtp.c:1724
1 	XUL 	mozilla::SrtpFlow::ProtectRtcp 	SrtpFlow.cpp:180
2 	XUL 	mozilla::MediaPipeline::PipelineTransport::SendRtcpPacket 	MediaPipeline.cpp:478
3 	XUL 	non-virtual thunk to mozilla::WebrtcVideoConduit::SendRTCPPacket 	
4 	XUL 	webrtc::ViESender::SendRTCPPacket 	vie_sender.cc:187
5 	XUL 	webrtc::RTCPSender::SendRTCP 	rtcp_sender.cc:1947
6 	XUL 	webrtc::ModuleRtpRtcpImpl::SetSendingStatus 	rtp_rtcp_impl.cc:738
7 	XUL 	webrtc::ViEChannel::StopSend 	vie_channel.cc:1430
8 	XUL 	webrtc::ViEBaseImpl::StopSend 	vie_base_impl.cc:299
9 	XUL 	mozilla::WebrtcVideoConduit::~WebrtcVideoConduit 	VideoConduit.cpp:73
10 	XUL 	mozilla::WebrtcVideoConduit::~WebrtcVideoConduit 	VideoConduit.cpp:32
11 	XUL 	mozilla::MediaPipeline::~MediaPipeline 	MediaConduitInterface.h:142
12 	XUL 	mozilla::MediaPipelineTransmit::~MediaPipelineTransmit 	MediaPipeline.h:185
13 	XUL 	std::_Rb_tree<int, std::pair<int const, mozilla::RefPtr<mozilla::MediaPipeline> 	MediaPipeline.h:88
14 	XUL 	sipcc::LocalSourceStreamInfo::~LocalSourceStreamInfo 	PeerConnectionImpl.h:187
15 	XUL 	mozilla::MediaStream::RemoveListener 	
16 	XUL 	nsTArray<nsAutoPtr<mozilla:: 	
17 	XUL 	mozilla::MediaStreamGraphImpl::RunThread 	MediaStreamGraph.cpp:409
18 	XUL 	mozilla:: 	MediaStreamGraph.cpp:1524
19 	XUL 	nsThread::ProcessNextEvent 	nsThread.cpp:612
20 	XUL 	NS_ProcessNextEvent_P 	nsThreadUtils.cpp:220
21 	XUL 	nsThread::ThreadFunc 	nsThread.cpp:256
22 	libnspr4.dylib 	_pt_root 	ptthread.c:156
23 	libsystem_c.dylib 	libsystem_c.dylib@0x14741 	
24 	libsystem_c.dylib 	libsystem_c.dylib@0x1180 	
25 	libnspr4.dylib 	libnspr4.dylib@0x1db3f

Comment 2

[Jason Smith [:jsmith]]

Not enough information for a crashtest yet. If a reproducible test case is found, then renom.

Comment 3

[David Bolter [:davidb] (NeedInfo me for attention)]

Please change the sec-moderate rating if appropriate - ideally along with STR.

Comment 4

[Florian Quèze [:florian]]

Just saw this again today, the stack hasn't changed since comment 0 (https://crash-stats.mozilla.com/report/index/bp-cfca05ab-39c4-4f1f-b15e-0d5332121204)

Comment 5

[Randell Jesup [:jesup] (needinfo me)]

Null deref, though not yet proven it can only be a null deref.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

While working on the first PC video mochitest I was able to see this crash constantly. I will try to minimize the test so we have a crashtest for it.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: testcase

This is the mochitest which let Firefox crash on shutdown with the specified signature. Not sure why it happens but it only occurs when I put the arguments to timeout() in a wrong order.

Steps:
1. Apply patch
2. Execute "./mach mochitest-plain dom/media/tests/mochitest/"
3. Quit Firefox by pressing Cmd+Q

So far only tested on OS X. I will try to get a reduced testcase, hopefully even a crashtest.

Comment 8

[Randell Jesup [:jesup] (needinfo me)]

Tried multiple times on Linux with a patch for bug 818680 (protect FakeMedia listeners) and bug 816640 (make nrappkit timers threadsafe).

No problems.  Could be linux vs mac or my patches (esp the fakeMedia)

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Randell, does it mean you can't even see the crash on a clean m-c checkout?

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

Correct.  Once it appeared to deadlock trying to exit; no crashes

Comment 11

[Randell Jesup [:jesup] (needinfo me)]

Fiddled the patches against current trunk to replace the guts of test_peerConnection_basicVideo.html to be "window.setTimeout(5000,disconnect);" (note: setTimeout, not "window.timeout()" from the patch - and with the arguments in the wrong order, per the comment.

I just get a JS warning about a useless setTimeout() call, and no problems shutting down.

Given the patches to fix leaving rtcp running have landed (and this was a crash in rtcp during shutdown), I believe strongly this is fixed.  Closing unless/until we get a similar stack during shutdown.

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to Randell Jesup [:jesup] from comment #11)
> "window.setTimeout(5000,disconnect);" (note: setTimeout, not
> "window.timeout()" from the patch - and with the arguments in the wrong
> order, per the comment.

That was an accident when I was playing with the test but exactly that caused the problem. When I changed that to setTimeout and the right order of parameters I haven't seen the problem. So all that is on purpose.