Added thread-safe initialization/cleanup support

Also went through and removed inappropriate uses of spinlocks.

Fixes https://github.com/libsdl-org/SDL/issues/10872

src/SDL_log.c

@@ -57,8 +57,7 @@ static void SDLCALL SDL_LogOutput(void *userdata, int category, SDL_LogPriority
 static void CleanupLogPriorities(void);
 static void CleanupLogPrefixes(void);
 
-static SDL_AtomicInt SDL_log_initializing;
-static SDL_AtomicInt SDL_log_initialized;
+static SDL_InitState SDL_log_init;
 static SDL_Mutex *SDL_log_lock;
 static SDL_Mutex *SDL_log_function_lock;
 static SDL_LogLevel *SDL_loglevels SDL_GUARDED_BY(SDL_log_lock);
@@ -128,16 +127,7 @@ static void SDLCALL SDL_LoggingChanged(void *userdata, const char *name, const c
 
 void SDL_InitLog(void)
 {
-    if (SDL_AtomicGet(&SDL_log_initialized)) {
-        return;
-    }
-
-    // Do a little tap dance to make sure only one thread initializes logging
-    if (!SDL_AtomicCompareAndSwap(&SDL_log_initializing, false, true)) {
-        // Wait for the other thread to complete initialization
-        while (!SDL_AtomicGet(&SDL_log_initialized)) {
-            SDL_Delay(1);
-        }
+    if (!SDL_ShouldInit(&SDL_log_init)) {
         return;
     }
 
@@ -147,13 +137,15 @@ void SDL_InitLog(void)
 
     SDL_AddHintCallback(SDL_HINT_LOGGING, SDL_LoggingChanged, NULL);
 
-    SDL_AtomicSet(&SDL_log_initializing, false);
-
-    SDL_AtomicSet(&SDL_log_initialized, true);
+    SDL_SetInitialized(&SDL_log_init, true);
 }
 
 void SDL_QuitLog(void)
 {
+    if (!SDL_ShouldQuit(&SDL_log_init)) {
+        return;
+    }
+
     SDL_RemoveHintCallback(SDL_HINT_LOGGING, SDL_LoggingChanged, NULL);
 
     CleanupLogPriorities();
@@ -167,15 +159,19 @@ void SDL_QuitLog(void)
         SDL_DestroyMutex(SDL_log_function_lock);
         SDL_log_function_lock = NULL;
     }
-    SDL_AtomicSet(&SDL_log_initialized, false);
+
+    SDL_SetInitialized(&SDL_log_init, false);
 }
 
-static void SDL_CheckInitLog()
+static void SDL_CheckInitLog(void)
 {
-    if (!SDL_AtomicGet(&SDL_log_initialized) &&
-        !SDL_AtomicGet(&SDL_log_initializing)) {
-        SDL_InitLog();
+    int status = SDL_AtomicGet(&SDL_log_init.status);
+    if (status == SDL_INIT_STATUS_INITIALIZED ||
+        (status == SDL_INIT_STATUS_INITIALIZING && SDL_log_init.thread == SDL_GetCurrentThreadID())) {
+        return;
     }
+
+    SDL_InitLog();
 }
 
 static void CleanupLogPriorities(void)

src/SDL_utils.c

@@ -121,7 +121,40 @@ bool SDL_endswith(const char *string, const char *suffix)
     return false;
 }
 
-// Assume we can wrap SDL_AtomicInt values and cast to Uint32
+bool SDL_ShouldInit(SDL_InitState *state)
+{
+    while (SDL_AtomicGet(&state->status) != SDL_INIT_STATUS_INITIALIZED) {
+        if (SDL_AtomicCompareAndSwap(&state->status, SDL_INIT_STATUS_UNINITIALIZED, SDL_INIT_STATUS_INITIALIZING)) {
+            state->thread = SDL_GetCurrentThreadID();
+            return true;
+        }
+
+        // Wait for the other thread to complete transition
+        SDL_Delay(1);
+    }
+    return false;
+}
+
+bool SDL_ShouldQuit(SDL_InitState *state)
+{
+    if (SDL_AtomicCompareAndSwap(&state->status, SDL_INIT_STATUS_INITIALIZED, SDL_INIT_STATUS_UNINITIALIZING)) {
+        state->thread = SDL_GetCurrentThreadID();
+        return true;
+    }
+    return false;
+}
+
+void SDL_SetInitialized(SDL_InitState *state, bool initialized)
+{
+    SDL_assert(state->thread == SDL_GetCurrentThreadID());
+
+    if (initialized) {
+        SDL_AtomicSet(&state->status, SDL_INIT_STATUS_INITIALIZED);
+    } else {
+        SDL_AtomicSet(&state->status, SDL_INIT_STATUS_UNINITIALIZED);
+    }
+}
+
 SDL_COMPILE_TIME_ASSERT(sizeof_object_id, sizeof(int) == sizeof(Uint32));
 
 Uint32 SDL_GetNextObjectID(void)

src/SDL_utils_c.h

@@ -47,6 +47,24 @@ extern bool SDL_endswith(const char *string, const char *suffix);
  */
 extern int SDL_URIToLocal(const char *src, char *dst);
 
+typedef enum SDL_InitStatus
+{
+    SDL_INIT_STATUS_UNINITIALIZED,
+    SDL_INIT_STATUS_INITIALIZING,
+    SDL_INIT_STATUS_INITIALIZED,
+    SDL_INIT_STATUS_UNINITIALIZING
+} SDL_InitStatus;
+
+typedef struct SDL_InitState
+{
+    SDL_AtomicInt status;
+    SDL_ThreadID thread;
+} SDL_InitState;
+
+extern bool SDL_ShouldInit(SDL_InitState *state);
+extern bool SDL_ShouldQuit(SDL_InitState *state);
+extern void SDL_SetInitialized(SDL_InitState *state, bool initialized);
+
 typedef enum
 {
     SDL_OBJECT_TYPE_UNKNOWN,

src/audio/SDL_audioresample.c

@@ -574,16 +574,11 @@ static void SetupAudioResampler(void)
 
 void SDL_SetupAudioResampler(void)
 {
-    static SDL_SpinLock running = 0;
+    static SDL_InitState init;
 
-    if (!ResampleFrame[0]) {
-        SDL_LockSpinlock(&running);
-
-        if (!ResampleFrame[0]) {
-            SetupAudioResampler();
-        }
-
-        SDL_UnlockSpinlock(&running);
+    if (SDL_ShouldInit(&init)) {
+        SetupAudioResampler();
+        SDL_SetInitialized(&init, true);
     }
 }
 

src/core/linux/SDL_dbus.c

@@ -122,60 +122,61 @@ static bool LoadDBUSLibrary(void)
     return result;
 }
 
-static SDL_SpinLock spinlock_dbus_init = 0;
+static SDL_InitState dbus_init;
 
-// you must hold spinlock_dbus_init before calling this!
-static void SDL_DBus_Init_Spinlocked(void)
+void SDL_DBus_Init(void)
 {
     static bool is_dbus_available = true;
+
     if (!is_dbus_available) {
         return; // don't keep trying if this fails.
     }
 
-    if (!dbus.session_conn) {
-        DBusError err;
-
-        if (!LoadDBUSLibrary()) {
-            is_dbus_available = false; // can't load at all? Don't keep trying.
-            return;
-        }
-
-        if (!dbus.threads_init_default()) {
-            is_dbus_available = false;
-            return;
-        }
-
-        dbus.error_init(&err);
-        // session bus is required
-
-        dbus.session_conn = dbus.bus_get_private(DBUS_BUS_SESSION, &err);
-        if (dbus.error_is_set(&err)) {
-            dbus.error_free(&err);
-            SDL_DBus_Quit();
-            is_dbus_available = false;
-            return; // oh well
-        }
-        dbus.connection_set_exit_on_disconnect(dbus.session_conn, 0);
-
-        // system bus is optional
-        dbus.system_conn = dbus.bus_get_private(DBUS_BUS_SYSTEM, &err);
-        if (!dbus.error_is_set(&err)) {
-            dbus.connection_set_exit_on_disconnect(dbus.system_conn, 0);
-        }
-
-        dbus.error_free(&err);
+    if (!SDL_ShouldInit(&dbus_init)) {
+        return;
     }
-}
 
-void SDL_DBus_Init(void)
-{
-    SDL_LockSpinlock(&spinlock_dbus_init); // make sure two threads can't init at same time, since this can happen before SDL_Init.
-    SDL_DBus_Init_Spinlocked();
-    SDL_UnlockSpinlock(&spinlock_dbus_init);
+    if (!LoadDBUSLibrary()) {
+        goto error;
+    }
+
+    if (!dbus.threads_init_default()) {
+        goto error;
+    }
+
+    DBusError err;
+    dbus.error_init(&err);
+    // session bus is required
+
+    dbus.session_conn = dbus.bus_get_private(DBUS_BUS_SESSION, &err);
+    if (dbus.error_is_set(&err)) {
+        dbus.error_free(&err);
+        goto error;
+    }
+    dbus.connection_set_exit_on_disconnect(dbus.session_conn, 0);
+
+    // system bus is optional
+    dbus.system_conn = dbus.bus_get_private(DBUS_BUS_SYSTEM, &err);
+    if (!dbus.error_is_set(&err)) {
+        dbus.connection_set_exit_on_disconnect(dbus.system_conn, 0);
+    }
+
+    dbus.error_free(&err);
+    SDL_SetInitialized(&dbus_init, true);
+    return;
+
+error:
+    is_dbus_available = false;
+    SDL_SetInitialized(&dbus_init, true);
+    SDL_DBus_Quit();
 }
 
 void SDL_DBus_Quit(void)
 {
+    if (!SDL_ShouldQuit(&dbus_init)) {
+        return;
+    }
+
     if (dbus.system_conn) {
         dbus.connection_close(dbus.system_conn);
         dbus.connection_unref(dbus.system_conn);
@@ -193,8 +194,12 @@ void SDL_DBus_Quit(void)
 
     SDL_zero(dbus);
     UnloadDBUSLibrary();
-    SDL_free(inhibit_handle);
-    inhibit_handle = NULL;
+    if (inhibit_handle) {
+        SDL_free(inhibit_handle);
+        inhibit_handle = NULL;
+    }
+
+    SDL_SetInitialized(&dbus_init, false);
 }
 
 SDL_DBusContext *SDL_DBus_GetContext(void)

src/joystick/hidapi/SDL_hidapijoystick.c

@@ -87,7 +87,7 @@ static SDL_HIDAPI_DeviceDriver *SDL_HIDAPI_drivers[] = {
 #endif
 };
 static int SDL_HIDAPI_numdrivers = 0;
-static SDL_SpinLock SDL_HIDAPI_spinlock;
+static SDL_AtomicInt SDL_HIDAPI_updating_devices;
 static bool SDL_HIDAPI_hints_changed = false;
 static Uint32 SDL_HIDAPI_change_count = 0;
 static SDL_HIDAPI_Device *SDL_HIDAPI_devices SDL_GUARDED_BY(SDL_joystick_lock);
@@ -1243,6 +1243,16 @@ static bool HIDAPI_IsEquivalentToDevice(Uint16 vendor_id, Uint16 product_id, SDL
     return false;
 }
 
+static bool HIDAPI_StartUpdatingDevices()
+{
+    return SDL_AtomicCompareAndSwap(&SDL_HIDAPI_updating_devices, false, true);
+}
+
+static void HIDAPI_FinishUpdatingDevices()
+{
+    SDL_AtomicSet(&SDL_HIDAPI_updating_devices, false);
+}
+
 bool HIDAPI_IsDeviceTypePresent(SDL_GamepadType type)
 {
     SDL_HIDAPI_Device *device;
@@ -1253,9 +1263,9 @@ bool HIDAPI_IsDeviceTypePresent(SDL_GamepadType type)
         return false;
     }
 
-    if (SDL_TryLockSpinlock(&SDL_HIDAPI_spinlock)) {
+    if (HIDAPI_StartUpdatingDevices()) {
         HIDAPI_UpdateDeviceList();
-        SDL_UnlockSpinlock(&SDL_HIDAPI_spinlock);
+        HIDAPI_FinishUpdatingDevices();
     }
 
     SDL_LockJoysticks();
@@ -1298,9 +1308,9 @@ bool HIDAPI_IsDevicePresent(Uint16 vendor_id, Uint16 product_id, Uint16 version,
     }
 #endif // SDL_JOYSTICK_HIDAPI_XBOX360 || SDL_JOYSTICK_HIDAPI_XBOXONE
     if (supported) {
-        if (SDL_TryLockSpinlock(&SDL_HIDAPI_spinlock)) {
+        if (HIDAPI_StartUpdatingDevices()) {
             HIDAPI_UpdateDeviceList();
-            SDL_UnlockSpinlock(&SDL_HIDAPI_spinlock);
+            HIDAPI_FinishUpdatingDevices();
         }
     }
 
@@ -1360,13 +1370,13 @@ SDL_GamepadType HIDAPI_GetGamepadTypeFromGUID(SDL_GUID guid)
 
 static void HIDAPI_JoystickDetect(void)
 {
-    if (SDL_TryLockSpinlock(&SDL_HIDAPI_spinlock)) {
+    if (HIDAPI_StartUpdatingDevices()) {
         Uint32 count = SDL_hid_device_change_count();
         if (SDL_HIDAPI_change_count != count) {
             SDL_HIDAPI_change_count = count;
             HIDAPI_UpdateDeviceList();
         }
-        SDL_UnlockSpinlock(&SDL_HIDAPI_spinlock);
+        HIDAPI_FinishUpdatingDevices();
     }
 }
 
@@ -1379,7 +1389,7 @@ void HIDAPI_UpdateDevices(void)
     // Update the devices, which may change connected joysticks and send events
 
     // Prepare the existing device list
-    if (SDL_TryLockSpinlock(&SDL_HIDAPI_spinlock)) {
+    if (HIDAPI_StartUpdatingDevices()) {
         for (device = SDL_HIDAPI_devices; device; device = device->next) {
             if (device->parent) {
                 continue;
@@ -1393,7 +1403,7 @@ void HIDAPI_UpdateDevices(void)
                 }
             }
         }
-        SDL_UnlockSpinlock(&SDL_HIDAPI_spinlock);
+        HIDAPI_FinishUpdatingDevices();
     }
 }
 

src/timer/SDL_timer.c

@@ -50,6 +50,7 @@ typedef struct SDL_TimerMap
 typedef struct
 {
     // Data used by the main thread
+    SDL_InitState init;
     SDL_Thread *thread;
     SDL_TimerMap *timermap;
     SDL_Mutex *timermap_lock;
@@ -209,29 +210,35 @@ bool SDL_InitTimers(void)
 {
     SDL_TimerData *data = &SDL_timer_data;
 
-    if (!SDL_AtomicGet(&data->active)) {
-        const char *name = "SDLTimer";
-        data->timermap_lock = SDL_CreateMutex();
-        if (!data->timermap_lock) {
-            return false;
-        }
-
-        data->sem = SDL_CreateSemaphore(0);
-        if (!data->sem) {
-            SDL_DestroyMutex(data->timermap_lock);
-            return false;
-        }
-
-        SDL_AtomicSet(&data->active, 1);
-
-        // Timer threads use a callback into the app, so we can't set a limited stack size here.
-        data->thread = SDL_CreateThread(SDL_TimerThread, name, data);
-        if (!data->thread) {
-            SDL_QuitTimers();
-            return false;
-        }
+    if (!SDL_ShouldInit(&data->init)) {
+        return true;
     }
+
+    data->timermap_lock = SDL_CreateMutex();
+    if (!data->timermap_lock) {
+        goto error;
+    }
+
+    data->sem = SDL_CreateSemaphore(0);
+    if (!data->sem) {
+        goto error;
+    }
+
+    SDL_AtomicSet(&data->active, true);
+
+    // Timer threads use a callback into the app, so we can't set a limited stack size here.
+    data->thread = SDL_CreateThread(SDL_TimerThread, "SDLTimer", data);
+    if (!data->thread) {
+        goto error;
+    }
+
+    SDL_SetInitialized(&data->init, true);
     return true;
+
+error:
+    SDL_SetInitialized(&data->init, true);
+    SDL_QuitTimers();
+    return false;
 }
 
 void SDL_QuitTimers(void)
@@ -240,37 +247,52 @@ void SDL_QuitTimers(void)
     SDL_Timer *timer;
     SDL_TimerMap *entry;
 
-    if (SDL_AtomicCompareAndSwap(&data->active, 1, 0)) { // active? Move to inactive.
-        // Shutdown the timer thread
-        if (data->thread) {
-            SDL_SignalSemaphore(data->sem);
-            SDL_WaitThread(data->thread, NULL);
-            data->thread = NULL;
-        }
+    if (!SDL_ShouldQuit(&data->init)) {
+        return;
+    }
 
+    SDL_AtomicSet(&data->active, false);
+
+    // Shutdown the timer thread
+    if (data->thread) {
+        SDL_SignalSemaphore(data->sem);
+        SDL_WaitThread(data->thread, NULL);
+        data->thread = NULL;
+    }
+
+    if (data->sem) {
         SDL_DestroySemaphore(data->sem);
         data->sem = NULL;
+    }
 
-        // Clean up the timer entries
-        while (data->timers) {
-            timer = data->timers;
-            data->timers = timer->next;
-            SDL_free(timer);
-        }
-        while (data->freelist) {
-            timer = data->freelist;
-            data->freelist = timer->next;
-            SDL_free(timer);
-        }
-        while (data->timermap) {
-            entry = data->timermap;
-            data->timermap = entry->next;
-            SDL_free(entry);
-        }
+    // Clean up the timer entries
+    while (data->timers) {
+        timer = data->timers;
+        data->timers = timer->next;
+        SDL_free(timer);
+    }
+    while (data->freelist) {
+        timer = data->freelist;
+        data->freelist = timer->next;
+        SDL_free(timer);
+    }
+    while (data->timermap) {
+        entry = data->timermap;
+        data->timermap = entry->next;
+        SDL_free(entry);
+    }
 
+    if (data->timermap_lock) {
         SDL_DestroyMutex(data->timermap_lock);
         data->timermap_lock = NULL;
     }
+
+    SDL_SetInitialized(&data->init, false);
+}
+
+static bool SDL_CheckInitTimers(void)
+{
+    return SDL_InitTimers();
 }
 
 static SDL_TimerID SDL_CreateTimer(Uint64 interval, SDL_TimerCallback callback_ms, SDL_NSTimerCallback callback_ns, void *userdata)
@@ -284,14 +306,11 @@ static SDL_TimerID SDL_CreateTimer(Uint64 interval, SDL_TimerCallback callback_m
         return 0;
     }
 
-    SDL_LockSpinlock(&data->lock);
-    if (!SDL_AtomicGet(&data->active)) {
-        if (!SDL_InitTimers()) {
-            SDL_UnlockSpinlock(&data->lock);
-            return 0;
-        }
+    if (!SDL_CheckInitTimers()) {
+        return 0;
     }
 
+    SDL_LockSpinlock(&data->lock);
     timer = data->freelist;
     if (timer) {
         data->freelist = timer->next;