833217 - crash in mozilla::DataChannelOnMessageAvailable::Run

Status: RESOLVED FIXED

(Core :: WebRTC: Signaling, defect, P1)

Description

[Scoobidiver (away)]

It first showed up in 21.0a1/20130121020044.

Signature 	mozilla::DataChannelOnMessageAvailable::Run() More Reports Search
UUID	568f8a7e-b40f-443f-857e-1a2c02130121
Date Processed	2013-01-21 21:15:20
Uptime	169
Last Crash	3.1 minutes before submission
Install Age	2.8 minutes since version was first installed.
Install Time	2013-01-21 21:12:19
Product	Firefox
Version	21.0a1
Build ID	20130121031005
Release Channel	nightly
OS	Windows NT
OS Version	6.1.7601 Service Pack 1
Build Architecture	x86
Build Architecture Info	GenuineIntel family 6 model 23 stepping 10
Crash Reason	EXCEPTION_ACCESS_VIOLATION_READ
Crash Address	0x6d74e010
App Notes 	
AdapterVendorID: 0x10de, AdapterDeviceID: 0x0652, AdapterSubsysID: 02011025, AdapterDriverVersion: 9.18.13.1064
D2D? D2D+ DWrite? DWrite+ D3D10 Layers? D3D10 Layers+ 
EMCheckCompatibility	True
Adapter Vendor ID	0x10de
Adapter Device ID	0x0652
Total Virtual Memory	4294836224
Available Virtual Memory	3320619008
System Memory Use Percentage	69
Available Page File	3657469952
Available Physical Memory	1291894784

Frame 	Module 	Signature 	Source
0 	xul.dll 	mozilla::DataChannelOnMessageAvailable::Run 	netwerk/sctp/datachannel/DataChannel.h:481
1 	xul.dll 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:627
2 	xul.dll 	NS_ProcessNextEvent_P 	obj-firefox/xpcom/build/nsThreadUtils.cpp:238
3 	xul.dll 	nsThread::Dispatch 	xpcom/threads/nsThread.cpp:410
4 	xul.dll 	nsSocketTransportService::Dispatch 	netwerk/base/src/nsSocketTransportService2.cpp:116
5 	xul.dll 	mozilla::RUN_ON_THREAD 	media/mtransport/runnable_utils.h:51
6 	xul.dll 	mozilla::MediaPipeline::DetachTransport 	media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp:112
7 	xul.dll 	mozilla::MediaPipeline::Shutdown 	media/webrtc/signaling/src/mediapipeline/MediaPipeline.h:105
8 	xul.dll 	sipcc::RemoteSourceStreamInfo::Detach 	media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h:217
9 	xul.dll 	sipcc::PeerConnectionMedia::DisconnectMediaStreams 	media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp:238
10 	xul.dll 	sipcc::PeerConnectionMedia::SelfDestruct 	media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp:217
11 	xul.dll 	mozilla::runnable_args_m_0<mozilla::MediaPipeline*,void 	media/mtransport/runnable_utils_generated.h:48
12 	xul.dll 	mozilla::RUN_ON_THREAD 	media/mtransport/runnable_utils.h:54
13 	xul.dll 	sipcc::PeerConnectionImpl::ShutdownMedia 	media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp:993
14 	xul.dll 	sipcc::PeerConnectionImpl::CloseInt 	media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp:966
15 	xul.dll 	sipcc::PeerConnectionImpl::~PeerConnectionImpl 	media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp:239
16 	xul.dll 	sipcc::PeerConnectionImpl::`scalar deleting destructor' 	
17 	xul.dll 	sipcc::PeerConnectionImpl::Release 	media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp:218
18 	xul.dll 	DoDeferredRelease<nsISupports*> 	js/xpconnect/src/XPCJSRuntime.cpp:541
19 	xul.dll 	XPCJSRuntime::GCCallback 	js/xpconnect/src/XPCJSRuntime.cpp:741
20 	mozjs.dll 	Collect 	js/src/jsgc.cpp:4380
21 	mozjs.dll 	js::GC 	js/src/jsgc.cpp:4398
22 	mozjs.dll 	js::AutoDebugModeGC::~AutoDebugModeGC 	js/src/jscompartment.h:561
23 	mozjs.dll 	JS_SetDebugModeForAllCompartments 	js/src/jsdbgapi.cpp:138
24 	xul.dll 	nsXPConnect::CheckForDebugMode 	js/xpconnect/src/nsXPConnect.cpp:2145
25 	xul.dll 	nsXPConnect::Push 	js/xpconnect/src/nsXPConnect.cpp:2216
...

More reports at:
https://crash-stats.mozilla.com/report/list?signature=mozilla%3A%3ADataChannelOnMessageAvailable%3A%3ARun%28%29

Comment 1

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

Attachment: Null out listener if we're no longer listening for DataChannel events

WeakPtrs just don't work for DataChannel listeners - the DOM object being released off mainthread borks us.  We can use them for the DataChannelConnection listener, though.  Need threadsafe WeakPtrs...

Comment 2

[Adam Roach [:abr]]

Comment on attachment 712996 [details] [diff] [review]
Null out listener if we're no longer listening for DataChannel events

Review of attachment 712996 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good to me.

::: media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
@@ +249,5 @@
>  PeerConnectionImpl::~PeerConnectionImpl()
>  {
> +#ifdef MOZILLA_INTERNAL_API
> +  MOZ_ASSERT(NS_IsMainThread());
> +#endif

This MOZ_ASSERT is redundant with the (admittedly confusingly-named) PC_AUTO_ENTER_API_CALL_NO_CHECK() below. I would suggest omitting this additional check, perhaps replacing it with a comment to the effect that the following macro enforces running on the main thread.

::: netwerk/sctp/datachannel/DataChannel.h
@@ +487,4 @@
>        case ON_CHANNEL_CREATED:
>        case ON_CONNECTION:
>        case ON_DISCONNECTED:
> +        // WeakPtr

Perhaps a more verbose comment here is warranted -- it would be helpful for future maintainers to indicate something like "mConnection is a weakptr. Since mListener can only be deleted from the main thread, and we always run in the main thread, we don't need to use the same locking scheme as we do for mChannel, above."

Comment 3

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/48824bf18390

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/48824bf18390

Comment 5

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

An URL we might be able to use for a crashtest after verification it really crashes in the same signature is http://sharefest.peer5.com/JGYtt.

Comment 6

[Jason Smith [:jsmith]]

In this bug's current state, it's going to be tough to get a crash test here unless we derive a reduced test case using knowledge of the patch. testsuite- until we get more info.