Maarten Lankhorst : winmm: Fix midi deadlock by not holding lock on release .
Ken Thomases
ken at codeweavers.com
Wed Mar 19 07:21:14 CDT 2008
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 <m.b.lankhorst at gmail.com>
> 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;
> }
>
>
>
>
More information about the wine-devel
mailing list