Closed Bug 865004 Opened 12 years ago Closed 12 years ago

Crash on shutdown with many ScriptProcessorNodes (AudioContext.createJavaScriptNode)

Categories

(Core :: Web Audio, defect)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla23
Tracking Status
firefox20 --- unaffected
firefox21 --- unaffected
firefox22 --- unaffected
firefox23 + fixed
firefox-esr17 --- unaffected
b2g18 --- unaffected

People

(Reporter: jruderman, Assigned: ehsan.akhgari)

References

Details

(5 keywords, Whiteboard: [fuzzblocker:AudioContext][adv-main23-])

Attachments

(5 files)

No description provided.
Assertion failure: 0 == rv, at /Users/jruderman/trees/mozilla-central/nsprpub/pr/src/pthreads/ptsynch.c:175
Whiteboard: [fuzzblocker:AudioContext]
Looks like it could be a dupe of bug 864709, which has patches. That other bug sounds like a UAF so I assume this is, too.
Depends on: 864709
Blocks: webaudio
OK, so what's going on here is some pretty evil things :( This is what we have on the main thread: #0 0x00007fff85a6f122 in __psynch_mutexwait () #1 0x00007fff87ed4dfd in pthread_mutex_lock () #2 0x0000000101234580 in PR_Lock () #3 0x00000001040891e7 in mozilla::Mutex::Lock (this=0x1247a05d0) at BlockingResourceBase.cpp:228 #4 0x00000001016474c7 in mozilla::BaseAutoLock<mozilla::Mutex>::BaseAutoLock (this=0x7fff5fbfe3f0, aLock=@0x1247a05d0, _notifier=@0x7fff5fbfe3e8) at Mutex.h:153 #5 0x0000000101646675 in mozilla::BaseAutoLock<mozilla::Mutex>::BaseAutoLock (this=0x7fff5fbfe3f0, aLock=@0x1247a05d0, _notifier=@0x7fff5fbfe3e8) at Mutex.h:154 #6 0x00000001026474d6 in mozilla::dom::AudioNode::DestroyMediaStream (this=0x124089040) at /Users/ehsanakhgari/moz/mozilla-central/content/media/webaudio/AudioNode.cpp:214 #7 0x00000001026466f6 in mozilla::dom::AudioNode::DisconnectFromGraph (this=0x124089040) at /Users/ehsanakhgari/moz/mozilla-central/content/media/webaudio/AudioNode.cpp:99 #8 0x0000000102646ba7 in mozilla::dom::AudioNode::~AudioNode (this=0x124089040) at /Users/ehsanakhgari/moz/mozilla-central/content/media/webaudio/AudioNode.cpp:41 #9 0x00000001026594a1 in mozilla::dom::ScriptProcessorNode::~ScriptProcessorNode (this=0x124089040) at /Users/ehsanakhgari/moz/mozilla-central/content/media/webaudio/ScriptProcessorNode.cpp:385 #10 0x0000000102659405 in mozilla::dom::ScriptProcessorNode::~ScriptProcessorNode (this=0x124089040) at /Users/ehsanakhgari/moz/mozilla-central/content/media/webaudio/ScriptProcessorNode.cpp:381 #11 0x00000001026593d9 in mozilla::dom::ScriptProcessorNode::~ScriptProcessorNode (this=0x124089040) at /Users/ehsanakhgari/moz/mozilla-central/content/media/webaudio/ScriptProcessorNode.cpp:381 #12 0x000000010235ebef in nsDOMEventTargetHelper::Release (this=0x124089040) at /Users/ehsanakhgari/moz/mozilla-central/content/events/src/nsDOMEventTargetHelper.cpp:74 #13 0x000000010264683f in mozilla::dom::AudioNode::Release (this=0x124089040) at /Users/ehsanakhgari/moz/mozilla-central/content/media/webaudio/AudioNode.cpp:27 #14 0x00000001026590df in mozilla::dom::ScriptProcessorNode::Release (this=0x124089040) at /Users/ehsanakhgari/moz/mozilla-central/content/media/webaudio/ScriptProcessorNode.cpp:35 #15 0x0000000102fd5b1b in DoDeferredRelease (array=@0x100522af8) at /Users/ehsanakhgari/moz/mozilla-central/js/xpconnect/src/XPCJSRuntime.cpp:614 #16 0x0000000102fd5997 in XPCJSRuntime::GCCallback (rt=0x10d666000, status=JSGC_END) at /Users/ehsanakhgari/moz/mozilla-central/js/xpconnect/src/XPCJSRuntime.cpp:814 #17 0x0000000105062354 in Collect (rt=0x10d666000, incremental=false, budget=0, gckind=js::GC_NORMAL, reason=JS::gcreason::API) at /Users/ehsanakhgari/moz/mozilla-central/js/src/jsgc.cpp:4611 #18 0x000000010505ea6c in js::GC (rt=0x10d666000, gckind=js::GC_NORMAL, reason=JS::gcreason::API) at /Users/ehsanakhgari/moz/mozilla-central/js/src/jsgc.cpp:4628 #19 0x0000000104f90d40 in JS_GC (rt=0x10d666000) at /Users/ehsanakhgari/moz/mozilla-central/js/src/jsapi.cpp:2930 #20 0x000000010157826e in nsXREDirProvider::DoShutdown (this=0x7fff5fbfea38) at /Users/ehsanakhgari/moz/mozilla-central/toolkit/xre/nsXREDirProvider.cpp:865 #21 0x000000010155aee3 in ScopedXPCOMStartup::~ScopedXPCOMStartup (this=0x10050e480) at /Users/ehsanakhgari/moz/mozilla-central/toolkit/xre/nsAppRunner.cpp:1114 #22 0x000000010155ae15 in ScopedXPCOMStartup::~ScopedXPCOMStartup (this=0x10050e480) at /Users/ehsanakhgari/moz/mozilla-central/toolkit/xre/nsAppRunner.cpp:1100 #23 0x0000000101564823 in XREMain::XRE_main (this=0x7fff5fbfea00, argc=6, argv=0x7fff5fbff308, aAppData=0x7fff5fbfec98) at /Users/ehsanakhgari/moz/mozilla-central/toolkit/xre/nsAppRunner.cpp:3970 #24 0x0000000101564c2d in XRE_main (argc=6, argv=0x7fff5fbff308, aAppData=0x7fff5fbfec98, aFlags=0) at /Users/ehsanakhgari/moz/mozilla-central/toolkit/xre/nsAppRunner.cpp:4146 #25 0x0000000100002387 in do_main (argc=6, argv=0x7fff5fbff308, xreDirectory=0x100531440) at /Users/ehsanakhgari/moz/mozilla-central/browser/app/nsBrowserApp.cpp:271 #26 0x0000000100001891 in main (argc=6, argv=0x7fff5fbff308) at /Users/ehsanakhgari/moz/mozilla-central/browser/app/nsBrowserApp.cpp:576 Note that the ScriptProcessorNode part of the object is fully destroyed there, and then we call ~AudioNode which calls DestroyMediaStream. Now, in the MSG thread, we have: #0 0x0000000104088a6e in mozilla::BlockingResourceBase::CheckAcquire (this=0x1247a0550, aCallContext=@0x1204a0720) at BlockingResourceBase.cpp:97 #1 0x00000001040891da in mozilla::Mutex::Lock (this=0x1247a0550) at BlockingResourceBase.cpp:227 #2 0x00000001016474c7 in mozilla::BaseAutoLock<mozilla::Mutex>::BaseAutoLock (this=0x1204a0800, aLock=@0x1247a0550, _notifier=@0x1204a07f8) at Mutex.h:153 #3 0x0000000101646675 in mozilla::BaseAutoLock<mozilla::Mutex>::BaseAutoLock (this=0x1204a0800, aLock=@0x1247a0550, _notifier=@0x1204a07f8) at Mutex.h:154 #4 0x000000010265b237 in mozilla::dom::SharedBuffers::GetOutputBuffer (this=0x1247a0550) at /Users/ehsanakhgari/moz/mozilla-central/content/media/webaudio/ScriptProcessorNode.cpp:127 #5 0x000000010265b0d1 in mozilla::dom::ScriptProcessorNodeEngine::ProduceAudioBlock (this=0x1247a05c0, aStream=0x1230c6d80, aInput=@0x1204a09c8, aOutput=0x1204a09f0, aFinished=0x1204a09bf) at /Users/ehsanakhgari/moz/mozilla-central/content/media/webaudio/ScriptProcessorNode.cpp:217 #6 0x00000001025d6baa in mozilla::AudioNodeStream::ProduceOutput (this=0x1230c6d80, aFrom=2648004, aTo=2650801) at /Users/ehsanakhgari/moz/mozilla-central/content/media/AudioNodeStream.cpp:295 #7 0x00000001025f643c in mozilla::MediaStreamGraphImpl::ProduceDataForStreamsBlockByBlock (this=0x11f32dfc0, aStreamIndex=0, aFrom=2645208, aTo=2664782) at /Users/ehsanakhgari/moz/mozilla-central/content/media/MediaStreamGraph.cpp:930 #8 0x00000001025f68b2 in mozilla::MediaStreamGraphImpl::RunThread (this=0x11f32dfc0) at /Users/ehsanakhgari/moz/mozilla-central/content/media/MediaStreamGraph.cpp:1001 #9 0x00000001025fb399 in mozilla::(anonymous namespace)::MediaStreamGraphThreadRunnable::Run (this=0x11bf52d00) at /Users/ehsanakhgari/moz/mozilla-central/content/media/MediaStreamGraph.cpp:1126 #10 0x00000001041248c6 in nsThread::ProcessNextEvent (this=0x10e2a3300, mayWait=true, result=0x1204a0dde) at /Users/ehsanakhgari/moz/mozilla-central/xpcom/threads/nsThread.cpp:627 #11 0x0000000104083f49 in NS_ProcessNextEvent (thread=0x10e2a3300, mayWait=true) at nsThreadUtils.cpp:238 #12 0x00000001041232b7 in nsThread::ThreadFunc (arg=0x10e2a3300) at /Users/ehsanakhgari/moz/mozilla-central/xpcom/threads/nsThread.cpp:265 #13 0x0000000101239ac5 in _pt_root () #14 0x00007fff87ecf7a2 in _pthread_start () #15 0x00007fff87ebc1e1 in thread_start () In other words, we have grabbed the lock first in the MSG thread, checked Node() to make sure it's non-null, then proceeded to access mSharedBuffers. Then we are interrupted and the main thread has gotten scheduled, which has destroyed the ScriptProcessorNode part (including what mSharedBuffers points to), and then we tried to grab the lock on the main thread which makes it wait, and then the MSG thread got scheduled again and we tried to access mSharedBuffers after it was freed. Now, technically, this would not have been an issue if we backed out bug 864086. That will fix this problem since when you call DestroyMediaStream, your entire object is still living. But I'd really like to avoid doing that since I still think that requiring all derived classes to call a function in their destructor is pretty bad. Another option, which I like better, is to hack AudioNode::Release to call DisconnectFromGraph _before_ calling delete this. Let's see if I can get that to work...
And in case I was not clear before, bug 864709 does not fix this.
Attached patch Patch (v1) (deleted) — Splinter Review
Assignee: nobody → ehsan
Status: NEW → ASSIGNED
Attachment #741469 - Flags: review?
Comment on attachment 741469 [details] [diff] [review] Patch (v1) Do we not need NS_IMETHODIMP_(nsrefcnt) in the .cpp? r=me, I think. The refcount logging won't mention AudioNode, but it'll mention the subclasses so should be ok.
Attachment #741469 - Flags: review? → review+
Backed out because of crashtest leak reports: https://hg.mozilla.org/integration/mozilla-inbound/rev/787287a94cc4 Those are most likely coming from the mismatching in refcount logging. Let me see how I can fix that.
I think the problem is that I removed the QueryInterface override, which causes the wrong cycle collection participant to be returned from QI. Let's see if undoing that fixes this? https://tbpl.mozilla.org/?tree=Try&rev=9f05cf656394
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
Mass moving Web Audio bugs to the Web Audio component. Filter on duckityduck.
Component: Video/Audio → Web Audio
Whiteboard: [fuzzblocker:AudioContext] → [fuzzblocker:AudioContext][adv-main23-]
Group: core-security
Flags: in-testsuite? → in-testsuite+
Pushed by eakhgari@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7c4cd68156c1 Add a crashtest based on the test case for the bug

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Keywords: regression
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: