Closed Bug 1585843 Opened 2 months ago Closed 2 months ago

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

Categories

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

Unspecified
Windows 10
defect

Tracking

()

RESOLVED FIXED
mozilla71
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- unaffected
firefox67 --- unaffected
firefox68 --- unaffected
firefox69 --- unaffected
firefox70 --- unaffected
firefox71 --- fixed

People

(Reporter: marcia, Assigned: kinetik)

References

(Regression)

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

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

Flags: needinfo?(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.

Assignee: nobody → kinetik
Status: NEW → ASSIGNED
Flags: needinfo?(kinetik)
Priority: -- → P2
Regressed by: 1544997

(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.

Pushed by mgregan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3418a8c2baa6
Don't try to reject disconnected promise in AudioSink::Errored.  r=padenot
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71

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

Flags: needinfo?(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.

Flags: needinfo?(kinetik)

(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!

You need to log in before you can comment on or make changes to this bug.