1585843 - Crash in [@ mozilla::MozPromise<T>::Private::Reject<T>]

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P2)

Description

[Marcia Knous [:marcia]]

This bug is for crash report bp-f834f364-d044-4085-bc72-2319f0191002.

Seen while looking at the nightly crash spike report - crashes started in 20191002033852: https://bit.ly/2n7QVec. All have EXCEPTION_ACCESS_VIOLATION_READ as the Moz Crash reason. 6 unique crashes so far.

Possible regression range based on build id: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=345d89faf40e4fbe6bceb0d4b4f57ae222a2978d&tochange=cb9bbf38fa45ad65e3016495455e216f69f57ba0

Bug 1544997 landed in that timeframe and there is some AudioSink in the stack. ni on :kinetik

Top 10 frames of crashing thread:

0 xul.dll void mozilla::MozPromise<bool, nsresult, 0>::Private::Reject<const nsresult&> xpcom/threads/MozPromise.h:1083
1 xul.dll void mozilla::AudioSink::Errored dom/media/mediasink/AudioSink.cpp:307
2 xul.dll void mozilla::AudioStream::StateCallback dom/media/AudioStream.cpp:607
3 xul.dll static int `anonymous namespace'::wasapi_stream_stop media/libcubeb/src/cubeb_wasapi.cpp:2521
4 xul.dll mozilla::AudioStream::Shutdown dom/media/AudioStream.cpp:399
5 xul.dll void mozilla::AudioSink::Shutdown dom/media/mediasink/AudioSink.cpp:133
6 xul.dll mozilla::AudioSinkWrapper::Stop dom/media/mediasink/AudioSinkWrapper.cpp:199
7 xul.dll void mozilla::VideoSink::Stop dom/media/mediasink/VideoSink.cpp:328
8 xul.dll void mozilla::MediaDecoderStateMachine::StopMediaSink dom/media/MediaDecoderStateMachine.cpp:3056
9 xul.dll class RefPtr<mozilla::MozPromise<bool, bool, 0> > mozilla::MediaDecoderStateMachine::ShutdownState::Enter dom/media/MediaDecoderStateMachine.cpp:2577

Comment 1

[Matthew Gregan [:kinetik]]

AudioSink::Errored() is new in bug 1544997. In this case, we've already disconnected the promise at https://hg.mozilla.org/mozilla-central/annotate/cb9bbf38fa45ad65e3016495455e216f69f57ba0/dom/media/mediasink/AudioSinkWrapper.cpp#l198, then called AudioSink::Shutdown(), which eventually triggered a cubeb state_callback(CUBEB_STATE_ERRORED) from wasapi_stream_stop.

Simple fix: switch AudioSink::Errored() to use RejectIfExists instead.

The curious thing here is that we're hitting the "failed to stop thread, use emergency bailout" path in cubeb_wasapi.cpp's stop_and_join_render_thread, which we only expect to hit in very unusual circumstances.

Comment 2

[Matthew Gregan [:kinetik]]

(In reply to Matthew Gregan [:kinetik] from comment #1)

The curious thing here is that we're hitting the "failed to stop thread, use emergency bailout" path in cubeb_wasapi.cpp's stop_and_join_render_thread, which we only expect to hit in very unusual circumstances.

Bug 1585848 for investigating that.

Comment 3

[Matthew Gregan [:kinetik]]

Attachment: Bug 1585843 - Don't try to reject disconnected promise in AudioSink::Errored. r?padenot

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3418a8c2baa6
Don't try to reject disconnected promise in AudioSink::Errored.  r=padenot

Comment 5

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/3418a8c2baa6

Comment 6

[Anca Soncutean, Desktop QA]

Is there anything here that could be covered by the manual QA, some steps maybe ? Thank you!

Comment 7

[Matthew Gregan [:kinetik]]

I'd expect the condition that causes this crash to be quite rare. I manually verified the fix, but I'm not sure if the process I used is suitable as a QA process: with a video playing it's possible to simulate the crash conditions by suspending the thread running wasapi_stream_render_loop using Process Explorer, seeking the video, waiting just over 5 seconds then resuming the suspended thread. Identifying that thread may be tricky - it should be possible to use MOZ_LOG=cubeb:5 logging to find the correct thread ID to suspend. On my development machine I used the debugger and symbols to find the correct thread ID.

Comment 8

[Anca Soncutean, Desktop QA]

(In reply to Matthew Gregan [:kinetik] from comment #7)

I'd expect the condition that causes this crash to be quite rare. I manually verified the fix, but I'm not sure if the process I used is suitable as a QA process: with a video playing it's possible to simulate the crash conditions by suspending the thread running wasapi_stream_render_loop using Process Explorer, seeking the video, waiting just over 5 seconds then resuming the suspended thread. Identifying that thread may be tricky - it should be possible to use MOZ_LOG=cubeb:5 logging to find the correct thread ID to suspend. On my development machine I used the debugger and symbols to find the correct thread ID.

Thank you for your time, taking in consideration the above mention details, I think it is safe to not set the qe+ as part of 71 Beta bug verification!