Closed Bug 1426867 Opened 2 years ago Closed 2 years ago

Crash in mozilla::CubebUtils::GetCubebContextUnlocked

Categories

(Core :: Audio/Video: cubeb, defect, P2, critical)

59 Branch
Unspecified
All
defect

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox-esr52 --- unaffected
firefox57 --- unaffected
firefox58 --- unaffected
firefox59 - fixed

People

(Reporter: calixte, Assigned: kinetik)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Keywords: crash, regression, Whiteboard: [clouseau])

Crash Data

Attachments

(2 files, 1 obsolete file)

This bug was filed from the Socorro interface and is
report bp-17e18cfa-9a00-4000-89e2-5a8b50171222.
=============================================================

Top 10 frames of crashing thread:

0 libxul.so mozilla::CubebUtils::GetCubebContextUnlocked dom/media/CubebUtils.cpp:457
1 libxul.so mozilla::CubebUtils::GetCubebContext dom/media/CubebUtils.cpp:294
2 libxul.so mozilla::AudioStream::Init dom/media/AudioStream.cpp:356
3 libxul.so mozilla::media::AudioSink::InitializeAudioStream dom/media/mediasink/AudioSink.cpp:204
4 libxul.so mozilla::media::AudioSink::Init dom/media/mediasink/AudioSink.cpp:95
5 libxul.so mozilla::media::AudioSinkWrapper::Start dom/media/mediasink/AudioSinkWrapper.cpp:196
6 libxul.so mozilla::media::VideoSink::Start dom/media/mediasink/VideoSink.cpp:172
7 libxul.so mozilla::MediaDecoderStateMachine::StartMediaSink dom/media/MediaDecoderStateMachine.cpp:3328
8 libxul.so mozilla::MediaDecoderStateMachine::MaybeStartPlayback dom/media/MediaDecoderStateMachine.cpp:2958
9 libxul.so mozilla::MediaDecoderStateMachine::DecodingState::Step dom/media/MediaDecoderStateMachine.cpp:2339

=============================================================

There are 5 crashes (from the same install) in nightly 59 with buildid 20171222100407. In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1405877.

[1] https://hg.mozilla.org/mozilla-central/rev/502337e7c6ae
Flags: needinfo?(kinetik)
It looks like we're trying to create a cubeb context before the child has completed the pre-init for the audio remoting.  That's unusual, so this should be an uncommon assert to hit.  We can fix it by forcing the cubeb context init to wait for the pre-init to complete, but I'd like to understand what's triggering this first, if possible.

I'll take a look at this when I'm back from PTO (early January).  In the meantime, this can be worked around by setting the pref media.cubeb.sandbox to false.
There are 5 crash results (4 Linux, 1 macOS) for this signature.  All of them are in "content (file)" process types, suggesting this is triggered via file:// playback.  Testing various local media, I can reproduce this reliably by running "mach run dom/media/test/gizmo.mp4" (some other files don't trigger the assert, possibly due to slightly different timing).
Assignee: nobody → kinetik
Status: NEW → ASSIGNED
Flags: needinfo?(kinetik)
I believe I've seen this issue too.  When it happens, I see this message:
Assertion failure: sIPCConnection.IsValid(), at /home/mfroman/mozilla/moz-central/dom/media/CubebUtils.cpp:458

https://dxr.mozilla.org/mozilla-central/source/dom/media/CubebUtils.cpp?q=GetCubebContextUnlocked&redirect_type=direct#458

In particular, I've seen it when using some test html from the fuzzing team on Bug 1425780.  See the trigger.html attachment:
https://bugzilla.mozilla.org/attachment.cgi?id=8937373

When running with basic prefs (not the prefs provided for fuzzing in the bug), I've been able to repro mostly reliably on my linux box.
[Tracking Requested - why for this release]: crash regression on linux and osx
Rank: 12
OS: Linux → All
Priority: -- → P2
It's Nightly-only code.
Attaching for posterity - this is what the earlier try push used, but it's broken due to main thread issues.

This removed the static global file descriptor and waited on the Gecko IPC initialization promise to complete.  This is a nicer solution than what we have now, but requires no GetCubebContext() calls are made from the main thread.  Unfortunately, Web Audio does exactly that (e.g. creating an AudioContext queries the max channel count, which creates a cubeb context).  

Long term, we should ban cubeb calls from the main thread, since they could be expensive and it's pretty dumb to block the main thread on a cubeb context initialization (which could involve initializating multiple audio backends, loading DSOs, connecting over sockets, etc.).  I'll file a separate bug for that.
Comment on attachment 8942796 [details] [diff] [review]
wip: wait for init promise to complete

Obsoleting because we can't use this approach today.
Attachment #8942796 - Attachment is obsolete: true
Attachment #8942797 - Flags: review?(dglastonbury)
For now, we take the dumb and ugly approach and try to win the initialization race by starting even earlier.  This works for all of the cases I found locally that would trigger the assert before.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1f52323c4ef843eaadc49c9a0cf0e11a086a387e
Attachment #8942798 - Flags: review?(dglastonbury)
Depends on: 1430704
Attachment #8942797 - Flags: review?(dglastonbury) → review+
Attachment #8942798 - Flags: review?(dglastonbury) → review+
Pushed by mgregan@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7b41fbd6b35a
Move private functions out of CubebUtils.h.  r=kamidphish
https://hg.mozilla.org/integration/mozilla-inbound/rev/14b5945f84c4
Init AudioIPC even earlier.  r=kamidphish
https://hg.mozilla.org/mozilla-central/rev/7b41fbd6b35a
https://hg.mozilla.org/mozilla-central/rev/14b5945f84c4
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.