1340278 - Crash in abort | `anonymous namespace''::wasapi_stream_start

Status: RESOLVED DUPLICATE of bug 1347944

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

Description

[[:philipp]]

This bug was filed from the Socorro interface and is 
report bp-93b2ea16-122c-4734-ba18-4ec982170216.
=============================================================
Crashing Thread (97)
Frame 	Module 	Signature 	Source
0 	ucrtbase.dll 	abort 	
1 	xul.dll 	`anonymous namespace'::wasapi_stream_start 	media/libcubeb/src/cubeb_wasapi.cpp:1874
2 	xul.dll 	cubeb_stream_start 	media/libcubeb/src/cubeb.c:281
3 	xul.dll 	mozilla::AudioStream::InvokeCubeb<int (*)(cubeb_stream*)>(int (*)(cubeb_stream*)) 	dom/media/AudioStream.cpp:317
4 	xul.dll 	mozilla::AudioStream::Resume() 	dom/media/AudioStream.cpp:449
5 	xul.dll 	mozilla::media::DecodedAudioDataSink::SetPlaying(bool) 	dom/media/mediasink/DecodedAudioDataSink.cpp:189
6 	xul.dll 	mozilla::media::AudioSinkWrapper::SetPlaying(bool) 	dom/media/mediasink/AudioSinkWrapper.cpp:164
7 	xul.dll 	mozilla::media::VideoSink::SetPlaying(bool) 	dom/media/mediasink/VideoSink.cpp:157
8 	xul.dll 	mozilla::MediaDecoderStateMachine::MaybeStartPlayback() 	dom/media/MediaDecoderStateMachine.cpp:2270
9 	xul.dll 	mozilla::MediaDecoderStateMachine::DecodingState::Step() 	dom/media/MediaDecoderStateMachine.cpp:567
10 	xul.dll 	mozilla::detail::RunnableMethodImpl<void ( nsBindingManager::*)(void), 1, 0>::Run() 	obj-firefox/dist/include/nsThreadUtils.h:810
11 	xul.dll 	mozilla::AutoTaskDispatcher::TaskGroupRunnable::Run() 	obj-firefox/dist/include/mozilla/TaskDispatcher.h:193
12 	xul.dll 	mozilla::TaskQueue::Runner::Run() 	xpcom/threads/TaskQueue.cpp:232
13 	xul.dll 	nsThreadPool::Run() 	xpcom/threads/nsThreadPool.cpp:226
14 	xul.dll 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp:1216
15 	xul.dll 	NS_ProcessNextEvent(nsIThread*, bool) 	xpcom/glue/nsThreadUtils.cpp:361
16 	xul.dll 	mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp:368
17 	xul.dll 	MessageLoop::RunHandler() 	ipc/chromium/src/base/message_loop.cc:225
18 	xul.dll 	MessageLoop::Run() 	ipc/chromium/src/base/message_loop.cc:205
19 	xul.dll 	nsThread::ThreadFunc(void*) 	xpcom/threads/nsThread.cpp:467
20 	nss3.dll 	_PR_NativeRunThread 	nsprpub/pr/src/threads/combined/pruthr.c:397
21 	nss3.dll 	pr_root 	nsprpub/pr/src/md/windows/w95thred.c:95
22 	ucrtbase.dll 	_o__CIpow 	
23 	kernel32.dll 	BaseThreadInitThunk 	
24 	ntdll.dll 	__RtlUserThreadStart 	
25 	ntdll.dll 	_RtlUserThreadStart

this crash signature on windows started regressing in 52.0b4 (where crashes from bug 1326176 ceased on the other hand), it's around 0.2% of browser crashes on 52.0b6 atm

https://hg.mozilla.org/releases/mozilla-beta/pushloghtml?fromchange=FIREFOX_52_0b3_RELEASE&tochange=FIREFOX_52_0b4_RELEASE

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

padenot, achronop: please check the regression-range for this, and see what landed to cause it.

Comment 2

[Paul Adenot (:padenot)]

https://github.com/kinetiknz/cubeb/pull/237, fix being reviewed upstream.

Comment 3

[Alex Chronopoulos [:achronop]]

Clear NI based on previous comment.

Comment 4

[Astley Chen (inactive)]

Bug 1342363 was fixed in FF54 now, we are waiting for uplift to beta 52 and verify the fix.

Comment 5

[[:philipp]]

there still was a report with the same signature, that happened after the uplift to 53 aurora: https://crash-stats.mozilla.com/report/index/914b4c7c-18e2-4e80-aabb-816ff2170304

Comment 6

[Paul Adenot (:padenot)]

This is not the same crash, it's crashing on the other assert. This means that the user of cubeb is calling `cubeb_stream_start` twice in a row. We should simply return here.

The broader issue is that there is (more-or-less) no clear API contract for this. That said, we should make it idempotent and document it. Matthew, does that sound good ?

Comment 7

[Alex Chronopoulos [:achronop]]

+1 for having a clean contract about every ambiguous case in cubeb. We had a related discussion in Hawaii.

Comment 8

[Paul Adenot (:padenot)]

JW, does comment 6 make sense to you ? Is that something that is possible in the current setup ? This is on beta.

Comment 9

[Paul Adenot (:padenot)]

Hrm maybe it doesn't, there are clear asserts at the call-site (debug-only though, but I'd have a hard time believing that this is an non-debug-only crash). I'll have a closer look tomorrow.

Comment 10

[Matthew Gregan [:kinetik]]

(In reply to Paul Adenot (:padenot) from comment #6)
> This is not the same crash, it's crashing on the other assert. This means
> that the user of cubeb is calling `cubeb_stream_start` twice in a row. We
> should simply return here.
> 
> The broader issue is that there is (more-or-less) no clear API contract for
> this. That said, we should make it idempotent and document it. Matthew, does
> that sound good ?

Documenting, and more importantly, adding tests for this stuff is good.  The original intention was for repeated calls to be an error, which is why these asserts are present (to catch caller API misuse).  I can't think of a good argument to allow this sort of misuse, it just hides/moves bugs around.

Comment 11

[JW Wang [:jwwang] [:jw_wang]]

(In reply to Paul Adenot (:padenot) from comment #8)
> JW, does comment 6 make sense to you ? Is that something that is possible in
> the current setup ? This is on beta.

Do you mean https://crash-stats.mozilla.com/report/index/914b4c7c-18e2-4e80-aabb-816ff2170304 or bp-93b2ea16-122c-4734-ba18-4ec982170216? Btw, 53 becomes beta a few hours ago...

Comment 12

[Astley Chen (inactive)]

(In reply to Paul Adenot (:padenot) from comment #6)
> This is not the same crash, it's crashing on the other assert. This means
> that the user of cubeb is calling `cubeb_stream_start` twice in a row. We
> should simply return here.

Could we open a new bug for tracking and probably with a need to uplift to beta 43 ?

Comment 13

[Ryan VanderMeulen [:RyanVM]]

IIUC, this was fixed on the release branches by bug 1347944.

Comment 14

[Emma Humphries ☕️🎸🧞‍♀️✨ (she/they) [:emceeaich] (Pacific Time) use needinfo]

This is a P1 bug without an assignee. 

P1 are bugs which are being worked on for the current release cycle/iteration/sprint. 

If the bug is not assigned by Monday, 28 August, the bug's priority will be reset to '--'.

Comment 15

[Bulk Bug Changes for mreavy's org]

Mass change P1->P2 to align with new Mozilla triage process