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

RESOLVED FIXED in Firefox 41

Status

()

Core
Audio/Video: MediaStreamGraph
--
critical
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: dmajor (offline), Assigned: kinetik)

Tracking

({crash})

41 Branch
mozilla43
x86
Windows NT
crash
Points:
---

Firefox Tracking Flags

(firefox41+ fixed, firefox42 fixed, firefox43 fixed)

Details

(crash signature)

Attachments

(2 attachments)

(Reporter)

Description

2 years ago
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
Flags: needinfo?(kinetik)
(Reporter)

Comment 1

2 years ago
[Tracking Requested - why for this release]: Regression from a bug that was just uplifted to 41 beta.
status-firefox41: --- → affected
status-firefox42: --- → affected
status-firefox43: --- → affected
tracking-firefox41: --- → ?
(Assignee)

Updated

2 years ago
Assignee: nobody → kinetik
Status: NEW → ASSIGNED
(Assignee)

Comment 2

2 years ago
Created attachment 8654452 [details] [diff] [review]
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.
Flags: needinfo?(kinetik)
Attachment #8654452 - Flags: review?(padenot)
(Assignee)

Comment 3

2 years ago
Created attachment 8654453 [details] [diff] [review]
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
Attachment #8654453 - Flags: approval-mozilla-beta?
Attachment #8654453 - Flags: approval-mozilla-aurora?

Comment 4

2 years ago
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+
Attachment #8654453 - Flags: approval-mozilla-beta?
Attachment #8654453 - Flags: approval-mozilla-beta+
Attachment #8654453 - Flags: approval-mozilla-aurora?
Attachment #8654453 - Flags: approval-mozilla-aurora+

Comment 5

2 years ago
Matthew, Wes: I just noticed that the patch has not been r+'d yet. Please uplift only after the patch has been reviewed. Thanks.
Flags: needinfo?(wkocher)
Flags: needinfo?(kinetik)

Updated

2 years ago
tracking-firefox41: ? → +

Updated

2 years ago
Flags: needinfo?(wkocher)

Updated

2 years ago
Attachment #8654452 - Flags: review?(padenot) → review+
(Assignee)

Comment 9

2 years ago
Thanks Ryan!
Flags: needinfo?(kinetik)
https://hg.mozilla.org/mozilla-central/rev/47fa2df291d4
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox43: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in before you can comment on or make changes to this bug.