From e9a86c541d1e325215acf8383798a9888b81f669 Mon Sep 17 00:00:00 2001 From: Rafael Kitover Date: Thu, 21 Sep 2017 15:39:30 -0700 Subject: [PATCH] fix deadlock in SoundSDL::deinit() #139 #130 On SoundSDL destructor and `reset()`, there was a deadlock with Linux pthreads, it happened sometimes that the reader thread would call `SDL_SemWait(data_available)` while `deinit()` would destroy the semaphore and the `SDL_SemWait()` would end up waiting forever on a null semaphore. Change the sequencing of deinit() to prevent this from happening, set `initialized = false` at the beginning of the sequence to prevent subsequent entries into the reader callback, and add an SDL_Delay(100) between the final `SDL_SemPost()` and `SDL_UnlockMutex()`s and the `SDL_DestroySemaphore()` and `SDL_DestroyMutex()`s to allow a running reader to complete with a valid mutex and semaphores before they are destroyed and the thread is joined. Resetting the sound system also sometimes triggers a memory corruption bug, but that's a separate issue. --- src/common/SoundSDL.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/common/SoundSDL.cpp b/src/common/SoundSDL.cpp index 6b3233d6..dd9650aa 100644 --- a/src/common/SoundSDL.cpp +++ b/src/common/SoundSDL.cpp @@ -51,12 +51,13 @@ std::size_t SoundSDL::buffer_size() { } void SoundSDL::read(uint16_t* stream, int length) { - if (!initialized || length <= 0) + if (length <= 0) return; SDL_memset(stream, audio_spec.silence, length); - if (!emulating) + // if not initialzed, paused or shutting down, do nothing + if (!initialized || !emulating) return; if (!buffer_size()) @@ -76,7 +77,7 @@ void SoundSDL::read(uint16_t* stream, int length) { void SoundSDL::write(uint16_t * finalWave, int length) { if (!initialized) - return; + return; SDL_LockMutex(mutex); @@ -148,6 +149,8 @@ void SoundSDL::deinit() { if (!initialized) return; + initialized = false; + SDL_LockMutex(mutex); int is_emulating = emulating; emulating = 0; @@ -155,6 +158,8 @@ void SoundSDL::deinit() { SDL_SemPost(data_read); SDL_UnlockMutex(mutex); + SDL_Delay(100); + SDL_DestroySemaphore(data_available); data_available = nullptr; SDL_DestroySemaphore(data_read); @@ -166,8 +171,6 @@ void SoundSDL::deinit() { SDL_CloseAudioDevice(sound_device); emulating = is_emulating; - - initialized = false; } SoundSDL::~SoundSDL() {