1262345 - crash in RtlpWaitOnCriticalSection | RtlpEnterCriticalSectionContended | audioses.dll@0xccee

Status: RESOLVED FIXED

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

Description

[Nicholas Nethercote [inactive]]

This bug was filed from the Socorro interface and is 
report bp-2f1d0581-5e26-4eee-9738-bb5152160405.
=============================================================

This crash has occurred 3 times in the latest Nightly (20160405030214), making it the #6 top crasher.

It ends up in system DLLs. The highest Mozilla frame on the stack is https://hg.mozilla.org/mozilla-central/annotate/d9f50aa0a1aa/dom/media/AudioStream.cpp#l691, which is at the very end of a function, which suggests a destructor is running? Not sure.

Comment 1

[Nicholas Nethercote [inactive]]

Within the past 7 days there have been 52 crashes matching this crash signature, and they're all in Nightly 48.0a1, which suggests this is a recent regression.

https://crash-stats.mozilla.com/report/list?range_unit=days&range_value=7&signature=RtlpWaitOnCriticalSection+%7C+RtlpEnterCriticalSectionContended+%7C+audioses.dll%400xccee#tab-reports

It might be related to bug 1251502.

Comment 2

[Nicholas Nethercote [inactive]]

Happened another 25 times in Nightly 20160407030234, and in smaller numbers in Nightly builds with other build ids.

kinetik, any idea what might be the problem?

Comment 3

[Matthew Gregan [:kinetik]]

Most likely a regression from bug 1251502, which landed a large number of changes in all of the major audio backends to add full duplex stream support.

From a quick look at the crashing location, the client pointer is almost certainly invalid because we close and reopen the IAudioClient at stm->{input,output}_client when the first client->Start() returns AUDCLNT_E_DEVICE_INVALIDATED, but we don't grab the updated client pointer from stm->{input,output}_client when trying to start the client after the close/reopen dance.

Not sure when Paul's free to look at this, so I'll add it to my queue and try to look at it soon.

Comment 4

[Paul Adenot (:padenot)]

Matthew, I'm back, I can have a look.

Comment 5

[Matthew Gregan [:kinetik]]

Attachment: bug1262345_v0.patch

Simple fix - avoid caching the client pointer in a local variable.

This also reintroduces a NULL test for output_client that was added in 2533a1f3 but changed to a has_output(stm) test in the full-duplex rework.

While testing this I found another new NULL deref in get_output_buffer where stm->output_client is NULL due to the only audio device being disconnected.  I haven't worked out why we crash here yet - the pre-full-duplex code didn't handle a NULL output_client here either, so the behavioural difference is non-local (or the crash I found isn't new).

Comment 6

[Matthew Gregan [:kinetik]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=23514250b9a1

Comment 7

[Matthew Gregan [:kinetik]]

Fix was incomplete, so final fix is in bug 1263505 which landed in bug 1265692.

Comment 8

[Matthew Gregan [:kinetik]]

This signature is not showing up in recent crash stats; calling this fixed by bug 1263505.