Closed
Bug 820978
Opened 11 years ago
Closed 11 years ago
Crash when closing Firefox while gUM() call is active [@ PR_Unlock][@ mozilla::MediaEngineWebRTC::EnumerateAudioDevices]
Categories
(Core :: WebRTC, defect, P1)
Tracking
()
VERIFIED
FIXED
mozilla20
Tracking | Status | |
---|---|---|
firefox19 | --- | disabled |
firefox20 | --- | verified |
firefox-esr17 | --- | unaffected |
b2g18 | --- | disabled |
People
(Reporter: whimboo, Assigned: jesup)
Details
(Keywords: crash, reproducible, testcase, Whiteboard: [getUserMedia][blocking-gum+][adv-main20-])
Crash Data
Attachments
(1 file, 1 obsolete file)
10.18 KB,
patch
|
derf
:
review+
|
Details | Diff | Splinter Review |
As I have mentioned on bug 818801 it takes a long time until the gUM notification comes up. When I quit Firefox before that time it crashes. Steps: 1. Open attachment 689100 [details] 2. Click start 3. Quit Firefox immediately Report: bp-b8fa9756-26b7-4af8-930a-18b402121212 Stack: 0 libnspr4.dylib PR_Unlock ptsynch.c:199 1 XUL mozilla::MediaEngineWebRTC::EnumerateAudioDevices Mutex.h:83 2 XUL mozilla::GetUserMediaDevicesRunnable::Run MediaManager.cpp:650 3 XUL nsThread::ProcessNextEvent nsThread.cpp:627 4 XUL NS_ProcessNextEvent_P nsThreadUtils.cpp:221 5 XUL nsThread::ThreadFunc nsThread.cpp:265 6 libnspr4.dylib _pt_root ptthread.c:156 7 libsystem_c.dylib libsystem_c.dylib@0x4e8be 8 libsystem_c.dylib libsystem_c.dylib@0x51b74 9 libnspr4.dylib libnspr4.dylib@0x1dacf
Updated•11 years ago
|
Whiteboard: [WebRTC] → [getUserMedia]
Updated•11 years ago
|
Crash Signature: [@ PR_Unlock]
[@ mozilla::MediaEngineWebRTC::EnumerateAudioDevices] → [@ PR_Unlock | mozilla::MediaEngineWebRTC::EnumerateAudioDevices(nsTArray<nsRefPtr<mozilla::MediaEngineAudioSource>, nsTArrayDefaultAllocator>*)]
Reporter | ||
Comment 1•11 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #0) Updated Steps: 1. Open attachment 689100 [details] 2. Click the start button twice (right after each other) 3. Quit Firefox immediately With those I can reproduce the crash all the time.
Keywords: reproducible,
testcase
Reporter | ||
Comment 2•11 years ago
|
||
With a debug build I get the following stack. Scoobidiver, what would be the correct signature here? Program received signal SIGABRT, Aborted. 0x00007fff8c792ce2 in __pthread_kill () (gdb) bt #0 0x00007fff8c792ce2 in __pthread_kill () #1 0x00007fff894a57d2 in pthread_kill () #2 0x00007fff89496a7a in abort () #3 0x000000010008261b in PR_Assert (s=<value temporarily unavailable, due to optimizations>, file=<value temporarily unavailable, due to optimizations>, ln=<value temporarily unavailable, due to optimizations>) at /Volumes/data/code/firefox/nightly/nsprpub/pr/src/io/prlog.c:554 #4 0x000000010009560b in __inline_memset_chk [inlined] () at /usr/include/secure/_string.h:162 #5 0x000000010009560b in PR_DestroyLock (lock=0x124ab5d60) at /Volumes/data/code/firefox/nightly/nsprpub/pr/src/pthreads/ptsynch.c:164 #6 0x0000000101bdb286 in mozilla::Mutex::~Mutex () at /Volumes/data/code/firefox/obj/debug/dist/include/mozilla/Mutex.h:62 #7 0x0000000101bdb286 in mozilla::MediaEngineWebRTC::~MediaEngineWebRTC (this=0x120646cf0) at Mutex.h:58 #8 0x0000000101bdb1fe in mozilla::MediaEngineWebRTC::~MediaEngineWebRTC () at /Volumes/data/code/firefox/obj/debug/dist/include/MediaEngineWebRTC.h:237 #9 0x0000000101bdb1fe in mozilla::MediaEngineWebRTC::~MediaEngineWebRTC (this=0x120646cf0) at mozalloc.h:237 #10 0x0000000101bde386 in mozilla::MediaManager::~MediaManager (this=0x107aec8a0) at MediaManager.h:332 11 0x0000000101bd8274 in nsAutoRefCnt::operator= () at /Volumes/data/code/firefox/obj/debug/dist/include/nsISupportsImpl.h:331 #12 0x0000000101bd8274 in mozilla::MediaManager::Release (this=0x107aec8a0) at mozalloc.h:688 #13 0x00000001028a22af in ReleaseObjects (aElement=<value temporarily unavailable, due to optimizations>) at nsCOMArray.cpp:134 #14 0x00000001028a6bef in nsVoidArray::EnumerateForwards (this=0x7fff5fbfeb20, aFunc=0x1028a22a0 <ReleaseObjects>, aData=0x0) at nsVoidArray.cpp:690 #15 0x00000001028a1ebe in nsAutoVoidArray::~nsAutoVoidArray () at /Volumes/data/code/firefox/nightly/xpcom/glue/nsVoidArray.h:144 #16 0x00000001028a1ebe in nsAutoVoidArray::~nsAutoVoidArray () at nsCOMArray.cpp:145 #17 0x00000001028a1ebe in nsCOMArray_base::Clear () at /Volumes/data/code/firefox/nightly/xpcom/glue/nsCOMArray.h:171 #18 0x00000001028a1ebe in nsCOMArray_base::~nsCOMArray_base (this=0x7fff5fbfeb90) at nsCOMArray.cpp:25 #19 0x00000001028c46c7 in nsObserverList::NotifyObservers (this=<value temporarily unavailable, due to optimizations>, aSubject=<value temporarily unavailable, due to optimizations>, aTopic=<value temporarily unavailable, due to optimizations>, someData=<value temporarily unavailable, due to optimizations>) at /Volumes/data/code/firefox/nightly/xpcom/ds/nsObserverList.cpp:101 #20 0x00000001028c5934 in nsTHashtable<nsObserverList>::GetEntry () at /Volumes/data/code/firefox/obj/debug/dist/include/nsTHashtable.h:161 #21 0x00000001028c5934 in nsObserverService::NotifyObservers (this=0x1079203c0, aSubject=0x10012b598, aTopic=0x1035c1c58 "xpcom-shutdown", someData=0x0) at /Volumes/data/code/firefox/nightly/xpcom/ds/nsObserverService.cpp:164
Updated•11 years ago
|
Assignee: nobody → rjesup
Whiteboard: [getUserMedia] → [getUserMedia][blocking-gum+]
Updated•11 years ago
|
Priority: -- → P1
Assignee | ||
Comment 3•11 years ago
|
||
I'm not seeing an assert, but I did get a deadlock apparently: (gdb) where #0 __lll_lock_wait () at ../nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:136 #1 0x0000003667809c8c in _L_lock_1024 () from /lib64/libpthread.so.0 #2 0x0000003667809c35 in __pthread_mutex_lock (mutex=0x7fffda858ed0) at pthread_mutex_lock.c:105 #3 0x00007ffff7adec2e in PR_Lock (lock=0x7fffda858ed0) at ../../../../../nsprpub/pr/src/pthreads/ptsynch.c:174 #4 0x00007ffff3800882 in mozilla::Mutex::Lock (this=0x7fffd01fb4c8) at ../../../xpcom/glue/BlockingResourceBase.cpp:228 #5 0x00007ffff1ba94f8 in mozilla::BaseAutoLock<mozilla::Mutex>::BaseAutoLock (this=0x7fffcd9f7ba0, aLock=..., _notifier=...) at ../../dist/include/mozilla/Mutex.h:153 #6 0x00007ffff2c1e3e0 in mozilla::MediaEngineWebRTC::EnumerateAudioDevices (this=0x7fffd01fb4c0, aASources=0x7fffcd9f7c50) at ../../../../content/media/webrtc/MediaEngineWebRTC.cpp:142 #7 0x00007ffff291c779 in mozilla::GetUserMediaDevicesRunnable::Run (this=0x7fffd83fdfd0) at ../../../dom/media/MediaManager.cpp:658
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•11 years ago
|
||
Closing as security-sensitive, as in frame 7: #7 0x00007ffff291c779 in mozilla::GetUserMediaDevicesRunnable::Run (this=0x7fffd83fdfd0) at ../../../dom/media/MediaManager.cpp:658 658 manager->GetBackend()->EnumerateAudioDevices(&audioSources); (gdb) p *manager $6 = {<nsIMediaManagerService> = {<nsISupports> = {_vptr.nsISupports = 0x5a5a5a5a5a5a5a5a}, <No data fields>}, <nsIObserver> = {<nsISupports> = {_vptr.nsISupports = 0x5a5a5a5a5a5a5a5a}, <No data fields>}, mRefCnt = {mValue = 1515870810}, _mOwningThread = { mThread = 0x5a5a5a5a5a5a5a5a}, mActiveWindows = This says we're using freed memory for a function call (though in this case we deadlocked first, and the manager object was freed while we were waiting, but very likely the timing could be tweaked to expose the problem).
Group: core-security
Assignee | ||
Comment 5•11 years ago
|
||
Assignee | ||
Updated•11 years ago
|
Attachment #695503 -
Flags: review?(tterribe)
Comment 6•11 years ago
|
||
Comment on attachment 695503 [details] [diff] [review] keep reference to MediaManager singleton Review of attachment 695503 [details] [diff] [review]: ----------------------------------------------------------------- I think you need to do this for GetUserMediaStreamRunnable, ErrorCallbackRunnable, and SuccessCallbackRunnable as well. Otherwise they can potentially re-create the singleton after it's set to nsnull in the shutdown observer.
Attachment #695503 -
Flags: review?(tterribe) → review-
Assignee | ||
Comment 7•11 years ago
|
||
Assignee | ||
Updated•11 years ago
|
Attachment #695503 -
Attachment is obsolete: true
Assignee | ||
Comment 8•11 years ago
|
||
Comment on attachment 696044 [details] [diff] [review] keep reference to MediaManager singleton Now does it for all of them.
Attachment #696044 -
Flags: review?(tterribe)
Comment 9•11 years ago
|
||
Comment on attachment 696044 [details] [diff] [review] keep reference to MediaManager singleton Review of attachment 696044 [details] [diff] [review]: ----------------------------------------------------------------- LGTM.
Attachment #696044 -
Flags: review?(tterribe) → review+
Assignee | ||
Comment 10•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/43187d64eea3
Target Milestone: --- → mozilla20
Assignee | ||
Comment 11•11 years ago
|
||
Was merged to m-c but not marked https://hg.mozilla.org/integration/mozilla-inbound/rev/43187d64eea3
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
status-firefox20:
--- → fixed
Comment 12•11 years ago
|
||
Verified on 12/30 with specified test case steps.
Status: RESOLVED → VERIFIED
Keywords: verifyme
Updated•11 years ago
|
Reporter | ||
Comment 13•11 years ago
|
||
(In reply to Randell Jesup [:jesup] from comment #11) > Was merged to m-c but not marked > https://hg.mozilla.org/integration/mozilla-inbound/rev/43187d64eea3 I believe you meant to add the changelog from m-c which is: http://hg.mozilla.org/mozilla-central/rev/43187d64eea3
Updated•11 years ago
|
Flags: in-testsuite-
Flags: in-qa-testsuite?
Reporter | ||
Comment 14•11 years ago
|
||
Jason, please add my name when requesting in-qa-testsuite. Thanks.
Flags: in-qa-testsuite? → in-qa-testsuite?(hskupin)
Updated•11 years ago
|
status-b2g18:
--- → disabled
status-firefox-esr17:
--- → unaffected
Comment 15•11 years ago
|
||
As a WebRTC, this is disabled in 19 and earlier behind a pref, so 19 and earlier are unaffected, right?
Comment 16•11 years ago
|
||
Marking this as '19 disabled.'
status-firefox19:
--- → disabled
Whiteboard: [getUserMedia][blocking-gum+] → [getUserMedia][blocking-gum+][adv-main20-]
Updated•11 years ago
|
Group: core-security
Reporter | ||
Comment 17•10 years ago
|
||
Nils, can you have a look and check which type of test would be good here? Not sure if a crashtest works with the requirements we need for webrtc code.
Flags: in-qa-testsuite?(hskupin) → in-qa-testsuite?(drno)
Reporter | ||
Comment 18•10 years ago
|
||
Oh, I see it's for a shutdown crash, so Mozmill would then be necessary. Nils, do you think it's worth creating one?
Flags: needinfo?(drno)
Flags: in-qa-testsuite?(drno)
Flags: in-qa-testsuite?
Comment 19•10 years ago
|
||
Please see https://bugzilla.mozilla.org/show_bug.cgi?id=799142#c54 for my request of writing one Mozmill test to cover these scenarios.
Flags: needinfo?(drno)
You need to log in
before you can comment on or make changes to this bug.
Description
•