Parcourir la source

SDL_HashTable is now optionally thread-safe

Fixes https://github.com/libsdl-org/SDL/issues/11635
Sam Lantinga il y a 4 mois
Parent
commit
61511c48a4

+ 80 - 18
src/SDL_hashtable.c

@@ -44,6 +44,7 @@ SDL_COMPILE_TIME_ASSERT(sizeof_SDL_HashItem, sizeof(SDL_HashItem) <= MAX_HASHITE
 
 struct SDL_HashTable
 {
+    SDL_RWLock *lock;
     SDL_HashItem *table;
     SDL_HashTable_HashFn hash;
     SDL_HashTable_KeyMatchFn keymatch;
@@ -60,6 +61,7 @@ SDL_HashTable *SDL_CreateHashTable(void *data,
                                    SDL_HashTable_HashFn hashfn,
                                    SDL_HashTable_KeyMatchFn keymatchfn,
                                    SDL_HashTable_NukeFn nukefn,
+                                   bool threadsafe,
                                    bool stackable)
 {
     SDL_HashTable *table;
@@ -80,9 +82,14 @@ SDL_HashTable *SDL_CreateHashTable(void *data,
         return NULL;
     }
 
+    if (threadsafe) {
+        // Don't fail if we can't create a lock (single threaded environment?)
+        table->lock = SDL_CreateRWLock();
+    }
+
     table->table = (SDL_HashItem *)SDL_calloc(num_buckets, sizeof(SDL_HashItem));
     if (!table->table) {
-        SDL_free(table);
+        SDL_DestroyHashTable(table);
         return NULL;
     }
 
@@ -289,17 +296,22 @@ bool SDL_InsertIntoHashTable(SDL_HashTable *table, const void *key, const void *
 {
     SDL_HashItem *item;
     Uint32 hash;
+    bool result = false;
 
     if (!table) {
         return false;
     }
 
+    if (table->lock) {
+        SDL_LockRWLockForWriting(table->lock);
+    }
+
     hash = calc_hash(table, key);
     item = find_first_item(table, key, hash);
 
     if (item && !table->stackable) {
-        // TODO: Maybe allow overwrites? We could do it more efficiently here than unset followed by insert.
-        return false;
+        // Allow overwrites, this might have been inserted on another thread
+        delete_item(table, item);
     }
 
     SDL_HashItem new_item;
@@ -313,18 +325,25 @@ bool SDL_InsertIntoHashTable(SDL_HashTable *table, const void *key, const void *
 
     if (!maybe_resize(table)) {
         table->num_occupied_slots--;
-        return false;
+        goto done;
     }
 
     // This never returns NULL
     insert_item(&new_item, table->table, table->hash_mask, &table->max_probe_len);
-    return true;
+    result = true;
+
+done:
+    if (table->lock) {
+        SDL_UnlockRWLock(table->lock);
+    }
+    return result;
 }
 
 bool SDL_FindInHashTable(const SDL_HashTable *table, const void *key, const void **value)
 {
     Uint32 hash;
     SDL_HashItem *i;
+    bool result = false;
 
     if (!table) {
         if (value) {
@@ -333,26 +352,39 @@ bool SDL_FindInHashTable(const SDL_HashTable *table, const void *key, const void
         return false;
     }
 
+    if (table->lock) {
+        SDL_LockRWLockForReading(table->lock);
+    }
+
     hash = calc_hash(table, key);
     i = find_first_item(table, key, hash);
     if (i) {
         if (value) {
             *value = i->value;
         }
-        return true;
+        result = true;
     }
-    return false;
+
+    if (table->lock) {
+        SDL_UnlockRWLock(table->lock);
+    }
+    return result;
 }
 
 bool SDL_RemoveFromHashTable(SDL_HashTable *table, const void *key)
 {
     Uint32 hash;
     SDL_HashItem *item;
+    bool result = false;
 
     if (!table) {
         return false;
     }
 
+    if (table->lock) {
+        SDL_LockRWLockForWriting(table->lock);
+    }
+
     // FIXME: what to do for stacking hashtables?
     // The original code removes just one item.
     // This hashtable happens to preserve the insertion order of multi-value keys,
@@ -361,13 +393,18 @@ bool SDL_RemoveFromHashTable(SDL_HashTable *table, const void *key)
 
     hash = calc_hash(table, key);
     item = find_first_item(table, key, hash);
-
     if (!item) {
-        return false;
+        goto done;
     }
 
     delete_item(table, item);
-    return true;
+    result = true;
+
+done:
+    if (table->lock) {
+        SDL_UnlockRWLock(table->lock);
+    }
+    return result;
 }
 
 bool SDL_IterateHashTableKey(const SDL_HashTable *table, const void *key, const void **_value, void **iter)
@@ -456,6 +493,28 @@ bool SDL_HashTableEmpty(SDL_HashTable *table)
     return !(table && table->num_occupied_slots);
 }
 
+void SDL_LockHashTable(SDL_HashTable *table, bool for_writing)
+{
+    if (!table) {
+        return;
+    }
+
+    if (for_writing) {
+        SDL_LockRWLockForWriting(table->lock);
+    } else {
+        SDL_LockRWLockForReading(table->lock);
+    }
+}
+
+void SDL_UnlockHashTable(SDL_HashTable *table)
+{
+    if (!table) {
+        return;
+    }
+
+    SDL_UnlockRWLock(table->lock);
+}
+
 static void nuke_all(SDL_HashTable *table)
 {
     void *data = table->data;
@@ -472,22 +531,25 @@ static void nuke_all(SDL_HashTable *table)
 void SDL_EmptyHashTable(SDL_HashTable *table)
 {
     if (table) {
-        if (table->nuke) {
-            nuke_all(table);
-        }
+        SDL_LockRWLockForWriting(table->lock);
+        {
+            if (table->nuke) {
+                nuke_all(table);
+            }
 
-        SDL_memset(table->table, 0, sizeof(*table->table) * (table->hash_mask + 1));
-        table->num_occupied_slots = 0;
+            SDL_memset(table->table, 0, sizeof(*table->table) * (table->hash_mask + 1));
+            table->num_occupied_slots = 0;
+        }
+        SDL_UnlockRWLock(table->lock);
     }
 }
 
 void SDL_DestroyHashTable(SDL_HashTable *table)
 {
     if (table) {
-        if (table->nuke) {
-            nuke_all(table);
-        }
+        SDL_EmptyHashTable(table);
 
+        SDL_DestroyRWLock(table->lock);
         SDL_free(table->table);
         SDL_free(table);
     }

+ 17 - 0
src/SDL_hashtable.h

@@ -34,19 +34,36 @@ extern SDL_HashTable *SDL_CreateHashTable(void *data,
                                           SDL_HashTable_HashFn hashfn,
                                           SDL_HashTable_KeyMatchFn keymatchfn,
                                           SDL_HashTable_NukeFn nukefn,
+                                          bool threadsafe,
                                           bool stackable);
 
+// This function is thread-safe if the hashtable was created with threadsafe = true
 extern void SDL_EmptyHashTable(SDL_HashTable *table);
+
+// This function is not thread-safe.
 extern void SDL_DestroyHashTable(SDL_HashTable *table);
+
+// This function is thread-safe if the hashtable was created with threadsafe = true
 extern bool SDL_InsertIntoHashTable(SDL_HashTable *table, const void *key, const void *value);
+
+// This function is thread-safe if the hashtable was created with threadsafe = true
 extern bool SDL_RemoveFromHashTable(SDL_HashTable *table, const void *key);
+
+// This function is thread-safe if the hashtable was created with threadsafe = true
 extern bool SDL_FindInHashTable(const SDL_HashTable *table, const void *key, const void **_value);
+
+// This function is thread-safe if the hashtable was created with threadsafe = true
 extern bool SDL_HashTableEmpty(SDL_HashTable *table);
 
+extern void SDL_LockHashTable(SDL_HashTable *table, bool for_writing);
+extern void SDL_UnlockHashTable(SDL_HashTable *table);
+
 // iterate all values for a specific key. This only makes sense if the hash is stackable. If not-stackable, just use SDL_FindInHashTable().
+// This function is not thread-safe, you should use SDL_LockHashTable() if necessary
 extern bool SDL_IterateHashTableKey(const SDL_HashTable *table, const void *key, const void **_value, void **iter);
 
 // iterate all key/value pairs in a hash (stackable hashes can have duplicate keys with multiple values).
+// This function is not thread-safe, you should use SDL_LockHashTable() if necessary
 extern bool SDL_IterateHashTable(const SDL_HashTable *table, const void **_key, const void **_value, void **iter);
 
 extern Uint32 SDL_HashPointer(const void *key, void *unused);

+ 11 - 52
src/SDL_properties.c

@@ -50,8 +50,7 @@ typedef struct
 
 static SDL_InitState SDL_properties_init;
 static SDL_HashTable *SDL_properties;
-static SDL_Mutex *SDL_properties_lock;
-static SDL_PropertiesID SDL_last_properties_id;
+static SDL_AtomicU32 SDL_last_properties_id;
 static SDL_AtomicU32 SDL_global_properties;
 
 
@@ -104,10 +103,7 @@ bool SDL_InitProperties(void)
         return true;
     }
 
-    // If this fails we'll continue without it.
-    SDL_properties_lock = SDL_CreateMutex();
-
-    SDL_properties = SDL_CreateHashTable(NULL, 16, SDL_HashID, SDL_KeyMatchID, SDL_FreeProperties, false);
+    SDL_properties = SDL_CreateHashTable(NULL, 16, SDL_HashID, SDL_KeyMatchID, SDL_FreeProperties, true, false);
     if (!SDL_properties) {
         goto error;
     }
@@ -140,10 +136,6 @@ void SDL_QuitProperties(void)
         SDL_DestroyHashTable(SDL_properties);
         SDL_properties = NULL;
     }
-    if (SDL_properties_lock) {
-        SDL_DestroyMutex(SDL_properties_lock);
-        SDL_properties_lock = NULL;
-    }
 
     SDL_SetInitialized(&SDL_properties_init, false);
 }
@@ -181,7 +173,7 @@ SDL_PropertiesID SDL_CreateProperties(void)
     if (!properties) {
         goto error;
     }
-    properties->props = SDL_CreateHashTable(NULL, 4, SDL_HashString, SDL_KeyMatchString, SDL_FreeProperty, false);
+    properties->props = SDL_CreateHashTable(NULL, 4, SDL_HashString, SDL_KeyMatchString, SDL_FreeProperty, false, false);
     if (!properties->props) {
         goto error;
     }
@@ -189,16 +181,18 @@ SDL_PropertiesID SDL_CreateProperties(void)
     // If this fails we'll continue without it.
     properties->lock = SDL_CreateMutex();
 
-    SDL_LockMutex(SDL_properties_lock);
-    ++SDL_last_properties_id;
-    if (SDL_last_properties_id == 0) {
-        ++SDL_last_properties_id;
+    for ( ; ; ) {
+        props = (SDL_GetAtomicU32(&SDL_last_properties_id) + 1);
+        if (props == 0) {
+            continue;
+        }
+        if (SDL_CompareAndSwapAtomicU32(&SDL_last_properties_id, props - 1, props)) {
+            break;
+        }
     }
-    props = SDL_last_properties_id;
     if (SDL_InsertIntoHashTable(SDL_properties, (const void *)(uintptr_t)props, properties)) {
         inserted = true;
     }
-    SDL_UnlockMutex(SDL_properties_lock);
 
     if (inserted) {
         // All done!
@@ -223,11 +217,8 @@ bool SDL_CopyProperties(SDL_PropertiesID src, SDL_PropertiesID dst)
         return SDL_InvalidParamError("dst");
     }
 
-    SDL_LockMutex(SDL_properties_lock);
     SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)src, (const void **)&src_properties);
     SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)dst, (const void **)&dst_properties);
-    SDL_UnlockMutex(SDL_properties_lock);
-
     if (!src_properties) {
         return SDL_InvalidParamError("src");
     }
@@ -290,10 +281,7 @@ bool SDL_LockProperties(SDL_PropertiesID props)
         return SDL_InvalidParamError("props");
     }
 
-    SDL_LockMutex(SDL_properties_lock);
     SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)props, (const void **)&properties);
-    SDL_UnlockMutex(SDL_properties_lock);
-
     if (!properties) {
         return SDL_InvalidParamError("props");
     }
@@ -310,10 +298,7 @@ void SDL_UnlockProperties(SDL_PropertiesID props)
         return;
     }
 
-    SDL_LockMutex(SDL_properties_lock);
     SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)props, (const void **)&properties);
-    SDL_UnlockMutex(SDL_properties_lock);
-
     if (!properties) {
         return;
     }
@@ -335,10 +320,7 @@ static bool SDL_PrivateSetProperty(SDL_PropertiesID props, const char *name, SDL
         return SDL_InvalidParamError("name");
     }
 
-    SDL_LockMutex(SDL_properties_lock);
     SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)props, (const void **)&properties);
-    SDL_UnlockMutex(SDL_properties_lock);
-
     if (!properties) {
         SDL_FreePropertyWithCleanup(NULL, property, NULL, true);
         return SDL_InvalidParamError("props");
@@ -496,10 +478,7 @@ SDL_PropertyType SDL_GetPropertyType(SDL_PropertiesID props, const char *name)
         return SDL_PROPERTY_TYPE_INVALID;
     }
 
-    SDL_LockMutex(SDL_properties_lock);
     SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)props, (const void **)&properties);
-    SDL_UnlockMutex(SDL_properties_lock);
-
     if (!properties) {
         return SDL_PROPERTY_TYPE_INVALID;
     }
@@ -528,10 +507,7 @@ void *SDL_GetPointerProperty(SDL_PropertiesID props, const char *name, void *def
         return value;
     }
 
-    SDL_LockMutex(SDL_properties_lock);
     SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)props, (const void **)&properties);
-    SDL_UnlockMutex(SDL_properties_lock);
-
     if (!properties) {
         return value;
     }
@@ -566,10 +542,7 @@ const char *SDL_GetStringProperty(SDL_PropertiesID props, const char *name, cons
         return value;
     }
 
-    SDL_LockMutex(SDL_properties_lock);
     SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)props, (const void **)&properties);
-    SDL_UnlockMutex(SDL_properties_lock);
-
     if (!properties) {
         return value;
     }
@@ -627,10 +600,7 @@ Sint64 SDL_GetNumberProperty(SDL_PropertiesID props, const char *name, Sint64 de
         return value;
     }
 
-    SDL_LockMutex(SDL_properties_lock);
     SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)props, (const void **)&properties);
-    SDL_UnlockMutex(SDL_properties_lock);
-
     if (!properties) {
         return value;
     }
@@ -674,10 +644,7 @@ float SDL_GetFloatProperty(SDL_PropertiesID props, const char *name, float defau
         return value;
     }
 
-    SDL_LockMutex(SDL_properties_lock);
     SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)props, (const void **)&properties);
-    SDL_UnlockMutex(SDL_properties_lock);
-
     if (!properties) {
         return value;
     }
@@ -721,10 +688,7 @@ bool SDL_GetBooleanProperty(SDL_PropertiesID props, const char *name, bool defau
         return value;
     }
 
-    SDL_LockMutex(SDL_properties_lock);
     SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)props, (const void **)&properties);
-    SDL_UnlockMutex(SDL_properties_lock);
-
     if (!properties) {
         return value;
     }
@@ -772,10 +736,7 @@ bool SDL_EnumerateProperties(SDL_PropertiesID props, SDL_EnumeratePropertiesCall
         return SDL_InvalidParamError("callback");
     }
 
-    SDL_LockMutex(SDL_properties_lock);
     SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)props, (const void **)&properties);
-    SDL_UnlockMutex(SDL_properties_lock);
-
     if (!properties) {
         return SDL_InvalidParamError("props");
     }
@@ -833,7 +794,5 @@ void SDL_DestroyProperties(SDL_PropertiesID props)
         return;
     }
 
-    SDL_LockMutex(SDL_properties_lock);
     SDL_RemoveFromHashTable(SDL_properties, (const void *)(uintptr_t)props);
-    SDL_UnlockMutex(SDL_properties_lock);
 }

+ 12 - 7
src/SDL_utils.c

@@ -132,6 +132,7 @@ Uint32 SDL_GetNextObjectID(void)
     return id;
 }
 
+static SDL_InitState SDL_objects_init;
 static SDL_HashTable *SDL_objects;
 
 static Uint32 SDL_HashObject(const void *key, void *unused)
@@ -148,16 +149,18 @@ void SDL_SetObjectValid(void *object, SDL_ObjectType type, bool valid)
 {
     SDL_assert(object != NULL);
 
-    if (valid) {
+    if (valid && SDL_ShouldInit(&SDL_objects_init)) {
+        SDL_objects = SDL_CreateHashTable(NULL, 32, SDL_HashObject, SDL_KeyMatchObject, NULL, true, false);
         if (!SDL_objects) {
-            SDL_objects = SDL_CreateHashTable(NULL, 32, SDL_HashObject, SDL_KeyMatchObject, NULL, false);
+            SDL_SetInitialized(&SDL_objects_init, false);
         }
+        SDL_SetInitialized(&SDL_objects_init, true);
+    }
 
+    if (valid) {
         SDL_InsertIntoHashTable(SDL_objects, object, (void *)(uintptr_t)type);
     } else {
-        if (SDL_objects) {
-            SDL_RemoveFromHashTable(SDL_objects, object);
-        }
+        SDL_RemoveFromHashTable(SDL_objects, object);
     }
 }
 
@@ -177,7 +180,7 @@ bool SDL_ObjectValid(void *object, SDL_ObjectType type)
 
 void SDL_SetObjectsInvalid(void)
 {
-    if (SDL_objects) {
+    if (SDL_ShouldQuit(&SDL_objects_init)) {
         // Log any leaked objects
         const void *object, *object_type;
         void *iter = NULL;
@@ -221,6 +224,8 @@ void SDL_SetObjectsInvalid(void)
 
         SDL_DestroyHashTable(SDL_objects);
         SDL_objects = NULL;
+
+        SDL_SetInitialized(&SDL_objects_init, false);
     }
 }
 
@@ -357,7 +362,7 @@ const char *SDL_GetPersistentString(const char *string)
 
     SDL_HashTable *strings = (SDL_HashTable *)SDL_GetTLS(&SDL_string_storage);
     if (!strings) {
-        strings = SDL_CreateHashTable(NULL, 32, SDL_HashString, SDL_KeyMatchString, SDL_NukeFreeValue, false);
+        strings = SDL_CreateHashTable(NULL, 32, SDL_HashString, SDL_KeyMatchString, SDL_NukeFreeValue, false, false);
         if (!strings) {
             return NULL;
         }

+ 1 - 1
src/audio/SDL_audio.c

@@ -909,7 +909,7 @@ bool SDL_InitAudio(const char *driver_name)
         return false;
     }
 
-    SDL_HashTable *device_hash = SDL_CreateHashTable(NULL, 8, HashAudioDeviceID, MatchAudioDeviceID, NukeAudioDeviceHashItem, false);
+    SDL_HashTable *device_hash = SDL_CreateHashTable(NULL, 8, HashAudioDeviceID, MatchAudioDeviceID, NukeAudioDeviceHashItem, false, false);
     if (!device_hash) {
         SDL_DestroyRWLock(device_hash_lock);
         return false;

+ 19 - 37
src/camera/SDL_camera.c

@@ -299,11 +299,9 @@ void UnrefPhysicalCamera(SDL_Camera *device)
 {
     if (SDL_AtomicDecRef(&device->refcount)) {
         // take it out of the device list.
-        SDL_LockRWLockForWriting(camera_driver.device_hash_lock);
         if (SDL_RemoveFromHashTable(camera_driver.device_hash, (const void *) (uintptr_t) device->instance_id)) {
             SDL_AddAtomicInt(&camera_driver.device_count, -1);
         }
-        SDL_UnlockRWLock(camera_driver.device_hash_lock);
         DestroyPhysicalCamera(device);  // ...and nuke it.
     }
 }
@@ -329,9 +327,7 @@ static SDL_Camera *ObtainPhysicalCamera(SDL_CameraID devid)  // !!! FIXME: SDL_A
     }
 
     SDL_Camera *device = NULL;
-    SDL_LockRWLockForReading(camera_driver.device_hash_lock);
     SDL_FindInHashTable(camera_driver.device_hash, (const void *) (uintptr_t) devid, (const void **) &device);
-    SDL_UnlockRWLock(camera_driver.device_hash_lock);
     if (!device) {
         SDL_SetError("Invalid camera device instance ID");
     } else {
@@ -424,9 +420,7 @@ SDL_Camera *SDL_AddCamera(const char *name, SDL_CameraPosition position, int num
     SDL_assert((specs != NULL) == (num_specs > 0));
     SDL_assert(handle != NULL);
 
-    SDL_LockRWLockForReading(camera_driver.device_hash_lock);
     const int shutting_down = SDL_GetAtomicInt(&camera_driver.shutting_down);
-    SDL_UnlockRWLock(camera_driver.device_hash_lock);
     if (shutting_down) {
         return NULL;  // we're shutting down, don't add any devices that are hotplugged at the last possible moment.
     }
@@ -496,7 +490,6 @@ SDL_Camera *SDL_AddCamera(const char *name, SDL_CameraPosition position, int num
     SDL_SetAtomicInt(&device->zombie, 0);
     RefPhysicalCamera(device);
 
-    SDL_LockRWLockForWriting(camera_driver.device_hash_lock);
     if (SDL_InsertIntoHashTable(camera_driver.device_hash, (const void *) (uintptr_t) device->instance_id, device)) {
         SDL_AddAtomicInt(&camera_driver.device_count, 1);
     } else {
@@ -514,13 +507,14 @@ SDL_Camera *SDL_AddCamera(const char *name, SDL_CameraPosition position, int num
             p->type = SDL_EVENT_CAMERA_DEVICE_ADDED;
             p->devid = device->instance_id;
             p->next = NULL;
+            SDL_LockHashTable(camera_driver.device_hash, true);
             SDL_assert(camera_driver.pending_events_tail != NULL);
             SDL_assert(camera_driver.pending_events_tail->next == NULL);
             camera_driver.pending_events_tail->next = p;
             camera_driver.pending_events_tail = p;
+            SDL_UnlockHashTable(camera_driver.device_hash);
         }
     }
-    SDL_UnlockRWLock(camera_driver.device_hash_lock);
 
     return device;
 }
@@ -574,12 +568,12 @@ void SDL_CameraDisconnected(SDL_Camera *device)
 
     if (first_disconnect) {
         if (pending.next) {  // NULL if event is disabled or disaster struck.
-            SDL_LockRWLockForWriting(camera_driver.device_hash_lock);
+            SDL_LockHashTable(camera_driver.device_hash, true);
             SDL_assert(camera_driver.pending_events_tail != NULL);
             SDL_assert(camera_driver.pending_events_tail->next == NULL);
             camera_driver.pending_events_tail->next = pending.next;
             camera_driver.pending_events_tail = pending_tail;
-            SDL_UnlockRWLock(camera_driver.device_hash_lock);
+            SDL_UnlockHashTable(camera_driver.device_hash);
         }
     }
 }
@@ -612,12 +606,12 @@ void SDL_CameraPermissionOutcome(SDL_Camera *device, bool approved)
     ReleaseCamera(device);
 
     if (pending.next) {  // NULL if event is disabled or disaster struck.
-        SDL_LockRWLockForWriting(camera_driver.device_hash_lock);
+        SDL_LockHashTable(camera_driver.device_hash, true);
         SDL_assert(camera_driver.pending_events_tail != NULL);
         SDL_assert(camera_driver.pending_events_tail->next == NULL);
         camera_driver.pending_events_tail->next = pending.next;
         camera_driver.pending_events_tail = pending_tail;
-        SDL_UnlockRWLock(camera_driver.device_hash_lock);
+        SDL_UnlockHashTable(camera_driver.device_hash);
     }
 }
 
@@ -633,16 +627,15 @@ SDL_Camera *SDL_FindPhysicalCameraByCallback(bool (*callback)(SDL_Camera *device
     const void *value;
     void *iter = NULL;
 
-    SDL_LockRWLockForReading(camera_driver.device_hash_lock);
+    SDL_LockHashTable(camera_driver.device_hash, false);
     while (SDL_IterateHashTable(camera_driver.device_hash, &key, &value, &iter)) {
         SDL_Camera *device = (SDL_Camera *) value;
         if (callback(device, userdata)) {  // found it?
-            SDL_UnlockRWLock(camera_driver.device_hash_lock);
+            SDL_UnlockHashTable(camera_driver.device_hash);
             return device;
         }
     }
-
-    SDL_UnlockRWLock(camera_driver.device_hash_lock);
+    SDL_UnlockHashTable(camera_driver.device_hash);
 
     SDL_SetError("Device not found");
     return NULL;
@@ -716,7 +709,7 @@ SDL_CameraID *SDL_GetCameras(int *count)
 
     SDL_CameraID *result = NULL;
 
-    SDL_LockRWLockForReading(camera_driver.device_hash_lock);
+    SDL_LockHashTable(camera_driver.device_hash, false);
     int num_devices = SDL_GetAtomicInt(&camera_driver.device_count);
     result = (SDL_CameraID *) SDL_malloc((num_devices + 1) * sizeof (SDL_CameraID));
     if (!result) {
@@ -733,7 +726,7 @@ SDL_CameraID *SDL_GetCameras(int *count)
         SDL_assert(devs_seen == num_devices);
         result[devs_seen] = 0;  // null-terminated.
     }
-    SDL_UnlockRWLock(camera_driver.device_hash_lock);
+    SDL_UnlockHashTable(camera_driver.device_hash);
 
     *count = num_devices;
 
@@ -1363,14 +1356,14 @@ void SDL_QuitCamera(void)
         return;
     }
 
-    SDL_LockRWLockForWriting(camera_driver.device_hash_lock);
-    SDL_SetAtomicInt(&camera_driver.shutting_down, 1);
     SDL_HashTable *device_hash = camera_driver.device_hash;
+    SDL_LockHashTable(device_hash, true);
+    SDL_SetAtomicInt(&camera_driver.shutting_down, 1);
     camera_driver.device_hash = NULL;
     SDL_PendingCameraEvent *pending_events = camera_driver.pending_events.next;
     camera_driver.pending_events.next = NULL;
     SDL_SetAtomicInt(&camera_driver.device_count, 0);
-    SDL_UnlockRWLock(camera_driver.device_hash_lock);
+    SDL_UnlockHashTable(device_hash);
 
     SDL_PendingCameraEvent *pending_next = NULL;
     for (SDL_PendingCameraEvent *i = pending_events; i; i = pending_next) {
@@ -1388,7 +1381,6 @@ void SDL_QuitCamera(void)
     // Free the driver data
     camera_driver.impl.Deinitialize();
 
-    SDL_DestroyRWLock(camera_driver.device_hash_lock);
     SDL_DestroyHashTable(device_hash);
 
     SDL_zero(camera_driver);
@@ -1417,14 +1409,8 @@ bool SDL_CameraInit(const char *driver_name)
         SDL_QuitCamera(); // shutdown driver if already running.
     }
 
-    SDL_RWLock *device_hash_lock = SDL_CreateRWLock();  // create this early, so if it fails we don't have to tear down the whole camera subsystem.
-    if (!device_hash_lock) {
-        return false;
-    }
-
-    SDL_HashTable *device_hash = SDL_CreateHashTable(NULL, 8, HashCameraID, MatchCameraID, NukeCameraHashItem, false);
+    SDL_HashTable *device_hash = SDL_CreateHashTable(NULL, 8, HashCameraID, MatchCameraID, NukeCameraHashItem, true, false);
     if (!device_hash) {
-        SDL_DestroyRWLock(device_hash_lock);
         return false;
     }
 
@@ -1441,7 +1427,6 @@ bool SDL_CameraInit(const char *driver_name)
         const char *driver_attempt = driver_name_copy;
 
         if (!driver_name_copy) {
-            SDL_DestroyRWLock(device_hash_lock);
             SDL_DestroyHashTable(device_hash);
             return false;
         }
@@ -1457,7 +1442,6 @@ bool SDL_CameraInit(const char *driver_name)
                     tried_to_init = true;
                     SDL_zero(camera_driver);
                     camera_driver.pending_events_tail = &camera_driver.pending_events;
-                    camera_driver.device_hash_lock = device_hash_lock;
                     camera_driver.device_hash = device_hash;
                     if (bootstrap[i]->init(&camera_driver.impl)) {
                         camera_driver.name = bootstrap[i]->name;
@@ -1481,7 +1465,6 @@ bool SDL_CameraInit(const char *driver_name)
             tried_to_init = true;
             SDL_zero(camera_driver);
             camera_driver.pending_events_tail = &camera_driver.pending_events;
-            camera_driver.device_hash_lock = device_hash_lock;
             camera_driver.device_hash = device_hash;
             if (bootstrap[i]->init(&camera_driver.impl)) {
                 camera_driver.name = bootstrap[i]->name;
@@ -1502,7 +1485,6 @@ bool SDL_CameraInit(const char *driver_name)
         }
 
         SDL_zero(camera_driver);
-        SDL_DestroyRWLock(device_hash_lock);
         SDL_DestroyHashTable(device_hash);
         return false;  // No driver was available, so fail.
     }
@@ -1519,20 +1501,20 @@ bool SDL_CameraInit(const char *driver_name)
 // ("UpdateSubsystem" is the same naming that the other things that hook into PumpEvents use.)
 void SDL_UpdateCamera(void)
 {
-    SDL_LockRWLockForReading(camera_driver.device_hash_lock);
+    SDL_LockHashTable(camera_driver.device_hash, false);
     SDL_PendingCameraEvent *pending_events = camera_driver.pending_events.next;
-    SDL_UnlockRWLock(camera_driver.device_hash_lock);
+    SDL_UnlockHashTable(camera_driver.device_hash);
 
     if (!pending_events) {
         return;  // nothing to do, check next time.
     }
 
     // okay, let's take this whole list of events so we can dump the lock, and new ones can queue up for a later update.
-    SDL_LockRWLockForWriting(camera_driver.device_hash_lock);
+    SDL_LockHashTable(camera_driver.device_hash, true);
     pending_events = camera_driver.pending_events.next;  // in case this changed...
     camera_driver.pending_events.next = NULL;
     camera_driver.pending_events_tail = &camera_driver.pending_events;
-    SDL_UnlockRWLock(camera_driver.device_hash_lock);
+    SDL_UnlockHashTable(camera_driver.device_hash);
 
     SDL_PendingCameraEvent *pending_next = NULL;
     for (SDL_PendingCameraEvent *i = pending_events; i; i = pending_next) {

+ 2 - 2
src/events/SDL_keymap.c

@@ -39,8 +39,8 @@ SDL_Keymap *SDL_CreateKeymap(void)
         return NULL;
     }
 
-    keymap->scancode_to_keycode = SDL_CreateHashTable(NULL, 64, SDL_HashID, SDL_KeyMatchID, NULL, false);
-    keymap->keycode_to_scancode = SDL_CreateHashTable(NULL, 64, SDL_HashID, SDL_KeyMatchID, NULL, false);
+    keymap->scancode_to_keycode = SDL_CreateHashTable(NULL, 64, SDL_HashID, SDL_KeyMatchID, NULL, false, false);
+    keymap->keycode_to_scancode = SDL_CreateHashTable(NULL, 64, SDL_HashID, SDL_KeyMatchID, NULL, false, false);
     if (!keymap->scancode_to_keycode || !keymap->keycode_to_scancode) {
         SDL_DestroyKeymap(keymap);
         return NULL;

+ 6 - 6
src/gpu/vulkan/SDL_gpu_vulkan.c

@@ -11628,7 +11628,7 @@ static SDL_GPUDevice *VULKAN_CreateDevice(bool debugMode, bool preferLowPower, S
         VULKAN_INTERNAL_CommandPoolHashFunction,
         VULKAN_INTERNAL_CommandPoolHashKeyMatch,
         VULKAN_INTERNAL_CommandPoolHashNuke,
-        false);
+        false, false);
 
     renderer->renderPassHashTable = SDL_CreateHashTable(
         (void *)renderer,
@@ -11636,7 +11636,7 @@ static SDL_GPUDevice *VULKAN_CreateDevice(bool debugMode, bool preferLowPower, S
         VULKAN_INTERNAL_RenderPassHashFunction,
         VULKAN_INTERNAL_RenderPassHashKeyMatch,
         VULKAN_INTERNAL_RenderPassHashNuke,
-        false);
+        false, false);
 
     renderer->framebufferHashTable = SDL_CreateHashTable(
         (void *)renderer,
@@ -11644,7 +11644,7 @@ static SDL_GPUDevice *VULKAN_CreateDevice(bool debugMode, bool preferLowPower, S
         VULKAN_INTERNAL_FramebufferHashFunction,
         VULKAN_INTERNAL_FramebufferHashKeyMatch,
         VULKAN_INTERNAL_FramebufferHashNuke,
-        false);
+        false, false);
 
     renderer->graphicsPipelineResourceLayoutHashTable = SDL_CreateHashTable(
         (void *)renderer,
@@ -11652,7 +11652,7 @@ static SDL_GPUDevice *VULKAN_CreateDevice(bool debugMode, bool preferLowPower, S
         VULKAN_INTERNAL_GraphicsPipelineResourceLayoutHashFunction,
         VULKAN_INTERNAL_GraphicsPipelineResourceLayoutHashKeyMatch,
         VULKAN_INTERNAL_GraphicsPipelineResourceLayoutHashNuke,
-        false);
+        false, false);
 
     renderer->computePipelineResourceLayoutHashTable = SDL_CreateHashTable(
         (void *)renderer,
@@ -11660,7 +11660,7 @@ static SDL_GPUDevice *VULKAN_CreateDevice(bool debugMode, bool preferLowPower, S
         VULKAN_INTERNAL_ComputePipelineResourceLayoutHashFunction,
         VULKAN_INTERNAL_ComputePipelineResourceLayoutHashKeyMatch,
         VULKAN_INTERNAL_ComputePipelineResourceLayoutHashNuke,
-        false);
+        false, false);
 
     renderer->descriptorSetLayoutHashTable = SDL_CreateHashTable(
         (void *)renderer,
@@ -11668,7 +11668,7 @@ static SDL_GPUDevice *VULKAN_CreateDevice(bool debugMode, bool preferLowPower, S
         VULKAN_INTERNAL_DescriptorSetLayoutHashFunction,
         VULKAN_INTERNAL_DescriptorSetLayoutHashKeyMatch,
         VULKAN_INTERNAL_DescriptorSetLayoutHashNuke,
-        false);
+        false, false);
 
     // Initialize fence pool
 

+ 1 - 1
src/joystick/SDL_gamepad.c

@@ -2582,7 +2582,7 @@ bool SDL_IsGamepad(SDL_JoystickID instance_id)
             }
 
             if (!s_gamepadInstanceIDs) {
-                s_gamepadInstanceIDs = SDL_CreateHashTable(NULL, 4, SDL_HashID, SDL_KeyMatchID, NULL, false);
+                s_gamepadInstanceIDs = SDL_CreateHashTable(NULL, 4, SDL_HashID, SDL_KeyMatchID, NULL, false, false);
             }
             SDL_InsertIntoHashTable(s_gamepadInstanceIDs, (void *)(uintptr_t)instance_id, (void *)(uintptr_t)result);
         }

+ 1 - 1
src/render/gpu/SDL_pipeline_gpu.c

@@ -83,7 +83,7 @@ static void NukePipelineCacheEntry(const void *key, const void *value, void *dat
 bool GPU_InitPipelineCache(GPU_PipelineCache *cache, SDL_GPUDevice *device)
 {
     // FIXME how many buckets do we need?
-    cache->table = SDL_CreateHashTable(device, 32, HashPassthrough, MatchPipelineCacheKey, NukePipelineCacheEntry, true);
+    cache->table = SDL_CreateHashTable(device, 32, HashPassthrough, MatchPipelineCacheKey, NukePipelineCacheEntry, false, true);
     if (!cache->table) {
         return false;
     }

+ 10 - 32
src/stdlib/SDL_getenv.c

@@ -53,7 +53,6 @@ static char **environ;
 
 struct SDL_Environment
 {
-    SDL_Mutex *lock;
     SDL_HashTable *strings;
 };
 static SDL_Environment *SDL_environment;
@@ -88,15 +87,12 @@ SDL_Environment *SDL_CreateEnvironment(bool populated)
         return NULL;
     }
 
-    env->strings = SDL_CreateHashTable(NULL, 16, SDL_HashString, SDL_KeyMatchString, SDL_NukeFreeKey, false);
+    env->strings = SDL_CreateHashTable(NULL, 16, SDL_HashString, SDL_KeyMatchString, SDL_NukeFreeKey, true, false);
     if (!env->strings) {
         SDL_free(env);
         return NULL;
     }
 
-    // Don't fail if we can't create a mutex (e.g. on a single-thread environment)
-    env->lock = SDL_CreateMutex();
-
     if (populated) {
 #ifdef SDL_PLATFORM_WINDOWS
         LPWCH strings = GetEnvironmentStringsW();
@@ -157,15 +153,10 @@ const char *SDL_GetEnvironmentVariable(SDL_Environment *env, const char *name)
         return NULL;
     }
 
-    SDL_LockMutex(env->lock);
-    {
-        const char *value;
-
-        if (SDL_FindInHashTable(env->strings, name, (const void **)&value)) {
-            result = SDL_GetPersistentString(value);
-        }
+    const char *value;
+    if (SDL_FindInHashTable(env->strings, name, (const void **)&value)) {
+        result = SDL_GetPersistentString(value);
     }
-    SDL_UnlockMutex(env->lock);
 
     return result;
 }
@@ -179,7 +170,7 @@ char **SDL_GetEnvironmentVariables(SDL_Environment *env)
         return NULL;
     }
 
-    SDL_LockMutex(env->lock);
+    SDL_LockHashTable(env->strings, false);
     {
         size_t count, length = 0;
         void *iter;
@@ -216,7 +207,7 @@ char **SDL_GetEnvironmentVariables(SDL_Environment *env)
         }
         result[count] = NULL;
     }
-    SDL_UnlockMutex(env->lock);
+    SDL_UnlockHashTable(env->strings);
 
     return result;
 }
@@ -233,7 +224,7 @@ bool SDL_SetEnvironmentVariable(SDL_Environment *env, const char *name, const ch
         return SDL_InvalidParamError("value");
     }
 
-    SDL_LockMutex(env->lock);
+    SDL_LockHashTable(env->strings, true);
     {
         const void *existing_value;
         bool insert = true;
@@ -258,33 +249,21 @@ bool SDL_SetEnvironmentVariable(SDL_Environment *env, const char *name, const ch
             }
         }
     }
-    SDL_UnlockMutex(env->lock);
+    SDL_UnlockHashTable(env->strings);
 
     return result;
 }
 
 bool SDL_UnsetEnvironmentVariable(SDL_Environment *env, const char *name)
 {
-    bool result = false;
-
     if (!env) {
         return SDL_InvalidParamError("env");
     } else if (!name || *name == '\0' || SDL_strchr(name, '=') != NULL) {
         return SDL_InvalidParamError("name");
     }
 
-    SDL_LockMutex(env->lock);
-    {
-        const void *value;
-        if (SDL_FindInHashTable(env->strings, name, &value)) {
-            result = SDL_RemoveFromHashTable(env->strings, name);
-        } else {
-            result = true;
-        }
-    }
-    SDL_UnlockMutex(env->lock);
-
-    return result;
+    SDL_RemoveFromHashTable(env->strings, name);
+    return true;
 }
 
 void SDL_DestroyEnvironment(SDL_Environment *env)
@@ -293,7 +272,6 @@ void SDL_DestroyEnvironment(SDL_Environment *env)
         return;
     }
 
-    SDL_DestroyMutex(env->lock);
     SDL_DestroyHashTable(env->strings);
     SDL_free(env);
 }

+ 15 - 24
src/video/SDL_pixels.c

@@ -572,8 +572,8 @@ SDL_PixelFormat SDL_GetPixelFormatForMasks(int bpp, Uint32 Rmask, Uint32 Gmask,
     return SDL_PIXELFORMAT_UNKNOWN;
 }
 
+static SDL_InitState SDL_format_details_init;
 static SDL_HashTable *SDL_format_details;
-static SDL_Mutex *SDL_format_details_lock;
 
 static bool SDL_InitPixelFormatDetails(SDL_PixelFormatDetails *details, SDL_PixelFormat format)
 {
@@ -646,53 +646,44 @@ const SDL_PixelFormatDetails *SDL_GetPixelFormatDetails(SDL_PixelFormat format)
 {
     SDL_PixelFormatDetails *details;
 
-    if (!SDL_format_details_lock) {
-        SDL_format_details_lock = SDL_CreateMutex();
-    }
-
-    SDL_LockMutex(SDL_format_details_lock);
-
-    if (!SDL_format_details) {
-        SDL_format_details = SDL_CreateHashTable(NULL, 8, SDL_HashID, SDL_KeyMatchID, SDL_NukeFreeValue, false);
+    if (SDL_ShouldInit(&SDL_format_details_init)) {
+        SDL_format_details = SDL_CreateHashTable(NULL, 8, SDL_HashID, SDL_KeyMatchID, SDL_NukeFreeValue, true, false);
+        if (!SDL_format_details) {
+            SDL_SetInitialized(&SDL_format_details_init, false);
+            return NULL;
+        }
+        SDL_SetInitialized(&SDL_format_details_init, true);
     }
 
     if (SDL_FindInHashTable(SDL_format_details, (const void *)(uintptr_t)format, (const void **)&details)) {
-        goto done;
+        return details;
     }
 
     // Allocate an empty pixel format structure, and initialize it
     details = (SDL_PixelFormatDetails *)SDL_malloc(sizeof(*details));
     if (!details) {
-        goto done;
+        return NULL;
     }
 
     if (!SDL_InitPixelFormatDetails(details, format)) {
         SDL_free(details);
-        details = NULL;
-        goto done;
+        return NULL;
     }
 
     if (!SDL_InsertIntoHashTable(SDL_format_details, (const void *)(uintptr_t)format, (void *)details)) {
         SDL_free(details);
-        details = NULL;
-        goto done;
+        return NULL;
     }
 
-done:
-    SDL_UnlockMutex(SDL_format_details_lock);
-
     return details;
 }
 
 void SDL_QuitPixelFormatDetails(void)
 {
-    if (SDL_format_details) {
+    if (SDL_ShouldQuit(&SDL_format_details_init)) {
         SDL_DestroyHashTable(SDL_format_details);
         SDL_format_details = NULL;
-    }
-    if (SDL_format_details_lock) {
-        SDL_DestroyMutex(SDL_format_details_lock);
-        SDL_format_details_lock = NULL;
+        SDL_SetInitialized(&SDL_format_details_init, false);
     }
 }
 
@@ -1505,7 +1496,7 @@ bool SDL_MapSurface(SDL_Surface *src, SDL_Surface *dst)
     } else {
         if (SDL_ISPIXELFORMAT_INDEXED(dstfmt->format)) {
             // BitField --> Palette
-            map->info.palette_map = SDL_CreateHashTable(NULL, 32, SDL_HashID, SDL_KeyMatchID, NULL, false);
+            map->info.palette_map = SDL_CreateHashTable(NULL, 32, SDL_HashID, SDL_KeyMatchID, NULL, false, false);
         } else {
             // BitField --> BitField
             if (srcfmt == dstfmt) {