Closed Bug 1040550 Opened 10 years ago Closed 10 years ago

Cycle collector crash in MediaSource

Categories

(Core :: Audio/Video, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla34

People

(Reporter: ajones, Assigned: jya)

References

()

Details

Attachments

(2 files, 1 obsolete file)

Steps to reproduce:

* Navigate to given URL
* Clock on load.

Expected results:

No segfault.

Actual results:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fff8f361700 (LWP 14681)]
mozilla::DOMEventTargetHelper::AddRef (this=<optimized out>) at /home/ajones/src/mozilla-central/dom/events/DOMEventTargetHelper.cpp:74
74	NS_IMPL_CYCLE_COLLECTING_ADDREF(DOMEventTargetHelper)
(gdb) bt
#0  mozilla::DOMEventTargetHelper::AddRef (this=<optimized out>) at /home/ajones/src/mozilla-central/dom/events/DOMEventTargetHelper.cpp:74
#1  0x00007ffff226fb6a in AddRef (this=<optimized out>, this=<optimized out>) at /home/ajones/src/mozilla-central/content/media/mediasource/SourceBuffer.cpp:564
#2  nsRunnable (aRawPtr=0x1b0f450, this=<optimized out>) at ../../../dist/include/nsAutoPtr.h:885
#3  operator new (this=0x16b7890, size=<optimized out>, aRawPtr=<optimized out>, aName=<optimized out>, size=<optimized out>) at /home/ajones/src/mozilla-central/content/media/mediasource/AsyncEventRunner.h:21
#4  mozilla::dom::SourceBuffer::QueueAsyncSimpleEvent (this=0x1b0f450, aName=0x7ffff3e1281f <.L.str59> "updatestart") at /home/ajones/src/mozilla-central/content/media/mediasource/SourceBuffer.cpp:393
#5  0x00007ffff226f31e in mozilla::dom::SourceBuffer::Remove (this=0x1b0f450, aStart=<optimized out>, aEnd=<optimized out>, aRv=...) at /home/ajones/src/mozilla-central/content/media/mediasource/SourceBuffer.cpp:320
#6  0x00007ffff226c4ae in mozilla::dom::SourceBufferList::Remove (this=0x429a660, aStart=0, aEnd=260.26600000000002, aRv=...) at /home/ajones/src/mozilla-central/content/media/mediasource/SourceBufferList.cpp:103
#7  0x00007ffff226e276 in mozilla::MediaSourceReader::ReadMetadata (this=0x387f120, aInfo=0x7fff8f360ab8, aTags=0x2e7c5c8) at /home/ajones/src/mozilla-central/content/media/mediasource/MediaSourceDecoder.cpp:564
#8  0x00007ffff222f018 in mozilla::MediaDecoderStateMachine::DecodeMetadata (this=0x2e7c5b0) at /home/ajones/src/mozilla-central/content/media/MediaDecoderStateMachine.cpp:1866
#9  0x00007ffff222debc in mozilla::MediaDecoderStateMachine::CallDecodeMetadata (this=0x2e7c5b0) at /home/ajones/src/mozilla-central/content/media/MediaDecoderStateMachine.cpp:1847
#10 0x00007ffff2253437 in nsRunnableMethodImpl<void (mozilla::MediaDecoderStateMachine::*)(), void, true>::Run (this=<optimized out>) at ../../dist/include/nsThreadUtils.h:391
#11 0x00007ffff224589b in mozilla::MediaTaskQueue::Runner::Run (this=0x2ed5db0) at /home/ajones/src/mozilla-central/content/media/MediaTaskQueue.cpp:127
#12 0x00007ffff05fdb90 in nsThreadPool::Run (this=0x2b0cba0) at /home/ajones/src/mozilla-central/xpcom/threads/nsThreadPool.cpp:220
#13 0x00007ffff05fdc7d in non-virtual thunk to nsThreadPool::Run() () at Unified_cpp_xpcom_threads0.cpp:234
#14 0x00007ffff05faffb in nsThread::ProcessNextEvent (this=0x7fff04925660, aMayWait=<optimized out>, aResult=0x7fff8f360d87) at /home/ajones/src/mozilla-central/xpcom/threads/nsThread.cpp:766
#15 0x00007ffff055ffb3 in NS_ProcessNextEvent (aThread=<optimized out>, aMayWait=false) at /home/ajones/src/mozilla-central/xpcom/glue/nsThreadUtils.cpp:265
#16 0x00007ffff0982817 in mozilla::ipc::MessagePumpForNonMainThreads::Run (this=0x7ffed00015d0, aDelegate=0x7ffed0000cf0) at /home/ajones/src/mozilla-central/ipc/glue/MessagePump.cpp:326
#17 0x00007ffff094c283 in RunInternal (this=0x7ffed0000cf0) at /home/ajones/src/mozilla-central/ipc/chromium/src/base/message_loop.cc:229
#18 RunHandler (this=0x7ffed0000cf0) at /home/ajones/src/mozilla-central/ipc/chromium/src/base/message_loop.cc:222
#19 MessageLoop::Run (this=0x7ffed0000cf0) at /home/ajones/src/mozilla-central/ipc/chromium/src/base/message_loop.cc:196
#20 0x00007ffff05f9b1e in nsThread::ThreadFunc (aArg=0x7fff04925660) at /home/ajones/src/mozilla-central/xpcom/threads/nsThread.cpp:346
#21 0x00007ffff6bbd82e in _pt_root (arg=0x7fff04925a20) at /home/ajones/src/mozilla-central/nsprpub/pr/src/pthreads/ptthread.c:212
#22 0x00007ffff7bc4182 in start_thread (arg=0x7fff8f361700) at pthread_create.c:312
#23 0x00007ffff6ecd30d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:111
(gdb)
I get from a nightly build:

"Codec (video/mp4;codecs="avc1.4D400D") is not supported."

When clicking load. Do I need to do anything else in your STR?
Need to add a pref for media.mediasource.ignore_codecs=true
Still getting the message from comment 1. Anything else different in your build? I do get this now though after the message:

Assertion failure: (!HasVideo() && !HasAudio()) || !(mDecoder->IsMediaSeekable() && mDecoder->IsTransportSeekable()) || mEndTime != -1 (Active seekable media should have end time), at /home/chris/src/gecko-dev/content/media/MediaDecoderStateMachine.cpp:1956
Attached patch use_mp4.patchSplinter Review
Sorry... my bad. I'm using the attached patch that enables MP4 and disables WebM.
(In reply to Anthony Jones (:kentuckyfriedtakahe, :k17e) from comment #4)
> Sorry... my bad. I'm using the attached patch that enables MP4 and disables
> WebM.

Still getting the same result with that patch applied, sorry.
media.fragmented-mp4.exposed=true
media.fragmented-mp4.ffmpeg.enabled=true
Missed the beginning of the backtrace which looks important:

Hit MOZ_CRASH(DOMEventTargetHelper not thread-safe) at /home/ajones/src/mozilla-central/dom/events/DOMEventTargetHelper.cpp:74
UNKNOWN [/home/ajones/objdir/mozilla-central/dist/bin/libxul.so +0x025D180E]
UNKNOWN [/home/ajones/objdir/mozilla-central/dist/bin/libxul.so +0x025CE99E]
UNKNOWN [/home/ajones/objdir/mozilla-central/dist/bin/libxul.so +0x025D0766]
UNKNOWN [/home/ajones/objdir/mozilla-central/dist/bin/libxul.so +0x02591908]
UNKNOWN [/home/ajones/objdir/mozilla-central/dist/bin/libxul.so +0x025907AC]
UNKNOWN [/home/ajones/objdir/mozilla-central/dist/bin/libxul.so +0x025B5A57]
UNKNOWN [/home/ajones/objdir/mozilla-central/dist/bin/libxul.so +0x025A7F7B]
UNKNOWN [/home/ajones/objdir/mozilla-central/dist/bin/libxul.so +0x0095FA00]
UNKNOWN [/home/ajones/objdir/mozilla-central/dist/bin/libxul.so +0x0095FAED]
UNKNOWN [/home/ajones/objdir/mozilla-central/dist/bin/libxul.so +0x0095CE6B]
UNKNOWN [/home/ajones/objdir/mozilla-central/dist/bin/libxul.so +0x008C1E23]
UNKNOWN [/home/ajones/objdir/mozilla-central/dist/bin/libxul.so +0x00CE474A]
UNKNOWN [/home/ajones/objdir/mozilla-central/dist/bin/libxul.so +0x00CAE113]
UNKNOWN [/home/ajones/objdir/mozilla-central/dist/bin/libxul.so +0x0095B98E]
UNKNOWN [/home/ajones/objdir/mozilla-central/dist/bin/libnspr4.so +0x0002D82E]
UNKNOWN [/lib/x86_64-linux-gnu/libpthread.so.0 +0x00008182]
clone+0x0000006D [/lib/x86_64-linux-gnu/libc.so.6 +0x000FB30D]
UNKNOWN (nil)
#6  0x00007ffff226c4ae in mozilla::dom::SourceBufferList::Remove (this=0x429a660, aStart=0, aEnd=260.26600000000002, aRv=...) at /home/ajones/src/mozilla-central/content/media/mediasource/SourceBufferList.cpp:103
#7  0x00007ffff226e276 in mozilla::MediaSourceReader::ReadMetadata (this=0x387f120, aInfo=0x7fff8f360ab8, aTags=0x2e7c5c8) at /home/ajones/src/mozilla-central/content/media/mediasource/MediaSourceDecoder.cpp:564

This stack looks strange; MSR::ReadMetadata doesn't call SBL::Remove.
I can repro on Win8 with:

http://dash-mse-test.appspot.com/dash-player.html?url=http://yt-dash-mse-test.commondatastorage.googleapis.com/media/car-20120827-manifest.mpd

With:
media.fragmented-mp4.exposed = true
media.mediasource.ignore_codecs = true
media.mediasource.enabled = true

Stack:

xul.dll!mozilla::dom::SourceBuffer::AddRef() Line 564	C++
xul.dll!nsRefPtr<mozilla::dom::SourceBuffer>::nsRefPtr<mozilla::dom::SourceBuffer>(mozilla::dom::SourceBuffer * aRawPtr) Line 887	C++
xul.dll!mozilla::AsyncEventRunner<mozilla::dom::SourceBuffer>::AsyncEventRunner<mozilla::dom::SourceBuffer>(mozilla::dom::SourceBuffer * aTarget, const char * aName) Line 21	C++
xul.dll!mozilla::dom::SourceBuffer::QueueAsyncSimpleEvent(const char * aName) Line 393	C++
xul.dll!mozilla::dom::SourceBuffer::StartUpdating() Line 427	C++
xul.dll!mozilla::dom::SourceBuffer::Remove(double aStart, double aEnd, mozilla::ErrorResult & aRv) Line 322	C++
xul.dll!mozilla::dom::SourceBufferList::Remove(double aStart, double aEnd, mozilla::ErrorResult & aRv) Line 104	C++
xul.dll!mozilla::dom::MediaSource::DurationChange(double aNewDuration, mozilla::ErrorResult & aRv) Line 375	C++
xul.dll!mozilla::dom::MediaSource::SetDuration(double aDuration, mozilla::ErrorResult & aRv) Line 155	C++
xul.dll!mozilla::MediaSourceReader::ReadMetadata(mozilla::MediaInfo * aInfo, nsDataHashtable<nsCStringHashKey,nsCString> * * aTags) Line 568	C++
xul.dll!mozilla::MediaDecoderStateMachine::DecodeMetadata() Line 1866	C++
xul.dll!mozilla::MediaDecoderStateMachine::CallDecodeMetadata() Line 1847	C++
xul.dll!nsRunnableMethodImpl<void (__thiscall mozilla::MediaDecoderStateMachine::*)(void),void,1>::Run() Line 393	C++
xul.dll!mozilla::MediaTaskQueue::Runner::Run() Line 134	C++

MSR::ReadMetadata calls ReadMetadata() which calls MediaSource::SetDuration() which calls MediaSource::DurationChange() which ends up addrefing a SourceBuffer to dispatch an event which has non-threadsafe refcounting which is asserting that it's being addrefed on a thread other than the one it was created on.

Maybe the MediaSource::SetDuration() call should be proxied to the main thread in some way that doesn't involving addrefing the MediaSource on a non-main thread.
(In reply to Chris Pearce (:cpearce) from comment #9)
> which has
> non-threadsafe refcounting which is asserting that it's being addrefed on a
> thread other than the one it was created on.


I meant: "which asserts that it's only addrefed on the thread that created it." That assert is failing here, as ReadMetadata() runs on the decode task queue.
Assignee: nobody → jyavenard
Comment on attachment 8462329 [details] [diff] [review]
Prevent MediaSource::SetDuration from being called outside the main thread

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

r+ with comments

::: content/media/mediasource/MediaSourceDecoder.cpp
@@ +222,5 @@
>      }
>      return mDecoders[mActiveVideoDecoder]->GetReader();
>    }
>  
> +  void SetMediaSourceDuration(double aDuration) {

New line before {

@@ +223,5 @@
>      return mDecoders[mActiveVideoDecoder]->GetReader();
>    }
>  
> +  void SetMediaSourceDuration(double aDuration) {
> +    // Can only ever be called from the main thread

Replace this comment with
  MOZ_ASSERT(NS_IsMainThread());
Attachment #8462329 - Flags: review?(kinetik) → review+
(In reply to Matthew Gregan [:kinetik] from comment #12)
> Comment on attachment 8462329 [details] [diff] [review]
> Prevent MediaSource::SetDuration from being called outside the main thread
> 
> Review of attachment 8462329 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r+ with comments
> 
> ::: content/media/mediasource/MediaSourceDecoder.cpp
> @@ +222,5 @@
> >      }
> >      return mDecoders[mActiveVideoDecoder]->GetReader();
> >    }
> >  
> > +  void SetMediaSourceDuration(double aDuration) {
> 
> New line before {

Most functions in that part of the code use that format:
  void Shutdown() MOZ_OVERRIDE {

  virtual void BreakCycles() MOZ_OVERRIDE {

etc...

should I modify all of those too ?

> > +  void SetMediaSourceDuration(double aDuration) {
> > +    // Can only ever be called from the main thread
> 
> Replace this comment with
>   MOZ_ASSERT(NS_IsMainThread());

will do.
(In reply to jyavenard@mozilla.com from comment #13)
> should I modify all of those too ?

Oh, I missed that you'd included it inside the class rather than standalone... pre-coffee reviewing.  It's fine as is, in that case.
Attachment #8462329 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/8111090f94a6
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.