826104 - Crash in MediaDecoder::UpdatePlaybackOffset

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Matt Wobensmith [:mwobensmith][:matt:]]

This was surfaced after bug #784918 was fixed. Use Abhishek's sample file for that bug to reproduce this one.

Also, if bug does not reproduce at first, try loading in several tabs at once and refreshing page.



=================================================================
==87236== ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x000105579b20 sp 0x000190549fe0 bp 0x00019054a0d0 T409)
AddressSanitizer can not provide additional info.
    #0 0x105579b1f in mozilla::MediaDecoder::UpdatePlaybackOffset(long long) (in XUL) + 175
    #1 0x105588d42 in mozilla::MediaDecoderStateMachine::PlayFromAudioQueue(unsigned long long, unsigned int) (in XUL) + 1394
    #2 0x1055877be in mozilla::MediaDecoderStateMachine::AudioLoop() (in XUL) + 4238
    #3 0x105594239 in nsRunnableMethodImpl<void (mozilla::MediaDecoderStateMachine::*)(), true>::Run() (in XUL) + 137
    #4 0x1069c2f4b in nsThread::ProcessNextEvent(bool, bool*) (in XUL) + 2139
    #5 0x106909a72 in NS_ProcessNextEvent_P(nsIThread*, bool) (in XUL) + 194
    #6 0x1069c122c in nsThread::ThreadFunc(void*) (in XUL) + 412
    #7 0x103258f1d in _pt_root (in libnspr4.dylib) + 749
    #8 0x1000113ea in 0x2000113ea
    #9 0x7fff913be180 in thread_start (in libsystem_c.dylib) + 12
Thread T409 created by T28 here:
    #0 0x1000090b4 in 0x2000090b4
    #1 0x1032548bc in _PR_CreateThread (in libnspr4.dylib) + 1612
    #2 0x10325425a in PR_CreateThread (in libnspr4.dylib) + 26
    #3 0x1069c1c67 in nsThread::Init() (in XUL) + 327
    #4 0x1069c6717 in nsThreadManager::NewThread(unsigned int, unsigned int, nsIThread**) (in XUL) + 119
    #5 0x106909267 in NS_NewThread_P(nsIThread**, nsIRunnable*, unsigned int) (in XUL) + 199
    #6 0x1051b1ecc in tag_nsresult NS_NewNamedThread_P<12ul>(char const (&) [12ul], nsIThread**, nsIRunnable*, unsigned int) (in XUL) + 28
    #7 0x105589dab in mozilla::MediaDecoderStateMachine::StartAudioThread() (in XUL) + 539
    #8 0x105589ada in mozilla::MediaDecoderStateMachine::StartPlayback() (in XUL) + 474
    #9 0x10558f5f5 in mozilla::MediaDecoderStateMachine::AdvanceFrame() (in XUL) + 2405
    #10 0x10558deac in mozilla::MediaDecoderStateMachine::RunStateMachine() (in XUL) + 492
    #11 0x105590f2d in mozilla::MediaDecoderStateMachine::CallRunStateMachine() (in XUL) + 541
    #12 0x105590cb7 in mozilla::MediaDecoderStateMachine::Run() (in XUL) + 231
    #13 0x1069c2f4b in nsThread::ProcessNextEvent(bool, bool*) (in XUL) + 2139
    #14 0x106909a72 in NS_ProcessNextEvent_P(nsIThread*, bool) (in XUL) + 194
    #15 0x1069c243c in nsThread::Shutdown() (in XUL) + 764
    #16 0x10558c8ff in mozilla::MediaDecoderStateMachine::StopAudioThread() (in XUL) + 655
    #17 0x105590ea5 in mozilla::MediaDecoderStateMachine::CallRunStateMachine() (in XUL) + 405
    #18 0x105590cb7 in mozilla::MediaDecoderStateMachine::Run() (in XUL) + 231
    #19 0x1069c2f4b in nsThread::ProcessNextEvent(bool, bool*) (in XUL) + 2139
    #20 0x106909a72 in NS_ProcessNextEvent_P(nsIThread*, bool) (in XUL) + 194
    #21 0x1069c122c in nsThread::ThreadFunc(void*) (in XUL) + 412
    #22 0x103258f1d in _pt_root (in libnspr4.dylib) + 749
    #23 0x1000113ea in 0x2000113ea
    #24 0x7fff913be180 in thread_start (in libsystem_c.dylib) + 12
Thread T28 created by T0 here:
    #0 0x1000090b4 in 0x2000090b4
    #1 0x1032548bc in _PR_CreateThread (in libnspr4.dylib) + 1612
    #2 0x10325425a in PR_CreateThread (in libnspr4.dylib) + 26
    #3 0x1069c1c67 in nsThread::Init() (in XUL) + 327
    #4 0x1069c6717 in nsThreadManager::NewThread(unsigned int, unsigned int, nsIThread**) (in XUL) + 119
    #5 0x106909267 in NS_NewThread_P(nsIThread**, nsIRunnable*, unsigned int) (in XUL) + 199
    #6 0x1051b1ecc in tag_nsresult NS_NewNamedThread_P<12ul>(char const (&) [12ul], nsIThread**, nsIRunnable*, unsigned int) (in XUL) + 28
    #7 0x10557e344 in mozilla::StateMachineTracker::EnsureGlobalStateMachine() (in XUL) + 356
    #8 0x10557fb6d in mozilla::MediaDecoderStateMachine::MediaDecoderStateMachine(mozilla::MediaDecoder*, mozilla::MediaDecoderReader*, bool) (in XUL) + 1821
    #9 0x1055fd6f2 in mozilla::WaveDecoder::CreateStateMachine() (in XUL) + 66
    #10 0x1055726c6 in mozilla::MediaDecoder::Load(mozilla::MediaResource*, nsIStreamListener**, mozilla::MediaDecoder*) (in XUL) + 134
    #11 0x104db717f in nsHTMLMediaElement::FinishDecoderSetup(mozilla::MediaDecoder*, mozilla::MediaResource*, nsIStreamListener**, mozilla::MediaDecoder*) (in XUL) + 943
    #12 0x104da857f in nsHTMLMediaElement::InitializeDecoderForChannel(nsIChannel*, nsIStreamListener**) (in XUL) + 719
    #13 0x104da7ee4 in nsHTMLMediaElement::MediaLoadListener::OnStartRequest(nsIRequest*, nsISupports*) (in XUL) + 1380
    #14 0x103c2301f in nsBaseChannel::OnStartRequest(nsIRequest*, nsISupports*) (in XUL) + 495
    #15 0x103c4ad53 in nsInputStreamPump::OnStateStart() (in XUL) + 515
    #16 0x103c4a8e2 in nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*) (in XUL) + 530
    #17 0x106989e0d in nsInputStreamReadyEvent::Run() (in XUL) + 125
    #18 0x1069c2f4b in nsThread::ProcessNextEvent(bool, bool*) (in XUL) + 2139
    #19 0x1069098ae in NS_ProcessPendingEvents_P(nsIThread*, unsigned int) (in XUL) + 254
    #20 0x1061a6813 in nsBaseAppShell::NativeEventCallback() (in XUL) + 451
    #21 0x1061266ca in nsAppShell::ProcessGeckoEvents(void*) (in XUL) + 490
    #22 0x7fff96815100 in __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ (in CoreFoundation) + 16
    #23 0x7fff96814a24 in __CFRunLoopDoSources0 (in CoreFoundation) + 244
    #24 0x7fff96837dc4 in __CFRunLoopRun (in CoreFoundation) + 788
    #25 0x7fff968376b1 in CFRunLoopRunSpecific (in CoreFoundation) + 289
    #26 0x7fff901db0a3 in RunCurrentEventLoopInMode (in HIToolbox) + 208
    #27 0x7fff901dad83 in ReceiveNextEventCommon (in HIToolbox) + 165
    #28 0x7fff901dacd2 in BlockUntilNextEventMatchingListInMode (in HIToolbox) + 61
    #29 0x7fff924d8612 in _DPSNextEvent (in AppKit) + 684
    #30 0x7fff924d7ed1 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] (in AppKit) + 127
    #31 0x106124f15 in -[GeckoNSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] (in XUL) + 245
    #32 0x7fff924cf282 in -[NSApplication run] (in AppKit) + 516
    #33 0x1061272a9 in nsAppShell::Run() (in XUL) + 185
    #34 0x105d028c7 in nsAppStartup::Run() (in XUL) + 311
    #35 0x103bc674f in XREMain::XRE_mainRun() (in XUL) + 4287
    #36 0x103bc7707 in XREMain::XRE_main(int, char**, nsXREAppData const*) (in XUL) + 599
    #37 0x103bc7bb2 in XRE_main (in XUL) + 146
    #38 0x100002d50 in 0x200002d50
    #39 0x1000023fa in 0x2000023fa
    #40 0x100001853 in 0x200001853
    #41 0x0 in 0x0000000100000000 (in firefox-bin)
Stats: 1909M malloced (1465M for red zones) by 2527360 calls
Stats: 260M realloced by 160538 calls
Stats: 1785M freed by 2116352 calls
Stats: 1741M really freed by 2026212 calls
Stats: 703M (180003 full pages) mmaped in 1041 calls
  mmaps   by size class: 7:356265; 8:124867; 9:27621; 10:13797; 11:9180; 12:4480; 13:11648; 14:3200; 15:800; 16:1160; 17:464; 18:148; 19:38; 20:29; 21:5; 22:23; 23:2; 24:4;
  mallocs by size class: 7:1596497; 8:494426; 9:131434; 10:117840; 11:84623; 12:32593; 13:33559; 14:13390; 15:12108; 16:5815; 17:3815; 18:874; 19:165; 20:120; 21:14; 22:80; 23:2; 24:7;
  frees   by size class: 7:1318903; 8:396513; 9:113833; 10:109720; 11:81490; 12:29796; 13:32710; 14:11321; 15:11885; 16:5314; 17:3744; 18:760; 19:155; 20:116; 21:13; 22:73; 23:2; 24:7;
  rfrees  by size class: 7:1258696; 8:381608; 9:109079; 10:106352; 11:78281; 12:28656; 13:31792; 14:10212; 15:11507; 16:5240; 17:3698; 18:726; 19:154; 20:116; 21:13; 22:73; 23:2; 24:7;
Stats: malloc large: 24019 small slow: 41712
==87236== ABORTING

Comment 1

[Daniel Veditz [:dveditz]]

Looks like a null deref which wouldn't be too worrying, but CC'ing Roc who fixed the original bug.

Comment 2

[Andrew McCreight [:mccr8]]

Marking as sec-low because it looks like a null deref. Please re-rate or clear the sec- flag if that's wrong.

Comment 3

[Matthew Gregan [:kinetik]]

The audio thread thread blocks in PlayFromAudioQueue (at mAudioStream->Write).

The state machine thread calls StopAudioThread (triggered by SetAudioCaptured(true) rescheduling the state machine thread), mStopAudioThread is false so it "blocks" (spins a nested event queue) at mAudioThread->Shutdown waiting for the audio thread to complete.

As the nested event queue spins, additional calls are made to StopAudioThread (from CallRunStateMachine  because mAudioCaptured == true, triggered by TimeoutExpired) which bail early due to mStopAudioThread being true.

Still in the nested event queue, Shutdown is called from somewhere, which calls StopAudioThread from RunStateMachine#DECODER_STATE_SHUTDOWN but does not block because mStopAudioThread == true so shutdown continues, resulting in the state machine's mDecoder pointer being set to null.

The audio thread eventually wakes up in PlayFromAudioQueue and calls mDecoder->UpdatePlaybackOffset, crashing due to a null dereference.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

How about having PlayFromAudioQueue take a strong reference to mDecoder while holding the monitor?

One thing I don't understand is that MediaDecoder::UpdatePlaybackRate asserts we're holding the monitor, but it looks like MediaDecoderStateMachine::PlayFromAudioQueue calls it without holding the monitor?

Comment 5

[Matthew Gregan [:kinetik]]

Attachment: patch v0

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #4)
> How about having PlayFromAudioQueue take a strong reference to mDecoder
> while holding the monitor?

That's an option.  The fix I implemented and pushed to try on Monday night (https://tbpl.mozilla.org/?tree=Try&rev=942e57738c8c) takes the approach of deferring RunStateMachine's SHUTDOWN steps until the audio thread has completed.  Perhaps that's too ugly--if you think so I'll try your approach.

> One thing I don't understand is that MediaDecoder::UpdatePlaybackRate
> asserts we're holding the monitor, but it looks like
> MediaDecoderStateMachine::PlayFromAudioQueue calls it without holding the
> monitor?

It's MediaDecoder::UpdatePlaybackOffset, which takes the monitor internally.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 702628 [details] [diff] [review]
patch v0

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

This isn't ugly, it's brilliant! :-)

Comment 7

[Matthew Gregan [:kinetik]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ee9dd4854dfc

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/ee9dd4854dfc