From 76f81797b71e12391ce434582b911ef4f6b61390 Mon Sep 17 00:00:00 2001 From: "Ryan C. Gordon" Date: Thu, 19 Oct 2023 15:44:58 -0400 Subject: [PATCH] audio: Device threads don't increment physical device refcounts. Otherwise, they risk the device thread joining on itself. Now we make sure the reference is held at the logical device level until the physical device is closed, so it can't destroy the device in normal usage until the thread is joined, etc. --- src/audio/SDL_audio.c | 6 ++---- src/audio/emscripten/SDL_emscriptenaudio.c | 2 -- src/audio/haiku/SDL_haikuaudio.cc | 2 -- src/audio/jack/SDL_jackaudio.c | 2 -- 4 files changed, 2 insertions(+), 10 deletions(-) diff --git a/src/audio/SDL_audio.c b/src/audio/SDL_audio.c index b96dae2d9c..05e1ffac9b 100644 --- a/src/audio/SDL_audio.c +++ b/src/audio/SDL_audio.c @@ -848,7 +848,6 @@ void SDL_QuitAudio(void) void SDL_AudioThreadFinalize(SDL_AudioDevice *device) { - UnrefPhysicalAudioDevice(device); } static void MixFloat32Audio(float *dst, const float *src, const int buffer_size) @@ -864,7 +863,6 @@ static void MixFloat32Audio(float *dst, const float *src, const int buffer_size) void SDL_OutputAudioThreadSetup(SDL_AudioDevice *device) { SDL_assert(!device->iscapture); - RefPhysicalAudioDevice(device); // unref'd when the audio thread terminates (ProvidesOwnCallbackThread implementations should call SDL_AudioThreadFinalize appropriately). current_audio.impl.ThreadInit(device); } @@ -1014,7 +1012,6 @@ static int SDLCALL OutputAudioThread(void *devicep) // thread entry point void SDL_CaptureAudioThreadSetup(SDL_AudioDevice *device) { SDL_assert(device->iscapture); - RefPhysicalAudioDevice(device); // unref'd when the audio thread terminates (ProvidesOwnCallbackThread implementations should call SDL_AudioThreadFinalize appropriately). current_audio.impl.ThreadInit(device); } @@ -1354,13 +1351,14 @@ void SDL_CloseAudioDevice(SDL_AudioDeviceID devid) if (logdev) { SDL_AudioDevice *device = logdev->physical_device; DestroyLogicalAudioDevice(logdev); - UnrefPhysicalAudioDevice(device); // one reference for each logical device. // !!! FIXME: we _need_ to release this lock, but doing so can cause a race condition if someone opens a device while we're closing it. SDL_UnlockMutex(device->lock); // can't hold the lock or the audio thread will deadlock while we WaitThread it. If not closing, we're done anyhow. if (device->logical_devices == NULL) { // no more logical devices? Close the physical device, too. ClosePhysicalAudioDevice(device); } + + UnrefPhysicalAudioDevice(device); // one reference for each logical device. } } diff --git a/src/audio/emscripten/SDL_emscriptenaudio.c b/src/audio/emscripten/SDL_emscriptenaudio.c index 368499227f..32afd20bc8 100644 --- a/src/audio/emscripten/SDL_emscriptenaudio.c +++ b/src/audio/emscripten/SDL_emscriptenaudio.c @@ -181,8 +181,6 @@ static int EMSCRIPTENAUDIO_OpenDevice(SDL_AudioDevice *device) return SDL_OutOfMemory(); } - RefPhysicalAudioDevice(device); // CloseDevice will always unref this through SDL_AudioThreadFinalize, even if we failed to start the thread. - // limit to native freq device->spec.freq = EM_ASM_INT({ return Module['SDL3'].audioContext.sampleRate; }); diff --git a/src/audio/haiku/SDL_haikuaudio.cc b/src/audio/haiku/SDL_haikuaudio.cc index f52dae593e..5772fbe056 100644 --- a/src/audio/haiku/SDL_haikuaudio.cc +++ b/src/audio/haiku/SDL_haikuaudio.cc @@ -111,8 +111,6 @@ static int HAIKUAUDIO_OpenDevice(SDL_AudioDevice *device) } SDL_zerop(device->hidden); - RefPhysicalAudioDevice(device); // CloseDevice will always unref this through SDL_AudioThreadFinalize, even if we failed to start the thread. - // Parse the audio format and fill the Be raw audio format media_raw_audio_format format; SDL_zero(format); diff --git a/src/audio/jack/SDL_jackaudio.c b/src/audio/jack/SDL_jackaudio.c index 703122c8a8..77e2f03fa9 100644 --- a/src/audio/jack/SDL_jackaudio.c +++ b/src/audio/jack/SDL_jackaudio.c @@ -300,8 +300,6 @@ static int JACK_OpenDevice(SDL_AudioDevice *device) return SDL_OutOfMemory(); } - RefPhysicalAudioDevice(device); // CloseDevice will always unref this through SDL_AudioThreadFinalize, even if we failed to start the thread. - client = JACK_jack_client_open(GetJackAppName(), JackNoStartServer, &status, NULL); device->hidden->client = client; if (client == NULL) {