diff --git a/src/SDL_hashtable.c b/src/SDL_hashtable.c index 203607a086..192121a36c 100644 --- a/src/SDL_hashtable.c +++ b/src/SDL_hashtable.c @@ -493,28 +493,6 @@ 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; diff --git a/src/SDL_hashtable.h b/src/SDL_hashtable.h index b1998094a6..5d0f810590 100644 --- a/src/SDL_hashtable.h +++ b/src/SDL_hashtable.h @@ -55,15 +55,12 @@ extern bool SDL_FindInHashTable(const SDL_HashTable *table, const void *key, con // 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 +// This function is not thread-safe, you should use external locking if you use this function 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 +// This function is not thread-safe, you should use external locking if you use this function 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); diff --git a/src/camera/SDL_camera.c b/src/camera/SDL_camera.c index 55a0f1a0eb..c11f08affc 100644 --- a/src/camera/SDL_camera.c +++ b/src/camera/SDL_camera.c @@ -299,9 +299,11 @@ 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. } } @@ -327,7 +329,9 @@ 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 { @@ -420,7 +424,9 @@ 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. } @@ -490,6 +496,7 @@ 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 { @@ -507,14 +514,13 @@ 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; } @@ -568,12 +574,12 @@ void SDL_CameraDisconnected(SDL_Camera *device) if (first_disconnect) { if (pending.next) { // NULL if event is disabled or disaster struck. - SDL_LockHashTable(camera_driver.device_hash, true); + SDL_LockRWLockForWriting(camera_driver.device_hash_lock); 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_UnlockHashTable(camera_driver.device_hash); + SDL_UnlockRWLock(camera_driver.device_hash_lock); } } } @@ -606,12 +612,12 @@ void SDL_CameraPermissionOutcome(SDL_Camera *device, bool approved) ReleaseCamera(device); if (pending.next) { // NULL if event is disabled or disaster struck. - SDL_LockHashTable(camera_driver.device_hash, true); + SDL_LockRWLockForWriting(camera_driver.device_hash_lock); 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_UnlockHashTable(camera_driver.device_hash); + SDL_UnlockRWLock(camera_driver.device_hash_lock); } } @@ -627,15 +633,16 @@ SDL_Camera *SDL_FindPhysicalCameraByCallback(bool (*callback)(SDL_Camera *device const void *value; void *iter = NULL; - SDL_LockHashTable(camera_driver.device_hash, false); + SDL_LockRWLockForReading(camera_driver.device_hash_lock); while (SDL_IterateHashTable(camera_driver.device_hash, &key, &value, &iter)) { SDL_Camera *device = (SDL_Camera *) value; if (callback(device, userdata)) { // found it? - SDL_UnlockHashTable(camera_driver.device_hash); + SDL_UnlockRWLock(camera_driver.device_hash_lock); return device; } } - SDL_UnlockHashTable(camera_driver.device_hash); + + SDL_UnlockRWLock(camera_driver.device_hash_lock); SDL_SetError("Device not found"); return NULL; @@ -709,7 +716,7 @@ SDL_CameraID *SDL_GetCameras(int *count) SDL_CameraID *result = NULL; - SDL_LockHashTable(camera_driver.device_hash, false); + SDL_LockRWLockForReading(camera_driver.device_hash_lock); int num_devices = SDL_GetAtomicInt(&camera_driver.device_count); result = (SDL_CameraID *) SDL_malloc((num_devices + 1) * sizeof (SDL_CameraID)); if (!result) { @@ -726,7 +733,7 @@ SDL_CameraID *SDL_GetCameras(int *count) SDL_assert(devs_seen == num_devices); result[devs_seen] = 0; // null-terminated. } - SDL_UnlockHashTable(camera_driver.device_hash); + SDL_UnlockRWLock(camera_driver.device_hash_lock); *count = num_devices; @@ -1356,14 +1363,14 @@ void SDL_QuitCamera(void) return; } - SDL_HashTable *device_hash = camera_driver.device_hash; - SDL_LockHashTable(device_hash, true); + SDL_LockRWLockForWriting(camera_driver.device_hash_lock); SDL_SetAtomicInt(&camera_driver.shutting_down, 1); + SDL_HashTable *device_hash = camera_driver.device_hash; 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_UnlockHashTable(device_hash); + SDL_UnlockRWLock(camera_driver.device_hash_lock); SDL_PendingCameraEvent *pending_next = NULL; for (SDL_PendingCameraEvent *i = pending_events; i; i = pending_next) { @@ -1381,6 +1388,7 @@ 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); @@ -1409,8 +1417,14 @@ bool SDL_CameraInit(const char *driver_name) SDL_QuitCamera(); // shutdown driver if already running. } - SDL_HashTable *device_hash = SDL_CreateHashTable(NULL, 8, HashCameraID, MatchCameraID, NukeCameraHashItem, true, false); + 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, false); if (!device_hash) { + SDL_DestroyRWLock(device_hash_lock); return false; } @@ -1427,6 +1441,7 @@ 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; } @@ -1442,6 +1457,7 @@ 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; @@ -1465,6 +1481,7 @@ 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; @@ -1485,6 +1502,7 @@ 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. } @@ -1501,20 +1519,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_LockHashTable(camera_driver.device_hash, false); + SDL_LockRWLockForReading(camera_driver.device_hash_lock); SDL_PendingCameraEvent *pending_events = camera_driver.pending_events.next; - SDL_UnlockHashTable(camera_driver.device_hash); + SDL_UnlockRWLock(camera_driver.device_hash_lock); 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_LockHashTable(camera_driver.device_hash, true); + SDL_LockRWLockForWriting(camera_driver.device_hash_lock); 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_UnlockHashTable(camera_driver.device_hash); + SDL_UnlockRWLock(camera_driver.device_hash_lock); SDL_PendingCameraEvent *pending_next = NULL; for (SDL_PendingCameraEvent *i = pending_events; i; i = pending_next) { diff --git a/src/stdlib/SDL_getenv.c b/src/stdlib/SDL_getenv.c index 39af4d6b1a..81d3bfbe2a 100644 --- a/src/stdlib/SDL_getenv.c +++ b/src/stdlib/SDL_getenv.c @@ -53,6 +53,7 @@ static char **environ; struct SDL_Environment { + SDL_Mutex *lock; SDL_HashTable *strings; }; static SDL_Environment *SDL_environment; @@ -87,12 +88,15 @@ SDL_Environment *SDL_CreateEnvironment(bool populated) return NULL; } - env->strings = SDL_CreateHashTable(NULL, 16, SDL_HashString, SDL_KeyMatchString, SDL_NukeFreeKey, true, false); + env->strings = SDL_CreateHashTable(NULL, 16, SDL_HashString, SDL_KeyMatchString, SDL_NukeFreeKey, false, 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(); @@ -153,10 +157,15 @@ const char *SDL_GetEnvironmentVariable(SDL_Environment *env, const char *name) return NULL; } - const char *value; - if (SDL_FindInHashTable(env->strings, name, (const void **)&value)) { - result = SDL_GetPersistentString(value); + SDL_LockMutex(env->lock); + { + const char *value; + + if (SDL_FindInHashTable(env->strings, name, (const void **)&value)) { + result = SDL_GetPersistentString(value); + } } + SDL_UnlockMutex(env->lock); return result; } @@ -170,7 +179,7 @@ char **SDL_GetEnvironmentVariables(SDL_Environment *env) return NULL; } - SDL_LockHashTable(env->strings, false); + SDL_LockMutex(env->lock); { size_t count, length = 0; void *iter; @@ -207,7 +216,7 @@ char **SDL_GetEnvironmentVariables(SDL_Environment *env) } result[count] = NULL; } - SDL_UnlockHashTable(env->strings); + SDL_UnlockMutex(env->lock); return result; } @@ -224,7 +233,7 @@ bool SDL_SetEnvironmentVariable(SDL_Environment *env, const char *name, const ch return SDL_InvalidParamError("value"); } - SDL_LockHashTable(env->strings, true); + SDL_LockMutex(env->lock); { const void *existing_value; bool insert = true; @@ -249,21 +258,33 @@ bool SDL_SetEnvironmentVariable(SDL_Environment *env, const char *name, const ch } } } - SDL_UnlockHashTable(env->strings); + SDL_UnlockMutex(env->lock); 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_RemoveFromHashTable(env->strings, name); - return true; + 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; } void SDL_DestroyEnvironment(SDL_Environment *env) @@ -272,6 +293,7 @@ void SDL_DestroyEnvironment(SDL_Environment *env) return; } + SDL_DestroyMutex(env->lock); SDL_DestroyHashTable(env->strings); SDL_free(env); }