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=15907b5035ea7f03b369a0463ab1f6ac2b24704e > > Author: Maarten Lankhorst <[EMAIL PROTECTED]> > 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; > } > > > >
