Code in AudioSession::OnSessionDisconnected needs to run asynchronously

RESOLVED FIXED in mozilla12

Status

()

Core
Widget: Win32
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: khuey, Assigned: khuey)

Tracking

unspecified
mozilla12
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

http://msdn.microsoft.com/en-us/library/windows/desktop/dd368289%28v=VS.85%29.aspx

>In implementing the IAudioSessionEvents interface, the client should observe these rules to avoid deadlocks and undefined behavior:
>
>    The methods in the interface must be nonblocking. The client should never wait on a synchronization object during an event callback.
>    The client should never call the IAudioSessionControl::UnregisterAudioSessionNotification method during an event callback.
>    The client should never release the final reference on a WASAPI object during an event callback.

I hit a deadlock at shutdown today that looked like it might be a result of us doing this.

Patch coming up.
Created attachment 584764 [details] [diff] [review]
Patch
Attachment #584764 - Flags: review?(ehsan)
Technically NS_DispatchToMainThread takes a synchronization object ... but I think we can get away with that since it's a Gecko-only object.
Comment on attachment 584764 [details] [diff] [review]
Patch

Review of attachment 584764 [details] [diff] [review]:
-----------------------------------------------------------------

r=me

::: widget/src/windows/AudioSession.cpp
@@ +84,5 @@
>    STDMETHODIMP OnDisplayNameChanged(LPCWSTR aDisplayName, LPCGUID aContext);
>    STDMETHODIMP OnGroupingParamChanged(LPCGUID aGroupingParam, LPCGUID aContext);
>    STDMETHODIMP OnIconPathChanged(LPCWSTR aIconPath, LPCGUID aContext);
>    STDMETHODIMP OnSessionDisconnected(AudioSessionDisconnectReason aReason);
> +  nsresult OnSessionDisconnectedInternal();

Make this private please.

@@ +451,5 @@
>    mAudioSessionControl->UnregisterAudioSessionNotification(this);
>    mAudioSessionControl = nsnull;
> +
> +  Start(); // If it fails there's not much we can do
> +  return NS_OK;

Allow me to express my disgust over (N)S_OK.  :-)
Attachment #584764 - Flags: review?(ehsan) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/ef6d44e60875
Flags: in-testsuite-
Target Milestone: --- → mozilla12
https://hg.mozilla.org/mozilla-central/rev/ef6d44e60875
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.