1199794 - crash in `anonymous namespace''::current_stream_delay(cubeb_stream*)

Status: RESOLVED FIXED

(Core :: Audio/Video: MediaStreamGraph, defect)

Description

[(Away)]

This bug was filed from the Socorro interface and is 
report bp-0f237bc4-e324-47fa-bb79-a2f632150826.
=============================================================

Not a huge topcrash but it did catch my eye as a recent regression.

stm->audio_clock is null in current_stream_delay.

0 	xul.dll 	`anonymous namespace'::current_stream_delay(cubeb_stream*) 	media/libcubeb/src/cubeb_wasapi.cpp
1 	xul.dll 	`anonymous namespace'::wasapi_stream_get_position(cubeb_stream*, unsigned __int64*) 	media/libcubeb/src/cubeb_wasapi.cpp
2 	xul.dll 	mozilla::AudioStream::GetPositionInFramesUnlocked() 	dom/media/AudioStream.cpp
3 	xul.dll 	mozilla::AudioClock::GetPositionUnlocked() 	dom/media/AudioStream.cpp
4 	xul.dll 	mozilla::AudioStream::GetPosition() 	dom/media/AudioStream.cpp
5 	xul.dll 	mozilla::media::DecodedAudioDataSink::GetPosition() 	dom/media/mediasink/DecodedAudioDataSink.cpp
6 	xul.dll 	mozilla::MediaDecoderStateMachine::GetClock(mozilla::TimeStamp*) 	dom/media/MediaDecoderStateMachine.cpp
7 	xul.dll 	mozilla::MediaDecoderStateMachine::UpdateRenderedVideoFrames() 	dom/media/MediaDecoderStateMachine.cpp
8 	xul.dll 	mozilla::MediaDecoderStateMachine::RunStateMachine() 	dom/media/MediaDecoderStateMachine.cpp
9 	xul.dll 	nsRunnableMethodImpl<void ( mozilla::dom::ServiceWorkerRegistrar::*)(void), 1>::Run() 	xpcom/glue/nsThreadUtils.h
10 	xul.dll 	mozilla::AutoTaskDispatcher::TaskGroupRunnable::Run() 	xpcom/threads/TaskDispatcher.h
11 	xul.dll 	mozilla::TaskQueue::Runner::Run() 	xpcom/threads/TaskQueue.cpp
12 	xul.dll 	nsThreadPool::Run() 	xpcom/threads/nsThreadPool.cpp

Comment 1

[(Away)]

[Tracking Requested - why for this release]: Regression from a bug that was just uplifted to 41 beta.

Comment 2

[Matthew Gregan [:kinetik]]

Attachment: bug1199794_v0.patch

Thanks for raising this Dave!

The fix is to add a simple NULL check.  It took me a little while to find a plausible case where this can happen, but here's what I understand (and reproduced locally):

- Playback is running normally
- We process a device change; closing the existing device but failing to open a new device (e.g. if there's only one device and it is disabled/disconnected)
- Playback engine calls stream_get_position while the error callback is propagating back from the audio render thread

It looks like we could hit the same type of failure in stream_start with stm->client and stream_set_volume with stm->audio_stream_volume but these are both called with such low frequency relative to stream_get_position that it's extremely unlikely.

Comment 3

[Matthew Gregan [:kinetik]]

Attachment: bug1199794_v0_uplift.patch

This is the same fix as the previous patch, but rebased for aurora and beta (which doesn't have the fix for bug 1197049).

Approval Request Comment
[Feature/regressing bug #]: bug 1136360
[User impact if declined]: crash during media playback if audio device removed
[Describe test coverage new/current, TreeHerder]: n/a
[Risks and why]: extremely low, adds simple NULL checks
[String/UUID change made/needed]: none

Comment 4

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8654453 [details] [diff] [review]
bug1199794_v0_uplift.patch

Seems like a safe patch to uplift as as it is a null check. Aurora42+, Beta41+

Comment 5

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Matthew, Wes: I just noticed that the patch has not been r+'d yet. Please uplift only after the patch has been reviewed. Thanks.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/e2c3777c5e82

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/47fa2df291d4

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/e2c3777c5e82

Comment 9

[Matthew Gregan [:kinetik]]

Thanks Ryan!

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/47fa2df291d4

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/440d55ad4feb