Closed
Bug 876856
Opened 11 years ago
Closed 11 years ago
Crash [@ mozilla::dom::MediaStreamList::Length] after closing a peer connection
Categories
(Core :: WebRTC, defect)
Core
WebRTC
Tracking
()
VERIFIED
FIXED
mozilla24
People
(Reporter: jruderman, Assigned: jesup)
Details
(Keywords: crash, testcase, verifyme, Whiteboard: [WebRTC][blocking-webrtc-])
Crash Data
Attachments
(2 files, 1 obsolete file)
353 bytes,
text/html
|
Details | |
1.46 KB,
patch
|
ekr
:
review+
akeybl
:
approval-mozilla-aurora+
akeybl
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
With: user_pref("media.peerconnection.enabled", true);
Comment 1•11 years ago
|
||
Confirmed on my Win 7 machine.
Comment 2•11 years ago
|
||
https://crash-stats.mozilla.com/report/index/bp-9695cf61-5322-455d-876d-8281b2130528
Crash Signature: [@ mozilla::dom::MediaStreamList::Length()]
Comment 3•11 years ago
|
||
Frame Module Signature Source 0 xul.dll mozilla::dom::MediaStreamList::Length media/webrtc/signaling/src/peerconnection/MediaStreamList.cpp:78 1 xul.dll mozilla::dom::MediaStreamListBinding::get_length obj-firefox/dom/bindings/MediaStreamListBinding.cpp:26 2 xul.dll mozilla::dom::MediaStreamListBinding::genericGetter obj-firefox/dom/bindings/MediaStreamListBinding.cpp:62 3 mozjs.dll js::Invoke js/src/jsinterp.cpp:441 4 mozjs.dll JS_ForwardGetPropertyTo js/src/jsapi.cpp:4277 5 xul.dll mozilla::dom::GetPropertyOnPrototype dom/bindings/BindingUtils.cpp:1261 6 xul.dll mozilla::dom::MediaStreamListBinding::DOMProxyHandler::get obj-firefox/dom/bindings/MediaStreamListBinding.cpp:367 7 mozjs.dll proxy_GetGeneric js/src/jsproxy.cpp:2811 8 mozjs.dll js::GetPropertyOperation js/src/jsinterpinlines.h:290 9 mozjs.dll js::Interpret js/src/jsinterp.cpp:2067 10 mozjs.dll js::ion::CanEnter js/src/ion/Ion.cpp:1695 11 mozjs.dll js::RunScript js/src/jsinterp.cpp:332 12 mozjs.dll js::Invoke js/src/jsinterp.cpp:441 13 mozjs.dll JS_CallFunctionValue js/src/jsapi.cpp:5871 14 xul.dll mozilla::dom::Function::Call obj-firefox/dom/bindings/FunctionBinding.cpp:41 15 xul.dll mozilla::dom::Function::Call<nsCOMPtr<nsISupports> > obj-firefox/dist/include/mozilla/dom/FunctionBinding.h:52 16 xul.dll nsGlobalWindow::RunTimeoutHandler dom/base/nsGlobalWindow.cpp:10176 17 xul.dll nsGlobalWindow::RunTimeout dom/base/nsGlobalWindow.cpp:10414 18 xul.dll nsGlobalWindow::TimerCallback dom/base/nsGlobalWindow.cpp:10661 19 xul.dll nsTimerImpl::Fire xpcom/threads/nsTimerImpl.cpp:547 20 xul.dll nsTimerEvent::Run xpcom/threads/nsTimerImpl.cpp:634 21 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:627 22 xul.dll NS_ProcessNextEvent obj-firefox/xpcom/build/nsThreadUtils.cpp:238 23 xul.dll mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:82 24 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:212 25 xul.dll MessageLoop::Run ipc/chromium/src/base/message_loop.cc:186 26 xul.dll nsBaseAppShell::Run widget/xpwidgets/nsBaseAppShell.cpp:163 27 xul.dll nsAppShell::Run widget/windows/nsAppShell.cpp:113 28 xul.dll nsAppStartup::Run toolkit/components/startup/nsAppStartup.cpp:269 29 xul.dll XREMain::XRE_mainRun toolkit/xre/nsAppRunner.cpp:3872 30 xul.dll XREMain::XRE_main toolkit/xre/nsAppRunner.cpp:3939 31 xul.dll XRE_main toolkit/xre/nsAppRunner.cpp:4151 32 firefox.exe do_main browser/app/nsBrowserApp.cpp:272 33 firefox.exe NS_internal_main browser/app/nsBrowserApp.cpp:632 34 firefox.exe wmain toolkit/xre/nsWindowsWMain.cpp:105 35 firefox.exe __tmainCRTStartup crtexe.c:552 36 kernel32.dll BaseThreadInitThunk 37 ntdll.dll __RtlUserThreadStart 38 ntdll.dll _RtlUserThreadStart
Comment 4•11 years ago
|
||
http://hg.mozilla.org/mozilla-central/annotate/6a739f41d1ba/media/webrtc/signaling/src/peerconnection/MediaStreamList.cpp#l78
Updated•11 years ago
|
OS: Mac OS X → All
Hardware: x86_64 → All
Assignee | ||
Comment 5•11 years ago
|
||
The MediaStreamList code isn't checking to see if the PeerConnection has been closed (which causes the MediaStream lists and rest of the PCMedia object to be released). Blocking- as this is a safe non-deref failure in an edge case. We'd likely uplift this fix to Aurora; the fix should be safe enough we could consider Beta uplift
Whiteboard: [WebRTC][blocking-webrtc-][webrtc-uplift]
Assignee | ||
Comment 6•11 years ago
|
||
This fix appears to work and throws warnings as expected (probably we should remove the warnings before landing)
Assignee | ||
Comment 7•11 years ago
|
||
Assignee | ||
Updated•11 years ago
|
Attachment #755025 -
Attachment is obsolete: true
Assignee | ||
Updated•11 years ago
|
Attachment #755037 -
Flags: review?(ekr)
Comment 8•11 years ago
|
||
Comment on attachment 755037 [details] [diff] [review] check if PeerConnection is closed before accessing MediaStreamList Review of attachment 755037 [details] [diff] [review]: ----------------------------------------------------------------- This looks OK, but I tend to agree you should get rid of the NS_WARNINGs.
Attachment #755037 -
Flags: review?(ekr) → review+
Assignee | ||
Comment 9•11 years ago
|
||
https://hg.mozilla.org/projects/cypress/rev/324169b939ae With NS_WARNINGS removed Fix is very safe; we should consider Beta uplift
status-firefox22:
--- → affected
status-firefox23:
--- → affected
status-firefox24:
--- → affected
Target Milestone: --- → mozilla24
Comment 10•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/324169b939ae
Assignee: nobody → rjesup
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 11•11 years ago
|
||
Comment on attachment 755037 [details] [diff] [review] check if PeerConnection is closed before accessing MediaStreamList [Approval Request Comment] Bug caused by (feature/regressing bug #): N/A User impact if declined: Race conditions that can cause crashes (null derefs) Testing completed (on m-c, etc.): on m-c. Risk to taking this patch (and alternatives if risky): None. Adds error checking and safe error returns. String or IDL/UUID changes made by this patch: none
Attachment #755037 -
Flags: approval-mozilla-beta?
Attachment #755037 -
Flags: approval-mozilla-aurora?
Updated•11 years ago
|
Comment 12•11 years ago
|
||
lgtm on 6/1 trunk build - test case isn't crashing after about 20 runs.
Updated•11 years ago
|
Attachment #755037 -
Flags: approval-mozilla-beta?
Attachment #755037 -
Flags: approval-mozilla-beta+
Attachment #755037 -
Flags: approval-mozilla-aurora?
Attachment #755037 -
Flags: approval-mozilla-aurora+
Comment 13•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/7c64168fea7b https://hg.mozilla.org/releases/mozilla-beta/rev/10caf14d9013
Updated•11 years ago
|
Whiteboard: [WebRTC][blocking-webrtc-][webrtc-uplift] → [WebRTC][blocking-webrtc-]
Comment 14•11 years ago
|
||
Verified as fixed on Firefox 22 beta 4: Mozilla/5.0 (Windows NT 6.1; rv:22.0) Gecko/20100101 Firefox/22.0 Mozilla/5.0 (X11; Linux i686; rv:22.0) Gecko/20100101 Firefox/22.0 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:22.0) Gecko/20100101 Firefox/22.0 Build ID: 20130605070403
Updated•11 years ago
|
Updated•11 years ago
|
QA Contact: jsmith
Comment 15•11 years ago
|
||
Verified fixed with Firefox 23 beta 1 (build ID: 20130625125232) on : Ubuntu 12.10 32bit, Mac OSX 10.8.3 and Win 7 64bit.
You need to log in
before you can comment on or make changes to this bug.
Description
•