User Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:37.0) Gecko/20100101 Firefox/37.0 Build ID: 20150128004009 Steps to reproduce: Changed the default audio device in the playback devices tab in sound options. Actual results: Either it crashes right away after change OR it allows the change to happen but after a few times of changing the default audio device, it crashes the browser entirely. Expected results: Switch the current audio to the new default audio device.

Possibly a regression from bug 698079.

On 37.0a2 (2015-01-28), I can't repro after swapping back-and-forth about 10 times while the first track on http://www.html5tutorial.info/html5-audio.php was playing. The audio switched devices as the default was changed.

(In reply to Mark Hammond [:markh] from comment #2) > On 37.0a2 (2015-01-28), I can't repro after swapping back-and-forth about 10 > times while the first track on http://www.html5tutorial.info/html5-audio.php > was playing. The audio switched devices as the default was changed. I am on the same build and it just started to happen on my laptop. For my desktop however it's been doing this since the fix from bug 698079.

Can you share some crash IDs from about:crashes? (assuming you have the crash reporter enabled)

(In reply to David Major [:dmajor] (UTC+13) from comment #4) > Can you share some crash IDs from about:crashes? (assuming you have the > crash reporter enabled) I am also getting the same issue. I am on 38.0a1 (2015-01-31). Win 7 x64 bp-7482ed0f-92c6-471c-8043-898802150201

The crash is during the |delete| of a cubeb_resampler_speex. At first glace this is either an allocator mismatch or a bad/uninitialized resampler pointer. Any ideas? bp-6c17a7eb-561d-46c2-834e-4ac5f2150128 may be related; that one looks like an uninitialized critical section.

[Tracking Requested - why for this release]: Given the uplift haste in bug 698079 this sounds like it's worth tracking for beta.

Assuming that this is indeed a regression from bug 698079 and MSE related, I take it that 36+ are affected. Tracking for 36+.

I've found and fixed some issues with the code today (including this crash and bug 1128371), and added some hardening (threading asserts mostly) to avoid both crashes and clock drifts. I'll attach the patch tomorrow, it needs some cleanup.

I have 3 audio out devices and this crash happens only for one of them. It seems to do with Equalizer APO, which is an EQ that uses Windows' Audio Processing Object API. The device with the crash is the only one with APO processing. Switching to this device while audio is playing crashes Firefox every time. If I disable APO processing using the "Enable audio enhancements" option in the device's properties, Firefox no longer crashes when switching to it while audio is playing, but audio only comes out of the left channel when it should be stereo. If APO processing is enabled ("Enable audio enhancements" ticked) and I select the troublesome audio device before playing audio, Firefox doesn't crash, but the audio stream immediately stops after pressing play. The other devices which work correctly do have built-in effects enabled in the "Enhancements" tab. None of the myriad other apps on my system have any issue with this device and work correctly with all of my audio devices (and switching between them on the fly), so the problem isn't with Equalizer APO itself.

Can you please share some of the crash IDs from about:crashes?

(In reply to Mark Hammond [:markh] from comment #13) > Can you please share some of the crash IDs from about:crashes? bp-1d6a417d-9063-49ff-874d-a81b02150204 bp-dcbbb068-5549-4f5a-8c77-860ac2150204 bp-7466012f-759b-4788-b905-4d11a2150204 bp-42769e36-cf0e-43a1-8f4a-15b8f2150204

This patch does the following: - Introduces an owned_critical_section object to be able to assert that a current thread owns a critical section - Change the auto_lock to use the above, add auto_unlock (basically like the ecko AutoEnter/AutoExit things) - Fix an issue during audio output device switch where the clock would use the old sample rate. Apparently I did not notice this because my headset and the sound card on this laptop have the same rate - Check that we could acquire a device_enumerator in the ctor before deallocating in the dtor, as that can happen if a ton of streams are running at once (I had this issue running the full mochitest suite) - Stop getting another device_enumator in unregister_notification_client, fixing a leak - Assert that setup_wasapi_stream and close_wasapi_stream are called with the lock held, this was the cause of the crash for this bug - Make close_wasapi_stream clear out its state to make sure setup_wasapi_stream and close_wasapi_stream are called in the right order (especially, not two setup_wasapi_stream without close in between, since that would leak stuff) - in wasapi_stream_destroy, unregister the notification client before destroying the CRITICAL_SECTION (this was the cause of a crash I duped against this bug)

Comment on attachment 8559176 [details] [diff] [review] patch Review of attachment 8559176 [details] [diff] [review]: ----------------------------------------------------------------- ::: media/libcubeb/src/cubeb_wasapi.cpp @@ +30,5 @@ > #ifndef STACK_SIZE_PARAM_IS_A_RESERVATION > #define STACK_SIZE_PARAM_IS_A_RESERVATION 0x00010000 // Threads only > #endif > > +#define LOGGING_ENABLED I'll remove this.

Adding the crash signature for tracking.

(In reply to hararghost@hotmail.com from comment #14) > (In reply to Mark Hammond [:markh] from comment #13) > > Can you please share some of the crash IDs from about:crashes? > > bp-1d6a417d-9063-49ff-874d-a81b02150204 > bp-dcbbb068-5549-4f5a-8c77-860ac2150204 > bp-7466012f-759b-4788-b905-4d11a2150204 > bp-42769e36-cf0e-43a1-8f4a-15b8f2150204 Awesome, thanks! These have the same crash signature, so should also be resolved in this bug.

Comment on attachment 8559176 [details] [diff] [review] patch Review of attachment 8559176 [details] [diff] [review]: ----------------------------------------------------------------- r+ with comments addressed ::: media/libcubeb/src/cubeb_wasapi.cpp @@ +1085,5 @@ > return CUBEB_ERROR; > } > > + { > + // Locking here is not stricly necessary, because we don't have a typo @@ +1116,2 @@ > > + if (stm->client) { SafeRelease already tests for nullptr. @@ +1121,2 @@ > > + if (stm->render_client) { Ditto. @@ +1128,5 @@ > + cubeb_resampler_destroy(stm->resampler); > + stm->resampler = nullptr; > + } > + > + if (stm->mix_buffer) { free(nullptr) is valid, so this is unnecessary.

Comment on attachment 8559176 [details] [diff] [review] patch Approval Request Comment [Feature/regressing bug #]: 698079 [User impact if declined]: crash [Describe test coverage new/current, TreeHerder]: [Risks and why]: this adds some hardening features, and is needed for MSE I believe [String/UUID change made/needed]: none

had to backout this too for another bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=6408280&repo=mozilla-inbound

Relanded, I confused assert() and MOZ_ASSERT(). https://hg.mozilla.org/integration/mozilla-inbound/rev/ad9e5849856d

Comment on attachment 8559176 [details] [diff] [review] patch Taking it to make sure it is in beta 8

(In reply to Paul Adenot from comment #15) > - in wasapi_stream_destroy, unregister the notification client This is probably the cause of the crash that my local build (which predates this patch) just got when switching audio devices from Remote Desktop to whatever the built-in audio is on my PC. Here's the stack annotated with relevant nearby source lines: xul!cubeb_resampler_destroy+0x11 delete resampler; xul!`anonymous namespace'::close_wasapi_stream+0x52 cubeb_resampler_destroy(stm->resampler); xul!`anonymous namespace'::wasapi_stream_destroy+0x88 unregister_notification_client(stm); xul!`anonymous namespace'::setup_wasapi_stream+0x1e3 if (!stm->resampler) { LOG("Could not get a resampler"); wasapi_stream_destroy(stm); xul!wasapi_endpoint_notification_client::OnDefaultDeviceChanged+0x68 MMDevApi!CDeviceEnumerator::OnDefaultDeviceChanged+0x8f MMDevApi!CDeviceEnumerator::OnMainEnumeratorNotification+0x81 MMDevApi!CDeviceEnumerator::OnMediaNotification+0xd5 MMDevApi!CMediaNotifications::OnMediaNotificationWorkerHandler+0x2e2 ntdll!TppSimplepExecuteCallback+0x91 ntdll!TppWorkerThread+0x5ff kernel32!BaseThreadInitThunk+0xd ntdll!RtlUserThreadStart+0x1d Note that at the 4th frame, resampler is null, but at the top frame, it turns out that resampler is pointing at deleted memory.

https://hg.mozilla.org/mozilla-central/rev/2f46afa97421 Is causing a regression where the sound subsystem cuts out under heavy load. See Bug 1134977