From 4cfcc99c2b09e90e59fd604d69feff5ea9a8e3e9 Mon Sep 17 00:00:00 2001 From: Stanislau Tsitsianok Date: Sat, 30 Mar 2024 00:44:57 +0300 Subject: [PATCH 1/2] faudio: Fix deadlock in voice callbacks Deadlock was being caused by the fact that some private FAudio routines hold internal mutexes while calling application callbacks. This may result in a deadlock because app might have a huge global mutex that is locked both by callbacks and by some code that e.g. submits new buffers to XAudio. Should fix https://bugs.winehq.org/show_bug.cgi?id=54246 --- src/FAudio_internal.c | 66 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 63 insertions(+), 3 deletions(-) diff --git a/src/FAudio_internal.c b/src/FAudio_internal.c index a20e003db..cb53d6523 100644 --- a/src/FAudio_internal.c +++ b/src/FAudio_internal.c @@ -383,6 +383,12 @@ static void FAudio_INTERNAL_DecodeBuffers( if ( voice->src.callback != NULL && voice->src.callback->OnBufferStart != NULL ) { + FAudio_PlatformUnlockMutex(voice->src.bufferLock); + LOG_MUTEX_UNLOCK(voice->audio, voice->src.bufferLock) + + FAudio_PlatformUnlockMutex(voice->sendLock); + LOG_MUTEX_UNLOCK(voice->audio, voice->sendLock) + FAudio_PlatformUnlockMutex(voice->audio->sourceLock); LOG_MUTEX_UNLOCK(voice->audio, voice->audio->sourceLock) @@ -393,6 +399,12 @@ static void FAudio_INTERNAL_DecodeBuffers( FAudio_PlatformLockMutex(voice->audio->sourceLock); LOG_MUTEX_LOCK(voice->audio, voice->audio->sourceLock) + + FAudio_PlatformLockMutex(voice->sendLock); + LOG_MUTEX_LOCK(voice->audio, voice->sendLock) + + FAudio_PlatformLockMutex(voice->src.bufferLock); + LOG_MUTEX_LOCK(voice->audio, voice->src.bufferLock) } } @@ -442,6 +454,12 @@ static void FAudio_INTERNAL_DecodeBuffers( if ( voice->src.callback != NULL && voice->src.callback->OnLoopEnd != NULL ) { + FAudio_PlatformUnlockMutex(voice->src.bufferLock); + LOG_MUTEX_UNLOCK(voice->audio, voice->src.bufferLock) + + FAudio_PlatformUnlockMutex(voice->sendLock); + LOG_MUTEX_UNLOCK(voice->audio, voice->sendLock) + FAudio_PlatformUnlockMutex(voice->audio->sourceLock); LOG_MUTEX_UNLOCK(voice->audio, voice->audio->sourceLock) @@ -452,6 +470,12 @@ static void FAudio_INTERNAL_DecodeBuffers( FAudio_PlatformLockMutex(voice->audio->sourceLock); LOG_MUTEX_LOCK(voice->audio, voice->audio->sourceLock) + + FAudio_PlatformLockMutex(voice->sendLock); + LOG_MUTEX_LOCK(voice->audio, voice->sendLock) + + FAudio_PlatformLockMutex(voice->src.bufferLock); + LOG_MUTEX_LOCK(voice->audio, voice->src.bufferLock) } } else @@ -504,6 +528,12 @@ static void FAudio_INTERNAL_DecodeBuffers( /* Callbacks */ if (voice->src.callback != NULL) { + FAudio_PlatformUnlockMutex(voice->src.bufferLock); + LOG_MUTEX_UNLOCK(voice->audio, voice->src.bufferLock) + + FAudio_PlatformUnlockMutex(voice->sendLock); + LOG_MUTEX_UNLOCK(voice->audio, voice->sendLock) + FAudio_PlatformUnlockMutex(voice->audio->sourceLock); LOG_MUTEX_UNLOCK(voice->audio, voice->audio->sourceLock) @@ -522,6 +552,15 @@ static void FAudio_INTERNAL_DecodeBuffers( ); } + FAudio_PlatformLockMutex(voice->audio->sourceLock); + LOG_MUTEX_LOCK(voice->audio, voice->audio->sourceLock) + + FAudio_PlatformLockMutex(voice->sendLock); + LOG_MUTEX_LOCK(voice->audio, voice->sendLock) + + FAudio_PlatformLockMutex(voice->src.bufferLock); + LOG_MUTEX_LOCK(voice->audio, voice->src.bufferLock) + /* One last chance at redemption */ if (buffer == NULL && voice->src.bufferList != NULL) { @@ -531,14 +570,29 @@ static void FAudio_INTERNAL_DecodeBuffers( if (buffer != NULL && voice->src.callback->OnBufferStart != NULL) { + FAudio_PlatformUnlockMutex(voice->src.bufferLock); + LOG_MUTEX_UNLOCK(voice->audio, voice->src.bufferLock) + + FAudio_PlatformUnlockMutex(voice->sendLock); + LOG_MUTEX_UNLOCK(voice->audio, voice->sendLock) + + FAudio_PlatformUnlockMutex(voice->audio->sourceLock); + LOG_MUTEX_UNLOCK(voice->audio, voice->audio->sourceLock) + voice->src.callback->OnBufferStart( voice->src.callback, buffer->pContext ); - } - FAudio_PlatformLockMutex(voice->audio->sourceLock); - LOG_MUTEX_LOCK(voice->audio, voice->audio->sourceLock) + FAudio_PlatformLockMutex(voice->audio->sourceLock); + LOG_MUTEX_LOCK(voice->audio, voice->audio->sourceLock) + + FAudio_PlatformLockMutex(voice->sendLock); + LOG_MUTEX_LOCK(voice->audio, voice->sendLock) + + FAudio_PlatformLockMutex(voice->src.bufferLock); + LOG_MUTEX_LOCK(voice->audio, voice->src.bufferLock) + } } voice->audio->pFree(toDelete); @@ -1250,6 +1304,9 @@ static void FAudio_INTERNAL_FlushPendingBuffers(FAudioSourceVoice *voice) if (voice->src.callback != NULL && voice->src.callback->OnBufferEnd != NULL) { + FAudio_PlatformUnlockMutex(voice->src.bufferLock); + LOG_MUTEX_UNLOCK(voice->audio, voice->src.bufferLock) + FAudio_PlatformUnlockMutex(voice->audio->sourceLock); LOG_MUTEX_UNLOCK(voice->audio, voice->audio->sourceLock) @@ -1260,6 +1317,9 @@ static void FAudio_INTERNAL_FlushPendingBuffers(FAudioSourceVoice *voice) FAudio_PlatformLockMutex(voice->audio->sourceLock); LOG_MUTEX_LOCK(voice->audio, voice->audio->sourceLock) + + FAudio_PlatformLockMutex(voice->src.bufferLock); + LOG_MUTEX_LOCK(voice->audio, voice->src.bufferLock) } voice->audio->pFree(entry); } From 90e82e9740cc1f824fdcdae40c9cbe9b17c2206b Mon Sep 17 00:00:00 2001 From: anonymous Date: Sun, 21 Apr 2024 17:22:36 +0300 Subject: [PATCH 2/2] faudio: Fix XNA_GetSongEnded false positives Now that we release buffer lock before executing callbacks, some code from another thread can get scheduled between mutex unlock and callback call and observe empty buffer queue right before callback puts a new buffer there. XNA_Song determines the end of the song by simply checking if the queue is empty, which may lead to random playback breaks. Not sure how to properly check for the song end but the buffer decoding function also zeroes out total samples counter when it finishes a EOS-flagged buffer so checking if that counter is zero should cut it... probably? --- src/FAudio_platform_win32.c | 2 +- src/XNA_Song.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/FAudio_platform_win32.c b/src/FAudio_platform_win32.c index cd07e5d8c..4f7be92b1 100644 --- a/src/FAudio_platform_win32.c +++ b/src/FAudio_platform_win32.c @@ -1055,7 +1055,7 @@ FAUDIOAPI uint32_t XNA_GetSongEnded() return 1; } FAudioSourceVoice_GetState(songVoice, &state, 0); - return state.BuffersQueued == 0; + return state.BuffersQueued == 0 && state.SamplesPlayed == 0; } FAUDIOAPI void XNA_EnableVisualization(uint32_t enable) diff --git a/src/XNA_Song.c b/src/XNA_Song.c index 982ff904a..a8755833c 100644 --- a/src/XNA_Song.c +++ b/src/XNA_Song.c @@ -325,7 +325,7 @@ FAUDIOAPI uint32_t XNA_GetSongEnded() return 1; } FAudioSourceVoice_GetState(songVoice, &state, 0); - return state.BuffersQueued == 0; + return state.BuffersQueued == 0 && state.SamplesPlayed == 0; } FAUDIOAPI void XNA_EnableVisualization(uint32_t enable)