Forráskód Böngészése

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).
Ryan C. Gordon 1 éve
szülő
commit
0e614d9179
1 módosított fájl, 345 hozzáadás és 284 törlés
  1. 345 284
      src/audio/SDL_audio.c

+ 345 - 284
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_OpenAudioDeviceStream");
+                }
             }
-        }
 
-        if (retval != 0) {
-            int j;
-            for (j = 0; j <= i; j++) {
-                SDL_UnlockMutex(streams[j]->lock);
+            if (retval != 0) {
+                int j;
+                for (j = 0; j <= i; j++) {
+                    SDL_UnlockMutex(streams[j]->lock);
+                }
+                break;
             }
-            break;
         }
     }
 
     if (retval == 0) {
         // Now that everything is verified, chain everything together.
+        const SDL_bool iscapture = device->iscapture;
         for (int i = 0; i < num_streams; i++) {
             SDL_AudioStream *stream = streams[i];
 
@@ -1729,7 +1778,7 @@ int SDL_BindAudioStreams(SDL_AudioDeviceID devid, SDL_AudioStream **streams, int
 
     UpdateAudioStreamFormatsPhysical(device);
 
-    SDL_UnlockMutex(device->lock);
+    ReleaseAudioDevice(device);
 
     return retval;
 }
@@ -1739,6 +1788,7 @@ int SDL_BindAudioStream(SDL_AudioDeviceID devid, SDL_AudioStream *stream)
     return SDL_BindAudioStreams(devid, &stream, 1);
 }
 
+// !!! FIXME: this and BindAudioStreams are mutex nightmares.  :/
 void SDL_UnbindAudioStreams(SDL_AudioStream **streams, int num_streams)
 {
     /* to prevent deadlock when holding both locks, we _must_ lock the device first, and the stream second, as that is the order the audio thread will do it.
@@ -1831,51 +1881,51 @@ SDL_AudioStream *SDL_OpenAudioDeviceStream(SDL_AudioDeviceID devid, const SDL_Au
         return NULL;  // error string should already be set.
     }
 
-    SDL_LogicalAudioDevice *logdev = ObtainLogicalAudioDevice(logdevid);
+    SDL_bool failed = SDL_FALSE;
+    SDL_AudioStream *stream = NULL;
+    SDL_AudioDevice *device = NULL;
+    SDL_LogicalAudioDevice *logdev = ObtainLogicalAudioDevice(logdevid, &device);
     if (logdev == NULL) { // this shouldn't happen, but just in case.
-        SDL_CloseAudioDevice(logdevid);
-        return NULL;  // error string should already be set.
-    }
-
-    SDL_AudioDevice *physdevice = logdev->physical_device;
-    SDL_assert(physdevice != NULL);
+        failed = SDL_TRUE;
+    } else {
+        SDL_AtomicSet(&logdev->paused, 1);   // start the device paused, to match SDL2.
 
-    SDL_AtomicSet(&logdev->paused, 1);   // start the device paused, to match SDL2.
-    SDL_UnlockMutex(physdevice->lock);  // we don't need to hold the lock for any of this.
+        SDL_assert(device != NULL);
+        const SDL_bool iscapture = device->iscapture;
 
-    const SDL_bool iscapture = physdevice->iscapture;
+        if (iscapture) {
+            stream = SDL_CreateAudioStream(&device->spec, spec);
+        } else {
+            stream = SDL_CreateAudioStream(spec, &device->spec);
+        }
 
-    SDL_AudioStream *stream = NULL;
-    if (iscapture) {
-        stream = SDL_CreateAudioStream(&physdevice->spec, spec);
-    } else {
-        stream = SDL_CreateAudioStream(spec, &physdevice->spec);
+        if (!stream || (SDL_BindAudioStream(logdevid, stream) == -1)) {
+            failed = SDL_TRUE;
+        } else {
+            logdev->simplified = SDL_TRUE;  // forbid further binding changes on this logical device.
+            stream->simplified = SDL_TRUE;  // so we know to close the audio device when this is destroyed.
+
+            if (callback) {
+                int rc;
+                if (iscapture) {
+                    rc = SDL_SetAudioStreamPutCallback(stream, callback, userdata);
+                } else {
+                    rc = SDL_SetAudioStreamGetCallback(stream, callback, userdata);
+                }
+                SDL_assert(rc == 0);  // should only fail if stream==NULL atm.
+            }
+        }
     }
 
-    if (!stream) {
-        SDL_CloseAudioDevice(logdevid);
-        return NULL;  // error string should already be set.
-    }
-    if (SDL_BindAudioStream(logdevid, stream) == -1) {
+    ReleaseAudioDevice(device);
+
+    if (failed) {
         SDL_DestroyAudioStream(stream);
         SDL_CloseAudioDevice(logdevid);
-        return NULL;  // error string should already be set.
-    }
-
-    logdev->simplified = SDL_TRUE;  // forbid further binding changes on this logical device.
-    stream->simplified = SDL_TRUE;  // so we know to close the audio device when this is destroyed.
-
-    if (callback) {
-        int rc;
-        if (iscapture) {
-            rc = SDL_SetAudioStreamPutCallback(stream, callback, userdata);
-        } else {
-            rc = SDL_SetAudioStreamGetCallback(stream, callback, userdata);
-        }
-        SDL_assert(rc == 0);  // should only fail if stream==NULL atm.
+        stream = NULL;
     }
 
-    return stream;   // ready to rock.
+    return stream;
 }
 
 #define NUM_FORMATS 8
@@ -1913,9 +1963,21 @@ void SDL_DefaultAudioDeviceChanged(SDL_AudioDevice *new_default_device)
     }
 
     const SDL_bool iscapture = new_default_device->iscapture;
+
+    // change the official default over right away, so new opens will go to the new device.
+    SDL_LockRWLockForWriting(current_audio.device_hash_lock);
     const SDL_AudioDeviceID current_devid = iscapture ? current_audio.default_capture_device_id : current_audio.default_output_device_id;
+    const SDL_bool is_already_default = (new_default_device->instance_id == current_devid) ? SDL_TRUE : SDL_FALSE;
+    if (!is_already_default) {
+        if (iscapture) {
+            current_audio.default_capture_device_id = new_default_device->instance_id;
+        } else {
+            current_audio.default_output_device_id = new_default_device->instance_id;
+        }
+    }
+    SDL_UnlockRWLock(current_audio.device_hash_lock);
 
-    if (new_default_device->instance_id == current_devid) {
+    if (is_already_default) {
         return;  // this is already the default.
     }
 
@@ -1926,17 +1988,12 @@ void SDL_DefaultAudioDeviceChanged(SDL_AudioDevice *new_default_device)
     pending.next = NULL;
     SDL_PendingAudioDeviceEvent *pending_tail = &pending;
 
-    SDL_LockMutex(new_default_device->lock);
+    // Default device gets an extra ref, so it lives until a new default replaces it, even if disconnected.
+    RefPhysicalAudioDevice(new_default_device);
 
-    SDL_AudioDevice *current_default_device = ObtainPhysicalAudioDevice(current_devid);
+    ObtainPhysicalAudioDeviceObj(new_default_device);
 
-    /* change the official default ID over while we have locks on both devices, so if something raced to open the default during
-       this, it either gets the new device or is ready on the old and can be migrated. */
-    if (iscapture) {
-        current_audio.default_capture_device_id = new_default_device->instance_id;
-    } else {
-        current_audio.default_output_device_id = new_default_device->instance_id;
-    }
+    SDL_AudioDevice *current_default_device = ObtainPhysicalAudioDevice(current_devid);
 
     if (current_default_device) {
         // migrate any logical devices that were opened as a default to the new physical device...
@@ -1984,7 +2041,8 @@ void SDL_DefaultAudioDeviceChanged(SDL_AudioDevice *new_default_device)
                     continue;  // not opened as a default, leave it on the current physical device.
                 }
 
-                // now migrate the logical device.
+                // now migrate the logical device. Hold device_hash_lock so ObtainLogicalAudioDevice doesn't get a device in the middle of transition.
+                SDL_LockRWLockForWriting(current_audio.device_hash_lock);
                 if (logdev->next) {
                     logdev->next->prev = logdev->prev;
                 }
@@ -1999,6 +2057,7 @@ void SDL_DefaultAudioDeviceChanged(SDL_AudioDevice *new_default_device)
                 logdev->prev = NULL;
                 logdev->next = new_default_device->logical_devices;
                 new_default_device->logical_devices = logdev;
+                SDL_UnlockRWLock(current_audio.device_hash_lock);
 
                 SDL_assert(SDL_AtomicGet(&current_default_device->refcount) > 1);  // we should hold at least one extra reference to this device, beyond logical devices, during this phase...
                 RefPhysicalAudioDevice(new_default_device);
@@ -2024,19 +2083,21 @@ void SDL_DefaultAudioDeviceChanged(SDL_AudioDevice *new_default_device)
 
             if (current_default_device->logical_devices == NULL) {   // nothing left on the current physical device, close it.
                 // !!! 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(current_default_device->lock);  // can't hold the lock or the audio thread will deadlock while we WaitThread it.
+                RefPhysicalAudioDevice(current_default_device);  // hold a temp ref for a moment while we release...
+                ReleaseAudioDevice(current_default_device);  // can't hold the lock or the audio thread will deadlock while we WaitThread it.
                 ClosePhysicalAudioDevice(current_default_device);
-                SDL_LockMutex(current_default_device->lock);  // we're about to unlock this again, so make sure the locks match.
+                ObtainPhysicalAudioDeviceObj(current_default_device);  // we're about to unlock this again, so make sure the locks match.
+                UnrefPhysicalAudioDevice(current_default_device);  // drop temp ref.
             }
         }
 
-        SDL_UnlockMutex(current_default_device->lock);
+        ReleaseAudioDevice(current_default_device);
     }
 
-    SDL_UnlockMutex(new_default_device->lock);
+    ReleaseAudioDevice(new_default_device);
 
-    // was current device already dead and just kept around to migrate to a new default device? Now we can kill it. Aim for the brain.
-    if (current_default_device && SDL_AtomicGet(&current_default_device->zombie)) {
+    // Default device gets an extra ref, so it lives until a new default replaces it, even if disconnected.
+    if (current_default_device) {  // (despite the name, it's no longer current at this point)
         UnrefPhysicalAudioDevice(current_default_device);
     }
 
@@ -2136,9 +2197,9 @@ int SDL_AudioDeviceFormatChangedAlreadyLocked(SDL_AudioDevice *device, const SDL
 
 int SDL_AudioDeviceFormatChanged(SDL_AudioDevice *device, const SDL_AudioSpec *newspec, int new_sample_frames)
 {
-    SDL_LockMutex(device->lock);
+    ObtainPhysicalAudioDeviceObj(device);
     const int retval = SDL_AudioDeviceFormatChangedAlreadyLocked(device, newspec, new_sample_frames);
-    SDL_UnlockMutex(device->lock);
+    ReleaseAudioDevice(device);
     return retval;
 }