Re: Maarten Lankhorst : winmm: Fix midi deadlock by not holding lock on release .

[Ken Thomases]

This has a whiff of the double-checked locking anti-pattern.  I can't  
see a way where it would really bite us in this case, but that anti- 
pattern has several subtleties.  So, I just wanted to prompt people to  
double-check it (pun acknowledged).

Cheers,
Ken

On Mar 18, 2008, at 7:44 AM, Alexandre Julliard wrote:
> Module: wine
> Branch: master
> Commit: 15907b5035ea7f03b369a0463ab1f6ac2b24704e
> URL:    http://source.winehq.org/git/wine.git/?a=commit;h=15907b5035ea7f03b369a0463a...
>
> Author: Maarten Lankhorst 
> Date:   Mon Mar 17 13:15:42 2008 -0700
>
> winmm: Fix midi deadlock by not holding lock on release.
>
> dlls/winmm/mci.c |   22 ++++++++++++++--------
> 1 files changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/dlls/winmm/mci.c b/dlls/winmm/mci.c
> index b7b5ac2..c792d18 100644
> --- a/dlls/winmm/mci.c
> +++ b/dlls/winmm/mci.c
> @@ -1767,19 +1767,25 @@ static	DWORD MCI_Close(UINT16 wDevID, DWORD  
> dwParam, LPMCI_GENERIC_PARMS lpParms
>     TRACE("(%04x, %08X, %p)\n", wDevID, dwParam, lpParms);
> if (wDevID == MCI_ALL_DEVICE_ID) {
> - LPWINE_MCIDRIVER	next;
> -
> - EnterCriticalSection(&WINMM_cs);
> /* FIXME: shall I notify once after all is done, or for
> - each of the open drivers ? if the latest, which notif
> - to return when only one fails ?
> */
> - for (wmd = MciDrivers; wmd; ) {
> -
>    next = wmd->lpNext;
> -
>    MCI_Close(wmd->wDeviceID, dwParam, lpParms);
> -
>    wmd = next;
> - while (MciDrivers) {
> -
>        /* Retrieve the device ID under lock, but send the  
>
> message without,
> -
>         * the driver might be calling some winmm functions  
>
> from another
> -
>         * thread before being fully stopped.
> -
>         */
> -
>        EnterCriticalSection(&WINMM_cs);
> -
>        if (!MciDrivers)
> -
>        {
> -
>            LeaveCriticalSection(&WINMM_cs);
> -
>            break;
> -
>        }
> -
>        wDevID = MciDrivers->wDeviceID;
> -
>        LeaveCriticalSection(&WINMM_cs);
> -
>        MCI_Close(wDevID, dwParam, lpParms);
> }
> - LeaveCriticalSection(&WINMM_cs);
> return 0;
> }