Ken Thomases : winecoreaudio: Avoid potential deadlock in wodOpen.
Module: wine Branch: master Commit: 1f8d743c764db13e82a1629854c531548d6ee498 URL: http://source.winehq.org/git/wine.git/?a=commit;h=1f8d743c764db13e82a1629854... Author: Ken Thomases <ken(a)codeweavers.com> Date: Sat Apr 11 07:18:18 2009 -0500 winecoreaudio: Avoid potential deadlock in wodOpen. --- dlls/winecoreaudio.drv/audio.c | 82 ++++++++++++++++++++++++++++++--------- 1 files changed, 63 insertions(+), 19 deletions(-) diff --git a/dlls/winecoreaudio.drv/audio.c b/dlls/winecoreaudio.drv/audio.c index 2735fb7..e81aa3e 100644 --- a/dlls/winecoreaudio.drv/audio.c +++ b/dlls/winecoreaudio.drv/audio.c @@ -32,6 +32,7 @@ # include <unistd.h> #endif #include <fcntl.h> +#include <assert.h> #include "windef.h" #include "winbase.h" @@ -103,6 +104,7 @@ AudioUnitRender( AudioUnit ci, #define WINE_WS_PAUSED 1 #define WINE_WS_STOPPED 2 #define WINE_WS_CLOSED 3 +#define WINE_WS_OPENING 4 typedef struct tagCoreAudio_Device { char dev_name[32]; @@ -771,9 +773,10 @@ static DWORD wodGetDevCaps(WORD wDevID, LPWAVEOUTCAPSW lpCaps, DWORD dwSize) static DWORD wodOpen(WORD wDevID, LPWAVEOPENDESC lpDesc, DWORD dwFlags) { WINE_WAVEOUT* wwo; - DWORD retval; DWORD ret; AudioStreamBasicDescription streamFormat; + AudioUnit audioUnit = NULL; + BOOL auInited = FALSE; TRACE("(%u, %p, %08x);\n", wDevID, lpDesc, dwFlags); if (lpDesc == NULL) @@ -808,7 +811,15 @@ static DWORD wodOpen(WORD wDevID, LPWAVEOPENDESC lpDesc, DWORD dwFlags) lpDesc->lpFormat->nSamplesPerSec); return MMSYSERR_NOERROR; } - + + /* We proceed in three phases: + * o Reserve the device for us, marking it as unavailable (not closed) + * o Create, configure, and start the Audio Unit. To avoid deadlock, + * this has to be done without holding wwo->lock. + * o If that was successful, finish setting up our device info and + * mark the device as ready. + * Otherwise, clean up and mark the device as available. + */ wwo = &WOutDev[wDevID]; if (!OSSpinLockTry(&wwo->lock)) return MMSYSERR_ALLOCATED; @@ -819,11 +830,16 @@ static DWORD wodOpen(WORD wDevID, LPWAVEOPENDESC lpDesc, DWORD dwFlags) return MMSYSERR_ALLOCATED; } - if (!AudioUnit_CreateDefaultAudioUnit((void *) wwo, &wwo->audioUnit)) + wwo->state = WINE_WS_OPENING; + wwo->audioUnit = NULL; + OSSpinLockUnlock(&wwo->lock); + + + if (!AudioUnit_CreateDefaultAudioUnit((void *) wwo, &audioUnit)) { ERR("CoreAudio_CreateDefaultAudioUnit(%p) failed\n", wwo); - OSSpinLockUnlock(&wwo->lock); - return MMSYSERR_ERROR; + ret = MMSYSERR_ERROR; + goto error; } streamFormat.mFormatID = kAudioFormatLinearPCM; @@ -842,25 +858,35 @@ static DWORD wodOpen(WORD wDevID, LPWAVEOPENDESC lpDesc, DWORD dwFlags) streamFormat.mBytesPerFrame = streamFormat.mBitsPerChannel * streamFormat.mChannelsPerFrame / 8; streamFormat.mBytesPerPacket = streamFormat.mBytesPerFrame * streamFormat.mFramesPerPacket; - ret = AudioUnit_InitializeWithStreamDescription(wwo->audioUnit, &streamFormat); + ret = AudioUnit_InitializeWithStreamDescription(audioUnit, &streamFormat); if (!ret) { - AudioUnit_CloseAudioUnit(wwo->audioUnit); - OSSpinLockUnlock(&wwo->lock); - return WAVERR_BADFORMAT; /* FIXME return an error based on the OSStatus */ + ret = WAVERR_BADFORMAT; /* FIXME return an error based on the OSStatus */ + goto error; } - wwo->streamDescription = streamFormat; + auInited = TRUE; - ret = AudioOutputUnitStart(wwo->audioUnit); + /* Our render callback CoreAudio_woAudioUnitIOProc may be called before + * AudioOutputUnitStart returns. Core Audio will grab its own internal + * lock before calling it and the callback grabs wwo->lock. This would + * deadlock if we were holding wwo->lock. + * Also, the callback has to safely do nothing in that case, because + * wwo hasn't been completely filled out, yet. */ + ret = AudioOutputUnitStart(audioUnit); if (ret) { ERR("AudioOutputUnitStart failed: %08x\n", ret); - AudioUnitUninitialize(wwo->audioUnit); - AudioUnit_CloseAudioUnit(wwo->audioUnit); - OSSpinLockUnlock(&wwo->lock); - return MMSYSERR_ERROR; /* FIXME return an error based on the OSStatus */ + ret = MMSYSERR_ERROR; /* FIXME return an error based on the OSStatus */ + goto error; } + + OSSpinLockLock(&wwo->lock); + assert(wwo->state == WINE_WS_OPENING); + + wwo->audioUnit = audioUnit; + wwo->streamDescription = streamFormat; + wwo->state = WINE_WS_STOPPED; wwo->wFlags = HIWORD(dwFlags & CALLBACK_TYPEMASK); @@ -885,9 +911,24 @@ static DWORD wodOpen(WORD wDevID, LPWAVEOPENDESC lpDesc, DWORD dwFlags) OSSpinLockUnlock(&wwo->lock); - retval = wodNotifyClient(wwo, WOM_OPEN, 0L, 0L); + ret = wodNotifyClient(wwo, WOM_OPEN, 0L, 0L); - return retval; + return ret; + +error: + if (audioUnit) + { + if (auInited) + AudioUnitUninitialize(audioUnit); + AudioUnit_CloseAudioUnit(audioUnit); + } + + OSSpinLockLock(&wwo->lock); + assert(wwo->state == WINE_WS_OPENING); + wwo->state = WINE_WS_CLOSED; + OSSpinLockUnlock(&wwo->lock); + + return ret; } /************************************************************************** @@ -1291,7 +1332,7 @@ static DWORD wodReset(WORD wDevID) OSSpinLockLock(&wwo->lock); - if (wwo->state == WINE_WS_CLOSED) + if (wwo->state == WINE_WS_CLOSED || wwo->state == WINE_WS_OPENING) { OSSpinLockUnlock(&wwo->lock); WARN("resetting a closed device\n"); @@ -1589,6 +1630,9 @@ OSStatus CoreAudio_woAudioUnitIOProc(void *inRefCon, OSSpinLockLock(&wwo->lock); + /* We might have been called before wwo has been completely filled out by + * wodOpen. We have to do nothing in that case. The check of wwo->state + * below ensures that. */ while (dataNeeded > 0 && wwo->state == WINE_WS_PLAYING && wwo->lpPlayPtr) { unsigned int available = wwo->lpPlayPtr->dwBufferLength - wwo->dwPartialOffset;
participants (1)
-
Alexandre Julliard