SDL: audio: Massive reworking on thread locking.

From 0e614d9179ddfa1904e0ce06a22b825a3aba016e Mon Sep 17 00:00:00 2001
From: "Ryan C. Gordon" <[EMAIL REDACTED]>
Date: Wed, 1 Nov 2023 00:10:25 -0400
Subject: [PATCH] audio: Massive reworking on thread locking.

This cleans up a ton of race conditions, and starts moving towards something
we can use with Clang's -Wthread-safety (but that has a ways to go still).
---
 src/audio/SDL_audio.c | 629 +++++++++++++++++++++++-------------------
 1 file changed, 345 insertions(+), 284 deletions(-)

diff --git a/src/audio/SDL_audio.c b/src/audio/SDL_audio.c
index 9c6afb39e7c5..a46f4efa5390 100644
--- a/src/audio/SDL_audio.c
+++ b/src/audio/SDL_audio.c
@@ -289,6 +289,134 @@ static SDL_AudioDeviceID AssignAudioDeviceInstanceId(SDL_bool iscapture, SDL_boo
     return instance_id;
 }
 
+static void ObtainPhysicalAudioDeviceObj(SDL_AudioDevice *device) SDL_NO_THREAD_SAFETY_ANALYSIS  // !!! FIXMEL SDL_ACQUIRE
+{
+    if (device) {
+        RefPhysicalAudioDevice(device);
+        SDL_LockMutex(device->lock);
+    }
+}
+
+static void ReleaseAudioDevice(SDL_AudioDevice *device) SDL_NO_THREAD_SAFETY_ANALYSIS  // !!! FIXME: SDL_RELEASE
+{
+    if (device) {
+        SDL_UnlockMutex(device->lock);
+        UnrefPhysicalAudioDevice(device);
+    }
+}
+
+// If found, this locks _the physical device_ this logical device is associated with, before returning.
+static SDL_LogicalAudioDevice *ObtainLogicalAudioDevice(SDL_AudioDeviceID devid, SDL_AudioDevice **device) SDL_NO_THREAD_SAFETY_ANALYSIS    // !!! FIXME: SDL_ACQUIRE
+{
+    SDL_assert(device != NULL);
+
+    *device = NULL;
+
+    if (!SDL_GetCurrentAudioDriver()) {
+        SDL_SetError("Audio subsystem is not initialized");
+        return NULL;
+    }
+
+    SDL_LogicalAudioDevice *logdev = NULL;
+
+    // bit #1 of devid is set for physical devices and unset for logical.
+    const SDL_bool islogical = (devid & (1<<1)) ? SDL_FALSE : SDL_TRUE;
+    if (islogical) {  // don't bother looking if it's not a logical device id value.
+        SDL_LockRWLockForReading(current_audio.device_hash_lock);
+        SDL_FindInHashTable(current_audio.device_hash, (const void *) (uintptr_t) devid, (const void **) &logdev);
+        if (logdev) {
+            *device = logdev->physical_device;
+            RefPhysicalAudioDevice(*device);  // reference it, in case the logical device migrates to a new default.
+        }
+        SDL_UnlockRWLock(current_audio.device_hash_lock);
+    }
+
+    if (!logdev) {
+        SDL_SetError("Invalid audio device instance ID");
+    } else {
+        SDL_assert(*device != NULL);
+        SDL_LockMutex((*device)->lock);
+    }
+
+    return logdev;
+}
+
+
+/* this finds the physical device associated with `devid` and locks it for use.
+   Note that a logical device instance id will return its associated physical device! */
+static SDL_AudioDevice *ObtainPhysicalAudioDevice(SDL_AudioDeviceID devid)  // !!! FIXME: SDL_ACQUIRE
+{
+    SDL_AudioDevice *device = NULL;
+
+    // bit #1 of devid is set for physical devices and unset for logical.
+    const SDL_bool islogical = (devid & (1<<1)) ? SDL_FALSE : SDL_TRUE;
+    if (islogical) {
+        ObtainLogicalAudioDevice(devid, &device);
+    } else if (!SDL_GetCurrentAudioDriver()) {  // (the `islogical` path, above, checks this in ObtainLogicalAudioDevice.)
+        SDL_SetError("Audio subsystem is not initialized");
+    } else {
+        SDL_LockRWLockForReading(current_audio.device_hash_lock);
+        SDL_FindInHashTable(current_audio.device_hash, (const void *) (uintptr_t) devid, (const void **) &device);
+        SDL_UnlockRWLock(current_audio.device_hash_lock);
+
+        if (!device) {
+            SDL_SetError("Invalid audio device instance ID");
+        } else {
+            ObtainPhysicalAudioDeviceObj(device);
+        }
+    }
+
+    return device;
+}
+
+static SDL_AudioDevice *ObtainPhysicalAudioDeviceDefaultAllowed(SDL_AudioDeviceID devid)  // !!! FIXME: SDL_ACQUIRE
+{
+    const SDL_bool wants_default = ((devid == SDL_AUDIO_DEVICE_DEFAULT_OUTPUT) || (devid == SDL_AUDIO_DEVICE_DEFAULT_CAPTURE)) ? SDL_TRUE : SDL_FALSE;
+    if (!wants_default) {
+        return ObtainPhysicalAudioDevice(devid);
+    }
+
+    const SDL_AudioDeviceID orig_devid = devid;
+
+    while (SDL_TRUE) {
+        SDL_LockRWLockForReading(current_audio.device_hash_lock);
+        if (orig_devid == SDL_AUDIO_DEVICE_DEFAULT_OUTPUT) {
+            devid = current_audio.default_output_device_id;
+        } else if (orig_devid == SDL_AUDIO_DEVICE_DEFAULT_CAPTURE) {
+            devid = current_audio.default_capture_device_id;
+        }
+        SDL_UnlockRWLock(current_audio.device_hash_lock);
+
+        if (devid == 0) {
+            SDL_SetError("No default audio device available");
+            break;
+        }
+
+        SDL_AudioDevice *device = ObtainPhysicalAudioDevice(devid);
+        if (!device) {
+            break;
+        }
+
+        // make sure the default didn't change while we were waiting for the lock...
+        SDL_bool got_it = SDL_FALSE;
+        SDL_LockRWLockForReading(current_audio.device_hash_lock);
+        if ((orig_devid == SDL_AUDIO_DEVICE_DEFAULT_OUTPUT) && (devid == current_audio.default_output_device_id)) {
+            got_it = SDL_TRUE;
+        } else if ((orig_devid == SDL_AUDIO_DEVICE_DEFAULT_CAPTURE) && (devid == current_audio.default_capture_device_id)) {
+            got_it = SDL_TRUE;
+        }
+        SDL_UnlockRWLock(current_audio.device_hash_lock);
+
+        if (got_it) {
+            return device;
+        }
+
+        ReleaseAudioDevice(device);  // let it go and try again.
+    }
+
+    return NULL;
+}
+
 // this assumes you hold the _physical_ device lock for this logical device! This will not unlock the lock or close the physical device!
 //  It also will not unref the physical device, since we might be shutting down; SDL_CloseAudioDevice handles the unref.
 static void DestroyLogicalAudioDevice(SDL_LogicalAudioDevice *logdev)
@@ -334,11 +462,11 @@ static void DestroyPhysicalAudioDevice(SDL_AudioDevice *device)
     }
 
     // Destroy any logical devices that still exist...
-    SDL_LockMutex(device->lock);
+    SDL_LockMutex(device->lock);  // don't use ObtainPhysicalAudioDeviceObj because we don't want to change refcounts while destroying.
     while (device->logical_devices != NULL) {
         DestroyLogicalAudioDevice(device->logical_devices);
     }
-    SDL_UnlockMutex(device->lock);
+    SDL_UnlockMutex(device->lock);  // don't use ReleaseAudioDevice because we don't want to change refcounts while destroying.
 
     // it's safe to not hold the lock for this (we can't anyhow, or the audio thread won't quit), because we shouldn't be in the device list at this point.
     ClosePhysicalAudioDevice(device);
@@ -483,27 +611,6 @@ void SDL_AudioDeviceDisconnected(SDL_AudioDevice *device)
         return;
     }
 
-    SDL_LockMutex(device->lock);
-
-    if (!SDL_AtomicCAS(&device->zombie, 0, 1)) {
-        SDL_UnlockMutex(device->lock);
-        return;  // already disconnected this device, don't do it twice.
-    }
-
-    // Swap in "Zombie" versions of the usual platform interfaces, so the device will keep
-    // making progress until the app closes it. Otherwise, streams might continue to
-    // accumulate waste data that never drains, apps that depend on audio callbacks to
-    // progress will freeze, etc.
-    device->WaitDevice = ZombieWaitDevice;
-    device->GetDeviceBuf = ZombieGetDeviceBuf;
-    device->PlayDevice = ZombiePlayDevice;
-    device->WaitCaptureDevice = ZombieWaitDevice;
-    device->CaptureFromDevice = ZombieCaptureFromDevice;
-    device->FlushCapture = ZombieFlushCapture;
-
-    const SDL_AudioDeviceID devid = device->instance_id;
-    const SDL_bool is_default_device = ((devid == current_audio.default_output_device_id) || (devid == current_audio.default_capture_device_id)) ? SDL_TRUE : SDL_FALSE;
-
     // Save off removal info in a list so we can send events for each, next
     //  time the event queue pumps, in case something tries to close a device
     //  from an event filter, as this would risk deadlocks and other disasters
@@ -512,45 +619,64 @@ void SDL_AudioDeviceDisconnected(SDL_AudioDevice *device)
     pending.next = NULL;
     SDL_PendingAudioDeviceEvent *pending_tail = &pending;
 
-    // on default devices, dump any logical devices that explicitly opened this device. Things that opened the system default can stay.
-    // on non-default devices, dump everything.
-    // (by "dump" we mean send a REMOVED event; the zombie will keep consuming audio data for these logical devices until explicitly closed.)
-    for (SDL_LogicalAudioDevice *logdev = device->logical_devices; logdev != NULL; logdev = logdev->next) {
-        if (!is_default_device || !logdev->opened_as_default) {  // if opened as a default, leave it on the zombie device for later migration.
-            SDL_PendingAudioDeviceEvent *p = (SDL_PendingAudioDeviceEvent *) SDL_malloc(sizeof (SDL_PendingAudioDeviceEvent));
-            if (p) {  // if this failed, no event for you, but you have deeper problems anyhow.
-                p->type = SDL_EVENT_AUDIO_DEVICE_REMOVED;
-                p->devid = logdev->instance_id;
-                p->next = NULL;
-                pending_tail->next = p;
-                pending_tail = p;
+    ObtainPhysicalAudioDeviceObj(device);
+
+    SDL_LockRWLockForReading(current_audio.device_hash_lock);
+    const SDL_AudioDeviceID devid = device->instance_id;
+    const SDL_bool is_default_device = ((devid == current_audio.default_output_device_id) || (devid == current_audio.default_capture_device_id)) ? SDL_TRUE : SDL_FALSE;
+    SDL_UnlockRWLock(current_audio.device_hash_lock);
+
+    const SDL_bool first_disconnect = SDL_AtomicCAS(&device->zombie, 0, 1);
+    if (first_disconnect) {   // if already disconnected this device, don't do it twice.
+        // Swap in "Zombie" versions of the usual platform interfaces, so the device will keep
+        // making progress until the app closes it. Otherwise, streams might continue to
+        // accumulate waste data that never drains, apps that depend on audio callbacks to
+        // progress will freeze, etc.
+        device->WaitDevice = ZombieWaitDevice;
+        device->GetDeviceBuf = ZombieGetDeviceBuf;
+        device->PlayDevice = ZombiePlayDevice;
+        device->WaitCaptureDevice = ZombieWaitDevice;
+        device->CaptureFromDevice = ZombieCaptureFromDevice;
+        device->FlushCapture = ZombieFlushCapture;
+
+        // on default devices, dump any logical devices that explicitly opened this device. Things that opened the system default can stay.
+        // on non-default devices, dump everything.
+        // (by "dump" we mean send a REMOVED event; the zombie will keep consuming audio data for these logical devices until explicitly closed.)
+        for (SDL_LogicalAudioDevice *logdev = device->logical_devices; logdev != NULL; logdev = logdev->next) {
+            if (!is_default_device || !logdev->opened_as_default) {  // if opened as a default, leave it on the zombie device for later migration.
+                SDL_PendingAudioDeviceEvent *p = (SDL_PendingAudioDeviceEvent *) SDL_malloc(sizeof (SDL_PendingAudioDeviceEvent));
+                if (p) {  // if this failed, no event for you, but you have deeper problems anyhow.
+                    p->type = SDL_EVENT_AUDIO_DEVICE_REMOVED;
+                    p->devid = logdev->instance_id;
+                    p->next = NULL;
+                    pending_tail->next = p;
+                    pending_tail = p;
+                }
             }
         }
-    }
 
-    SDL_PendingAudioDeviceEvent *p = (SDL_PendingAudioDeviceEvent *) SDL_malloc(sizeof (SDL_PendingAudioDeviceEvent));
-    if (p) {  // if this failed, no event for you, but you have deeper problems anyhow.
-        p->type = SDL_EVENT_AUDIO_DEVICE_REMOVED;
-        p->devid = device->instance_id;
-        p->next = NULL;
-        pending_tail->next = p;
-        pending_tail = p;
+        SDL_PendingAudioDeviceEvent *p = (SDL_PendingAudioDeviceEvent *) SDL_malloc(sizeof (SDL_PendingAudioDeviceEvent));
+        if (p) {  // if this failed, no event for you, but you have deeper problems anyhow.
+            p->type = SDL_EVENT_AUDIO_DEVICE_REMOVED;
+            p->devid = device->instance_id;
+            p->next = NULL;
+            pending_tail->next = p;
+            pending_tail = p;
+        }
     }
 
-    SDL_UnlockMutex(device->lock);
+    ReleaseAudioDevice(device);
 
-    if (pending.next) {  // NULL if event is disabled or disaster struck.
-        SDL_LockRWLockForWriting(current_audio.device_hash_lock);
-        SDL_assert(current_audio.pending_events_tail != NULL);
-        SDL_assert(current_audio.pending_events_tail->next == NULL);
-        current_audio.pending_events_tail->next = pending.next;
-        current_audio.pending_events_tail = pending_tail;
-        SDL_UnlockRWLock(current_audio.device_hash_lock);
-    }
+    if (first_disconnect) {
+        if (pending.next) {  // NULL if event is disabled or disaster struck.
+            SDL_LockRWLockForWriting(current_audio.device_hash_lock);
+            SDL_assert(current_audio.pending_events_tail != NULL);
+            SDL_assert(current_audio.pending_events_tail->next == NULL);
+            current_audio.pending_events_tail->next = pending.next;
+            current_audio.pending_events_tail = pending_tail;
+            SDL_UnlockRWLock(current_audio.device_hash_lock);
+        }
 
-    // Is this a non-default device? We can unref it now.
-    // Otherwise, we'll unref it when a new default device is chosen.
-    if (!is_default_device) {
         UnrefPhysicalAudioDevice(device);
     }
 }
@@ -622,9 +748,10 @@ static void CompleteAudioEntryPoints(void)
     #undef FILL_STUB
 }
 
-static SDL_AudioDeviceID GetFirstAddedAudioDeviceID(const SDL_bool iscapture)
+static SDL_AudioDevice *GetFirstAddedAudioDevice(const SDL_bool iscapture)
 {
-    SDL_AudioDeviceID retval = (SDL_AudioDeviceID) SDL_AUDIO_DEVICE_DEFAULT_OUTPUT;  // According to AssignAudioDeviceInstanceId, nothing can have a value this large.
+    SDL_AudioDeviceID highest = (SDL_AudioDeviceID) SDL_AUDIO_DEVICE_DEFAULT_OUTPUT;  // According to AssignAudioDeviceInstanceId, nothing can have a value this large.
+    SDL_AudioDevice *retval = NULL;
 
     // (Device IDs increase as new devices are added, so the first device added has the lowest SDL_AudioDeviceID value.)
     SDL_LockRWLockForReading(current_audio.device_hash_lock);
@@ -633,16 +760,17 @@ static SDL_AudioDeviceID GetFirstAddedAudioDeviceID(const SDL_bool iscapture)
     const void *value;
     void *iter = NULL;
     while (SDL_IterateHashTable(current_audio.device_hash, &key, &value, &iter)) {
-        const SDL_AudioDeviceID devid = (SDL_AudioDeviceID) (uintptr_t) value;
+        const SDL_AudioDeviceID devid = (SDL_AudioDeviceID) (uintptr_t) key;
         // bit #1 of devid is set for physical devices and unset for logical.
         const SDL_bool isphysical = (devid & (1<<1)) ? SDL_TRUE : SDL_FALSE;
-        if (isphysical && (devid < retval)) {
-            retval = devid;
+        if (isphysical && (devid < highest)) {
+            highest = devid;
+            retval = (SDL_AudioDevice *) value;
         }
     }
 
     SDL_UnlockRWLock(current_audio.device_hash_lock);
-    return (retval == SDL_AUDIO_DEVICE_DEFAULT_OUTPUT) ? 0 : retval;
+    return retval;
 }
 
 static Uint32 HashAudioDeviceID(const void *key, void *data)
@@ -779,20 +907,23 @@ int SDL_InitAudio(const char *driver_name)
     SDL_AudioDevice *default_capture = NULL;
     current_audio.impl.DetectDevices(&default_output, &default_capture);
 
-    // these are only set if default_* is non-NULL, in case the backend just called SDL_DefaultAudioDeviceChanged directly during DetectDevices.
+    // If no default was _ever_ specified, just take the first device we see, if any.
+    if (!default_output) {
+        default_output = GetFirstAddedAudioDevice(/*iscapture=*/SDL_FALSE);
+    }
+
+    if (!default_capture) {
+        default_capture = GetFirstAddedAudioDevice(/*iscapture=*/SDL_TRUE);
+    }
+
     if (default_output) {
         current_audio.default_output_device_id = default_output->instance_id;
+        RefPhysicalAudioDevice(default_output);  // extra ref on default devices.
     }
+
     if (default_capture) {
         current_audio.default_capture_device_id = default_capture->instance_id;
-    }
-
-    // If no default was _ever_ specified, just take the first device we see, if any.
-    if (!current_audio.default_output_device_id) {
-        current_audio.default_output_device_id = GetFirstAddedAudioDeviceID(/*iscapture=*/SDL_FALSE);
-    }
-    if (!current_audio.default_capture_device_id) {
-        current_audio.default_capture_device_id = GetFirstAddedAudioDeviceID(/*iscapture=*/SDL_TRUE);
+        RefPhysicalAudioDevice(default_capture);  // extra ref on default devices.
     }
 
     return 0;
@@ -1171,62 +1302,6 @@ SDL_AudioDeviceID *SDL_GetAudioCaptureDevices(int *count)
     return GetAudioDevices(count, SDL_TRUE);
 }
 
-// If found, this locks _the physical device_ this logical device is associated with, before returning.
-static SDL_LogicalAudioDevice *ObtainLogicalAudioDevice(SDL_AudioDeviceID devid)
-{
-    if (!SDL_GetCurrentAudioDriver()) {
-        SDL_SetError("Audio subsystem is not initialized");
-        return NULL;
-    }
-
-    SDL_LogicalAudioDevice *logdev = NULL;
-
-    // bit #1 of devid is set for physical devices and unset for logical.
-    const SDL_bool islogical = (devid & (1<<1)) ? SDL_FALSE : SDL_TRUE;
-    if (islogical) {  // don't bother looking if it's not a logical device id value.
-        SDL_LockRWLockForReading(current_audio.device_hash_lock);
-        SDL_FindInHashTable(current_audio.device_hash, (const void *) (uintptr_t) devid, (const void **) &logdev);
-        SDL_UnlockRWLock(current_audio.device_hash_lock);
-    }
-
-    if (!logdev) {
-        SDL_SetError("Invalid audio device instance ID");
-    } else {
-        SDL_LockMutex(logdev->physical_device->lock);
-    }
-
-    return logdev;
-}
-
-/* this finds the physical device associated with `devid` and locks it for use.
-   Note that a logical device instance id will return its associated physical device! */
-static SDL_AudioDevice *ObtainPhysicalAudioDevice(SDL_AudioDeviceID devid)
-{
-    SDL_AudioDevice *device = NULL;
-
-    // bit #1 of devid is set for physical devices and unset for logical.
-    const SDL_bool islogical = (devid & (1<<1)) ? SDL_FALSE : SDL_TRUE;
-    if (islogical) {
-        SDL_LogicalAudioDevice *logdev = ObtainLogicalAudioDevice(devid);
-        if (logdev) {
-            device = logdev->physical_device;
-        }
-    } else if (!SDL_GetCurrentAudioDriver()) {  // (the `islogical` path, above,  checks this in ObtainLogicalAudioDevice.)
-        SDL_SetError("Audio subsystem is not initialized");
-    } else {
-        SDL_LockRWLockForReading(current_audio.device_hash_lock);
-        SDL_FindInHashTable(current_audio.device_hash, (const void *) (uintptr_t) devid, (const void **) &device);
-        SDL_UnlockRWLock(current_audio.device_hash_lock);
-
-        if (!device) {
-            SDL_SetError("Invalid audio device instance ID");
-        } else {
-            SDL_LockMutex(device->lock);  // caller must unlock.
-        }
-    }
-
-    return device;
-}
 
 SDL_AudioDevice *SDL_FindPhysicalAudioDeviceByCallback(SDL_bool (*callback)(SDL_AudioDevice *device, void *userdata), void *userdata)
 {
@@ -1270,17 +1345,15 @@ SDL_AudioDevice *SDL_FindPhysicalAudioDeviceByHandle(void *handle)
 
 char *SDL_GetAudioDeviceName(SDL_AudioDeviceID devid)
 {
+    char *retval = NULL;
     SDL_AudioDevice *device = ObtainPhysicalAudioDevice(devid);
-    if (!device) {
-        return NULL;
-    }
-
-    char *retval = SDL_strdup(device->name);
-    if (!retval) {
-        SDL_OutOfMemory();
+    if (device) {
+        retval = SDL_strdup(device->name);
+        if (!retval) {
+            SDL_OutOfMemory();
+        }
     }
-
-    SDL_UnlockMutex(device->lock);
+    ReleaseAudioDevice(device);
 
     return retval;
 }
@@ -1291,31 +1364,18 @@ int SDL_GetAudioDeviceFormat(SDL_AudioDeviceID devid, SDL_AudioSpec *spec, int *
         return SDL_InvalidParamError("spec");
     }
 
-    SDL_bool wants_default = SDL_FALSE;
-    if (devid == SDL_AUDIO_DEVICE_DEFAULT_OUTPUT) {
-        devid = current_audio.default_output_device_id;
-        wants_default = SDL_TRUE;
-    } else if (devid == SDL_AUDIO_DEVICE_DEFAULT_CAPTURE) {
-        devid = current_audio.default_capture_device_id;
-        wants_default = SDL_TRUE;
-    }
-
-    if ((devid == 0) && wants_default) {
-        return SDL_SetError("No default audio device available");
-    }
-
-    SDL_AudioDevice *device = ObtainPhysicalAudioDevice(devid);
-    if (!device) {
-        return -1;
-    }
-
-    SDL_copyp(spec, &device->spec);
-    if (sample_frames) {
-        *sample_frames = device->sample_frames;
+    int retval = -1;
+    SDL_AudioDevice *device = ObtainPhysicalAudioDeviceDefaultAllowed(devid);  // !!! FIXME: this needs an ObtainBlahDefaultAllowed to catch default device changes.
+    if (device) {
+        SDL_copyp(spec, &device->spec);
+        if (sample_frames) {
+            *sample_frames = device->sample_frames;
+        }
+        retval = 0;
     }
-    SDL_UnlockMutex(device->lock);
+    ReleaseAudioDevice(device);
 
-    return 0;
+    return retval;
 }
 
 // this expects the device lock to be held.  !!! FIXME: no it doesn't...?
@@ -1350,20 +1410,21 @@ static void ClosePhysicalAudioDevice(SDL_AudioDevice *device)
 
 void SDL_CloseAudioDevice(SDL_AudioDeviceID devid)
 {
-    SDL_LogicalAudioDevice *logdev = ObtainLogicalAudioDevice(devid);
+    SDL_bool close_physical = SDL_FALSE;
+    SDL_AudioDevice *device = NULL;
+    SDL_LogicalAudioDevice *logdev = ObtainLogicalAudioDevice(devid, &device);
     if (logdev) {
-        SDL_AudioDevice *device = logdev->physical_device;
         DestroyLogicalAudioDevice(logdev);
+        close_physical = (device->logical_devices == NULL); // no more logical devices? Close the physical device, too.
+    }
 
-        const SDL_bool close_physical = (device->logical_devices == NULL); // no more logical devices? Close the physical device, too.
-
-        // !!! 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.
+    // !!! 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.
+    ReleaseAudioDevice(device);  // can't hold the lock or the audio thread will deadlock while we WaitThread it. If not closing, we're done anyhow.
 
+    if (device) {
         if (close_physical) {
             ClosePhysicalAudioDevice(device);
         }
-
         UnrefPhysicalAudioDevice(device);  // one reference for each logical device.
     }
 }
@@ -1514,30 +1575,17 @@ SDL_AudioDeviceID SDL_OpenAudioDevice(SDL_AudioDeviceID devid, const SDL_AudioSp
         return 0;
     }
 
-    SDL_bool wants_default = SDL_FALSE;
-    if (devid == SDL_AUDIO_DEVICE_DEFAULT_OUTPUT) {
-        devid = current_audio.default_output_device_id;
-        wants_default = SDL_TRUE;
-    } else if (devid == SDL_AUDIO_DEVICE_DEFAULT_CAPTURE) {
-        devid = current_audio.default_capture_device_id;
-        wants_default = SDL_TRUE;
-    }
-
-    if ((devid == 0) && wants_default) {
-        SDL_SetError("No default audio device available");
-        return 0;
-    }
+    SDL_bool wants_default = ((devid == SDL_AUDIO_DEVICE_DEFAULT_OUTPUT) || (devid == SDL_AUDIO_DEVICE_DEFAULT_CAPTURE)) ? SDL_TRUE : SDL_FALSE;
 
     // this will let you use a logical device to make a new logical device on the parent physical device. Could be useful?
     SDL_AudioDevice *device = NULL;
-    const SDL_bool islogical = (devid & (1<<1)) ? SDL_FALSE : SDL_TRUE;
+    const SDL_bool islogical = (wants_default || (devid & (1<<1))) ? SDL_FALSE : SDL_TRUE;
     if (!islogical) {
-        device = ObtainPhysicalAudioDevice(devid);
+        device = ObtainPhysicalAudioDeviceDefaultAllowed(devid);
     } else {
-        SDL_LogicalAudioDevice *logdev = ObtainLogicalAudioDevice(devid);  // this locks the physical device, too.
+        SDL_LogicalAudioDevice *logdev = ObtainLogicalAudioDevice(devid, &device);
         if (logdev) {
             wants_default = logdev->opened_as_default;  // was the original logical device meant to be a default? Make this one, too.
-            device = logdev->physical_device;
         }
     }
 
@@ -1565,7 +1613,7 @@ SDL_AudioDeviceID SDL_OpenAudioDevice(SDL_AudioDeviceID devid, const SDL_AudioSp
             device->logical_devices = logdev;
             UpdateAudioStreamFormatsPhysical(device);
         }
-        SDL_UnlockMutex(device->lock);
+        ReleaseAudioDevice(device);
 
         if (retval) {
             SDL_LockRWLockForWriting(current_audio.device_hash_lock);
@@ -1583,13 +1631,13 @@ SDL_AudioDeviceID SDL_OpenAudioDevice(SDL_AudioDeviceID devid, const SDL_AudioSp
 
 static int SetLogicalAudioDevicePauseState(SDL_AudioDeviceID devid, int value)
 {
-    SDL_LogicalAudioDevice *logdev = ObtainLogicalAudioDevice(devid);
-    if (!logdev) {
-        return -1;  // ObtainLogicalAudioDevice will have set an error.
+    SDL_AudioDevice *device = NULL;
+    SDL_LogicalAudioDevice *logdev = ObtainLogicalAudioDevice(devid, &device);
+    if (logdev) {
+        SDL_AtomicSet(&logdev->paused, value);
     }
-    SDL_AtomicSet(&logdev->paused, value);
-    SDL_UnlockMutex(logdev->physical_device->lock);
-    return 0;
+    ReleaseAudioDevice(device);
+    return logdev ? 0 : -1;  // ObtainLogicalAudioDevice will have set an error.
 }
 
 int SDL_PauseAudioDevice(SDL_AudioDeviceID devid)
@@ -1604,23 +1652,22 @@ int SDLCALL SDL_ResumeAudioDevice(SDL_AudioDeviceID devid)
 
 SDL_bool SDL_AudioDevicePaused(SDL_AudioDeviceID devid)
 {
-    SDL_LogicalAudioDevice *logdev = ObtainLogicalAudioDevice(devid);
+    SDL_AudioDevice *device = NULL;
+    SDL_LogicalAudioDevice *logdev = ObtainLogicalAudioDevice(devid, &device);
     SDL_bool retval = SDL_FALSE;
-    if (logdev) {
-        if (SDL_AtomicGet(&logdev->paused)) {
-            retval = SDL_TRUE;
-        }
-        SDL_UnlockMutex(logdev->physical_device->lock);
+    if (logdev && SDL_AtomicGet(&logdev->paused)) {
+        retval = SDL_TRUE;
     }
+    ReleaseAudioDevice(device);
     return retval;
 }
 
 int SDL_SetAudioPostmixCallback(SDL_AudioDeviceID devid, SDL_AudioPostmixCallback callback, void *userdata)
 {
-    SDL_LogicalAudioDevice *logdev = ObtainLogicalAudioDevice(devid);
+    SDL_AudioDevice *device = NULL;
+    SDL_LogicalAudioDevice *logdev = ObtainLogicalAudioDevice(devid, &device);
     int retval = 0;
     if (logdev) {
-        SDL_AudioDevice *device = logdev->physical_device;
         if (callback && !device->postmix_buffer) {
             device->postmix_buffer = (float *)SDL_aligned_alloc(SDL_SIMDGetAlignment(), device->work_buffer_size);
             if (device->postmix_buffer == NULL) {
@@ -1644,15 +1691,17 @@ int SDL_SetAudioPostmixCallback(SDL_AudioDeviceID devid, SDL_AudioPostmixCallbac
         }
 
         UpdateAudioStreamFormatsPhysical(device);
-        SDL_UnlockMutex(device->lock);
     }
+    ReleaseAudioDevice(device);
     return retval;
 }
 
 int SDL_BindAudioStreams(SDL_AudioDeviceID devid, SDL_AudioStream **streams, int num_streams)
 {
     const SDL_bool islogical = (devid & (1<<1)) ? SDL_FALSE : SDL_TRUE;
-    SDL_LogicalAudioDevice *logdev;
+    SDL_AudioDevice *device = NULL;
+    SDL_LogicalAudioDevice *logdev = NULL;
+    int retval = 0;
 
     if (num_streams == 0) {
         return 0;  // nothing to do
@@ -1662,49 +1711,49 @@ int SDL_BindAudioStreams(SDL_AudioDeviceID devid, SDL_AudioStream **streams, int
         return SDL_InvalidParamError("streams");
     } else if (!islogical) {
         return SDL_SetError("Audio streams are bound to device ids from SDL_OpenAudioDevice, not raw physical devices");
-    } else if ((logdev = ObtainLogicalAudioDevice(devid)) == NULL) {
-        return -1;  // ObtainLogicalAudioDevice set the error message.
-    } else if (logdev->simplified) {
-        SDL_UnlockMutex(logdev->physical_device->lock);
-        return SDL_SetError("Cannot change stream bindings on device opened with SDL_OpenAudioDeviceStream");
     }
 
-    // !!! FIXME: We'll set the device's side's format below, but maybe we should refuse to bind a stream if the app's side doesn't have a format set yet.
-    // !!! FIXME: Actually, why do we allow there to be an invalid format, again?
+    logdev = ObtainLogicalAudioDevice(devid, &device);
+    if (!logdev) {
+        retval = -1;  // ObtainLogicalAudioDevice set the error string.
+    } else if (logdev->simplified) {
+        retval = SDL_SetError("Cannot change stream bindings on device opened with SDL_OpenAudioDeviceStream");
+    } else {
 
-    // make sure start of list is sane.
-    SDL_assert(!logdev->bound_streams || (logdev->bound_streams->prev_binding == NULL));
+        // !!! FIXME: We'll set the device's side's format below, but maybe we should refuse to bind a stream if the app's side doesn't have a format set yet.
+        // !!! FIXME: Actually, why do we allow there to be an invalid format, again?
 
-    SDL_AudioDevice *device = logdev->physical_device;
-    const SDL_bool iscapture = device->iscapture;
-    int retval = 0;
+        // make sure start of list is sane.
+        SDL_assert(!logdev->bound_streams || (logdev->bound_streams->prev_binding == NULL));
 
-    // lock all the streams upfront, so we can verify they aren't bound elsewhere and add them all in one block, as this is intended to add everything or nothing.
-    for (int i = 0; i < num_streams; i++) {
-        SDL_AudioStream *stream = streams[i];
-        if (stream == NULL) {
-            retval = SDL_SetError("Stream #%d is NULL", i);
-        } else {
-            SDL_LockMutex(stream->lock);
-            SDL_assert((stream->bound_device == NULL) == ((stream->prev_binding == NULL) || (stream->next_binding == NULL)));
-            if (stream->bound_device) {
-                retval = SDL_SetError("Stream #%d is already bound to a device", i);
-            } else if (stream->simplified) {  // You can get here if you closed the device instead of destroying the stream.
-                retval = SDL_SetError("Cannot change binding on a stream created with SDL_OpenAudioDeviceStream");
+        // lock all the streams upfront, so we can verify they aren't bound elsewhere and add them all in one block, as this is intended to add everything or nothing.
+        for (int i = 0; i < num_streams; i++) {
+            SDL_AudioStream *stream = streams[i];
+            if (stream == NULL) {
+                retval = SDL_SetError("Stream #%d is NULL", i);
+            } else {
+                SDL_LockMutex(stream->lock);
+                SDL_assert((stream->bound_device == NULL) == ((stream->prev_binding == NULL) || (stream->next_binding == NULL)));
+                if (stream->bound_device) {
+                    retval = SDL_SetError("Stream #%d is already bound to a device", i);
+                } else if (stream->simplified) {  // You can get here if you closed the device instead of destroying the stream.
+                    retval = SDL_SetError("Cannot change binding on a stream created with SDL_

(Patch may be truncated, please check the link at the top of this post.)