Closed Bug 1131768 Opened 10 years ago Closed 10 years ago

crash in _wassert

Categories

(Core :: Audio/Video, defect)

x86
Windows NT
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla38
Tracking Status
firefox35 --- unaffected
firefox36 + verified
firefox37 --- fixed
firefox38 --- fixed

People

(Reporter: away, Assigned: padenot)

References

Details

(Keywords: crash)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is report bp-50112851-3d94-44ab-83d7-5931d2150210. ============================================================= 0 msvcr120.dll _wassert f:\dd\vctools\crt\crtw32\misc\assert.c:369 1 xul.dll `anonymous namespace'::owned_critical_section::enter() media/libcubeb/src/cubeb_wasapi.cpp 2 xul.dll `anonymous namespace'::wasapi_stream_destroy(cubeb_stream*) media/libcubeb/src/cubeb_wasapi.cpp 3 xul.dll `anonymous namespace'::setup_wasapi_stream(cubeb_stream*) media/libcubeb/src/cubeb_wasapi.cpp 4 xul.dll `anonymous namespace'::wasapi_stream_init(cubeb*, cubeb_stream**, char const*, cubeb_stream_params, unsigned int, long (*)(cubeb_stream*, void*, void*, long), void (*)(cubeb_stream*, void*, cubeb_state), void*) media/libcubeb/src/cubeb_wasapi.cpp 5 xul.dll cubeb_stream_init media/libcubeb/src/cubeb.c 6 xul.dll mozilla::AudioStream::DataCallback(void*, long) dom/media/AudioStream.cpp 7 xul.dll mozilla::AudioStream::Init(int, int, mozilla::dom::AudioChannel, mozilla::AudioStream::LatencyRequest) dom/media/AudioStream.cpp 8 xul.dll mozilla::AudioSink::InitializeAudioStream() dom/media/AudioSink.cpp 9 xul.dll mozilla::AudioSink::AudioLoop() dom/media/AudioSink.cpp 10 nss3.dll _MD_CURRENT_THREAD nsprpub/pr/src/md/windows/w95thred.c 11 xul.dll nsRunnableMethodImpl<void ( mozilla::crashreporter::LSPAnnotationGatherer::*)(void), void, 1>::Run() xpcom/glue/nsThreadUtils.h 12 xul.dll nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp 13 xul.dll mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp 14 xul.dll MessageLoop::RunHandler() ipc/chromium/src/base/message_loop.cc
[Tracking Requested - why for this release]: This is a top crash in very early 36b8 data (5 crashes out of 114). Normally that little data is too noisy, but given that it's a new regression with little time left, this is worth keeping an eye on.
The assert is: assert(owner != GetCurrentThreadId() && "recursive locking"); For one thing I am puzzled that this is active in a release build. The |assert| family is disabled by the NDEBUG symbol which we do define. Here is the about:buildconfig from 36b8: Compiler Version Compiler flags cl 1800 -TC -nologo -D_HAS_EXCEPTIONS=0 -W3 -Gy -arch:IA32 -FS -wd4244 -wd4267 -wd4819 -we4553 cl 1800 -TP -nologo -D_HAS_EXCEPTIONS=0 -W3 -Gy -arch:IA32 -FS -wd4251 -wd4244 -wd4267 -wd4345 -wd4351 -wd4482 -wd4800 -wd4819 -we4553 -GR- -DNDEBUG -DTRIMMED -Zi -UDEBUG -DNDEBUG -GL -wd4624 -wd4952 -O1 -Oi -Oy (Why do we have two different lines there?) And does cubeb do its own custom flags or anything? In any case it still sounds like a bug that we mismatched threads, regardless of whether the assert fired.
Flags: needinfo?(padenot)
Flags: needinfo?(mh+mozilla)
Flags: needinfo?(kinetik)
Waitasec, this code is behind #ifdef DEBUG. What the?
Oh I see... 1.10 +// This enables assert in release, and lets us have debug-only code 1.11 +#ifdef NDEBUG 1.12 +#define DEBUG 1.13 #undef NDEBUG 1.14 +#endif // #ifdef NDEBUG Ok, one mystery solved, but still that's kind of scary, especially wrt unified builds.
Flags: needinfo?(mh+mozilla)
Ok so this seems to be: wasapi_stream_init: auto_lock lock(stm->stream_reset_lock); rv = setup_wasapi_stream(stm); Then setup_wasapi_stream: if (!stm->resampler) { LOG("Could not get a resampler"); wasapi_stream_destroy(stm); return CUBEB_ERROR; } Then wasapi_stream_destroy: { auto_lock lock(stm->stream_reset_lock); <-- locks again close_wasapi_stream(stm); }
Assignee: nobody → padenot
Flags: needinfo?(padenot)
> Normally that little data is too noisy With more data coming in this is up to the #1 spot at 12% of b8 crashes. Looks like it's real.
Untested, because I don't have a windows builder handy. I'm quite confused as why the resampler init fails, and I'll look at that again tomorrow morning first thing. Maybe there is a sound file out there that has a very very low intrinsic sample-rate, and people are using high sample-rate output devices, so we end up allocating a ton of memory, but given that :dmajor told us there is quite a lot of memory available (2.8GB on the crash report linked), I'm not sure. In any case, we can also backout 20f7d44346da, 0411d20465b4, and 9579b9ab68ca.
Attachment #8562405 - Flags: review?(kinetik)
Attachment #8562405 - Flags: review?(kinetik) → review+
Flags: needinfo?(kinetik)
Comment on attachment 8562405 [details] [diff] [review] Unlock before tearing down the stream in case of error on setup, to avoid reccursive locking. r= Approval Request Comment [Feature/regressing bug #]: bug 1127213 [User impact if declined]: crash in OOM/audio device error situations [Describe test coverage new/current, TreeHerder]: n/a [Risks and why]: low, only change is dropping lock on error path [String/UUID change made/needed]: none
Attachment #8562405 - Flags: approval-mozilla-beta?
Attachment #8562405 - Flags: approval-mozilla-aurora?
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Attachment #8562405 - Flags: approval-mozilla-beta?
Attachment #8562405 - Flags: approval-mozilla-beta+
Attachment #8562405 - Flags: approval-mozilla-aurora?
Attachment #8562405 - Flags: approval-mozilla-aurora+
I was seeing this frequently immediately after seeking on https://www.youtube.com/watch?v=9aMYvNCXA_I (with or without MSE, 480 or 720 lines). Selecting "Ignore" in the assertion failure dialog would leave the video playing with no audio until the next successful seek.
Looks to me like https://crash-stats.mozilla.com/report/list?signature=wassert is probably the Win64 equivalent of this.
Crash Signature: [@ _wassert] → [@ _wassert] [@ wassert]
Flags: qe-verify+
Depends on: 1133190
Socorro shows no crashes recorded in Firefox 36 beta with '@ _wassert' signature after the fix, marking this as verified fixed. https://crash-stats.mozilla.com/search/?signature=~_wassert&_facets=signature&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform#crash-reports Although there are still crashes recorded on latest Nightly with '@ wassert' signature. https://crash-stats.mozilla.com/signature/?signature=wassert&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=reason&_columns=address&page=1 Is this something that we should worry about?
Flags: needinfo?(padenot)
It should be fine for now. In any case, those '@ wassert' crashes don't look super actionable from here.
Flags: needinfo?(padenot)
Flags: qe-verify+
FWIW many of those remaining wassert's will change signatures after bug 1133386.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: