Tested on: OS: Ubuntu 12.04 Firefox: ASAN debug-build from https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux64-dbg-asan/1371553271/ To reproduce you might need to hit refresh few times after opening the repro-file in browser. ASAN-report: ==22795== ERROR: AddressSanitizer: heap-use-after-free on address 0x7fc6b99799c0 at pc 0x7fc6d324094a bp 0x7fc6a80861f0 sp 0x7fc6a80861e8 READ of size 4 at 0x7fc6b99799c0 thread T23 #0 0x7fc6d3240949 in nsTArray_base<nsTArrayInfallibleAllocator>::Length() const /builds/slave/m-cen-l64-dbg-asan-00000000000/build/obj-firefox/toolkit/xre/../../dist/include/nsTArray.h:363 #1 0x7fc6d33400b8 in nsTArray_base<nsTArrayInfallibleAllocator>::IsEmpty() const /builds/slave/m-cen-l64-dbg-asan-00000000000/build/../../../dist/include/nsTArray.h:368 #2 0x7fc6d467450a in mozilla::MediaStreamGraphImpl::PrepareUpdatesToMainThreadState(bool) /builds/slave/m-cen-l64-dbg-asan-00000000000/build/content/media/MediaStreamGraph.cpp:913 #3 0x7fc6d4675d49 in mozilla::MediaStreamGraphImpl::RunThread() /builds/slave/m-cen-l64-dbg-asan-00000000000/build/content/media/MediaStreamGraph.cpp:1124 #4 0x7fc6d4684e88 in mozilla::(anonymous namespace)::MediaStreamGraphThreadRunnable::Run() /builds/slave/m-cen-l64-dbg-asan-00000000000/build/content/media/MediaStreamGraph.cpp:1223 #5 0x7fc6d656bb3b in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-cen-l64-dbg-asan-00000000000/build/xpcom/threads/nsThread.cpp:626 . . . freed by thread T0 here: #0 0x43b3c5 in __interceptor_realloc ??:0 #1 0x7fc6de3cc4ba in moz_xrealloc /builds/slave/m-cen-l64-dbg-asan-00000000000/build/memory/mozalloc/mozalloc.cpp:86 #2 0x7fc6d3325013 in nsTArray_base<nsTArrayInfallibleAllocator>::EnsureCapacity(unsigned int, unsigned int) /builds/slave/m-cen-l64-dbg-asan-00000000000/build/../../dist/include/nsTArray-inl.h:167 #3 0x7fc6d467c529 in nsAutoPtr<mozilla::ControlMessage>* nsTArray_Impl<nsAutoPtr<mozilla::ControlMessage>, nsTArrayInfallibleAllocator>::AppendElements<mozilla::ControlMessage*>(mozilla::ControlMessage* const*, unsigned int) /builds/slave/m-cen-l64-dbg-asan-00000000000/build/../../dist/include/nsTArray.h:1044 #4 0x7fc6d4677a9c in mozilla::MediaStreamGraphImpl::AppendMessage(mozilla::ControlMessage*) /builds/slave/m-cen-l64-dbg-asan-00000000000/build/content/media/MediaStreamGraph.cpp:1470 #5 0x7fc6d5d95ef0 in mozilla::dom::AudioParamBinding::setValueAtTime(JSContext*, JS::Handle<JSObject*>, mozilla::dom::AudioParam*, JSJitMethodCallArgs const&) /builds/slave/m-cen-l64-dbg-asan-00000000000/build/obj-firefox/dom/bindings/AudioParamBinding.cpp:118 . . .

This fixes it.

Comment on attachment 764775 [details] [diff] [review] patch Review of attachment 764775 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/media/MediaStreamGraph.cpp @@ +1468,5 @@ > } > > + { > + MonitorAutoLock lock(mMonitor); > + mCurrentTaskMessageQueue.AppendElement(aMessage); mCurrentTaskMessageQueue is supposed to be main-thread only, so why would this be necessary?

Well, |mCurrentTaskMessageQueue.IsEmpty()| is called on the MSG thread in MediaStreamGraphImpl::PrepareUpdatesToMainThreadState, which is called from MediaStreamGraphImpl::RunThread. Maybe this is our bug, then, I don't know MSG well enough yet to be sure. [1]: http://mxr.mozilla.org/mozilla-central/source/content/media/MediaStreamGraph.cpp#919

That's a bug that I've put in then <http://hg.mozilla.org/mozilla-central/rev/6e264b512357> :( Not sure how best to fix it though...

Well, I reviewed it... I think we just don't need to check !mCurrentTaskMessageQueue.IsEmpty(). It doesn't matter if there are pending messages from the main thread to the graph.

If that works, then great! I don't remember why I added that check, could be that was just a mistake.

This is green locally on content/media/test and content/media/webaudio/test.

Comment on attachment 766043 [details] [diff] [review] patch Review of attachment 766043 [details] [diff] [review]: ----------------------------------------------------------------- OK, I wrote this code, so, r=me. :-) Thanks!

Please land the test case as a crashtest alongside with the patch. Also, since this only happens on m-c you don't need sec-approval. It would be amazing if you can land this before the uplift (Monday.)

Somehow this did not get RESOLVED FIXED during the m-i -> m-c merge.

No longer tracking for FF23