714078 - Code in AudioSession::OnSessionDisconnected needs to run asynchronously

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

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.

Comment 1

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Technically NS_DispatchToMainThread takes a synchronization object ... but I think we can get away with that since it's a Gecko-only object.

Comment 3

[(no longer active)]

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.  :-)

Comment 4

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ef6d44e60875

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

https://hg.mozilla.org/mozilla-central/rev/ef6d44e60875