Cleaned up thread state handling and added thread object validation

include/SDL3/SDL_thread.h

@@ -105,8 +105,7 @@ typedef enum SDL_ThreadPriority {
 /**
  * The SDL thread state.
  *
- * SDL stores the current state of a thread in an atomic int. The current
- * state of a thread can be checked by calling SDL_GetThreadState.
+ * The current state of a thread can be checked by calling SDL_GetThreadState.
  *
  * \since This enum is available since SDL 3.1.3.
  *
@@ -114,10 +113,10 @@ typedef enum SDL_ThreadPriority {
  */
 typedef enum SDL_ThreadState
 {
-    SDL_THREAD_STATE_ALIVE,
-    SDL_THREAD_STATE_DETACHED,
-    SDL_THREAD_STATE_ZOMBIE,
-    SDL_THREAD_STATE_CLEANED,
+    SDL_THREAD_UNKNOWN,     /**< The thread is not valid */
+    SDL_THREAD_ALIVE,       /**< The thread is currently running */
+    SDL_THREAD_DETACHED,    /**< The thread is detached and can't be waited on */
+    SDL_THREAD_COMPLETE,    /**< The thread has finished and should be cleaned up with SDL_WaitThread() */
 } SDL_ThreadState;
 
 /**
@@ -408,14 +407,14 @@ extern SDL_DECLSPEC bool SDLCALL SDL_SetCurrentThreadPriority(SDL_ThreadPriority
 /**
  * Wait for a thread to finish.
  *
- * Threads that haven't been detached will remain (as a "zombie") until this
+ * Threads that haven't been detached will remain until this
  * function cleans them up. Not doing so is a resource leak.
  *
  * Once a thread has been cleaned up through this function, the SDL_Thread
  * that references it becomes invalid and should not be referenced again. As
  * such, only one thread may call SDL_WaitThread() on another.
  *
- * The return code for the thread function is placed in the area pointed to by
+ * The return code from the thread function is placed in the area pointed to by
  * `status`, if `status` is not NULL.
  *
  * You may not wait on a thread that has been used in a call to
@@ -429,9 +428,8 @@ extern SDL_DECLSPEC bool SDLCALL SDL_SetCurrentThreadPriority(SDL_ThreadPriority
  *
  * \param thread the SDL_Thread pointer that was returned from the
  *               SDL_CreateThread() call that started this thread.
- * \param status pointer to an integer that will receive the value returned
- *               from the thread function by its 'return', or NULL to not
- *               receive such value back.
+ * \param status a pointer filled in with the value returned
+ *               from the thread function by its 'return', or -1 if the thread has been detached or isn't valid, may be NULL.
  *
  * \since This function is available since SDL 3.1.3.
  *
@@ -443,9 +441,8 @@ extern SDL_DECLSPEC void SDLCALL SDL_WaitThread(SDL_Thread *thread, int *status)
 /**
  * Get the current state of a thread.
  *
- * \param thread the thread whose status you want to check.
- * \returns the current state of a thread as defined in the SDL_ThreadState
- *          enum.
+ * \param thread the thread to query.
+ * \returns the current state of a thread, or SDL_THREAD_UNKNOWN if the thread isn't valid.
  *
  * \since This function is available since SDL 3.2.0.
  *

src/SDL_utils.c

@@ -214,6 +214,9 @@ void SDL_SetObjectsInvalid(void)
             case SDL_OBJECT_TYPE_HIDAPI_JOYSTICK:
                 type = "hidapi joystick";
                 break;
+            case SDL_OBJECT_TYPE_THREAD:
+                type = "thread";
+                break;
             default:
                 type = "unknown object";
                 break;

src/SDL_utils_c.h

@@ -60,6 +60,7 @@ typedef enum
     SDL_OBJECT_TYPE_SENSOR,
     SDL_OBJECT_TYPE_HIDAPI_DEVICE,
     SDL_OBJECT_TYPE_HIDAPI_JOYSTICK,
+    SDL_OBJECT_TYPE_THREAD,
 
 } SDL_ObjectType;
 

src/thread/SDL_thread.c

@@ -306,6 +306,11 @@ SDL_error *SDL_GetErrBuf(bool create)
 #endif // SDL_THREADS_DISABLED
 }
 
+static bool ThreadValid(SDL_Thread *thread)
+{
+    return SDL_ObjectValid(thread, SDL_OBJECT_TYPE_THREAD);
+}
+
 void SDL_RunThread(SDL_Thread *thread)
 {
     void *userdata = thread->userdata;
@@ -326,9 +331,10 @@ void SDL_RunThread(SDL_Thread *thread)
     SDL_CleanupTLS();
 
     // Mark us as ready to be joined (or detached)
-    if (!SDL_CompareAndSwapAtomicInt(&thread->state, SDL_THREAD_STATE_ALIVE, SDL_THREAD_STATE_ZOMBIE)) {
+    if (!SDL_CompareAndSwapAtomicInt(&thread->state, SDL_THREAD_ALIVE, SDL_THREAD_COMPLETE)) {
         // Clean up if something already detached us.
-        if (SDL_CompareAndSwapAtomicInt(&thread->state, SDL_THREAD_STATE_DETACHED, SDL_THREAD_STATE_CLEANED)) {
+        if (SDL_GetThreadState(thread) == SDL_THREAD_DETACHED) {
+            SDL_SetObjectValid(thread, SDL_OBJECT_TYPE_THREAD, false);
             SDL_free(thread->name); // Can't free later, we've already cleaned up TLS
             SDL_free(thread);
         }
@@ -364,7 +370,7 @@ SDL_Thread *SDL_CreateThreadWithPropertiesRuntime(SDL_PropertiesID props,
         return NULL;
     }
     thread->status = -1;
-    SDL_SetAtomicInt(&thread->state, SDL_THREAD_STATE_ALIVE);
+    SDL_SetAtomicInt(&thread->state, SDL_THREAD_ALIVE);
 
     // Set up the arguments for the thread
     if (name) {
@@ -379,9 +385,12 @@ SDL_Thread *SDL_CreateThreadWithPropertiesRuntime(SDL_PropertiesID props,
     thread->userdata = userdata;
     thread->stacksize = stacksize;
 
+    SDL_SetObjectValid(thread, SDL_OBJECT_TYPE_THREAD, true);
+
     // Create the thread and go!
     if (!SDL_SYS_CreateThread(thread, pfnBeginThread, pfnEndThread)) {
         // Oops, failed.  Gotta free everything
+        SDL_SetObjectValid(thread, SDL_OBJECT_TYPE_THREAD, false);
         SDL_free(thread->name);
         SDL_free(thread);
         thread = NULL;
@@ -420,10 +429,12 @@ SDL_Thread *SDL_CreateThreadWithStackSize(SDL_ThreadFunction fn, const char *nam
 
 SDL_ThreadID SDL_GetThreadID(SDL_Thread *thread)
 {
-    SDL_ThreadID id;
+    SDL_ThreadID id = 0;
 
     if (thread) {
-        id = thread->threadid;
+        if (ThreadValid(thread)) {
+            id = thread->threadid;
+        }
     } else {
         id = SDL_GetCurrentThreadID();
     }
@@ -432,7 +443,7 @@ SDL_ThreadID SDL_GetThreadID(SDL_Thread *thread)
 
 const char *SDL_GetThreadName(SDL_Thread *thread)
 {
-    if (thread) {
+    if (ThreadValid(thread)) {
         return SDL_GetPersistentString(thread->name);
     } else {
         return NULL;
@@ -446,39 +457,47 @@ bool SDL_SetCurrentThreadPriority(SDL_ThreadPriority priority)
 
 void SDL_WaitThread(SDL_Thread *thread, int *status)
 {
-    if (thread) {
-        SDL_SYS_WaitThread(thread);
+    if (!ThreadValid(thread) || SDL_GetThreadState(thread) == SDL_THREAD_DETACHED) {
         if (status) {
-            *status = thread->status;
+            *status = -1;
         }
-        SDL_free(thread->name);
-        SDL_free(thread);
+        return;
     }
+
+    SDL_SYS_WaitThread(thread);
+    if (status) {
+        *status = thread->status;
+    }
+    SDL_SetObjectValid(thread, SDL_OBJECT_TYPE_THREAD, false);
+    SDL_free(thread->name);
+    SDL_free(thread);
 }
 
 SDL_ThreadState SDL_GetThreadState(SDL_Thread *thread)
 {
+    if (!ThreadValid(thread)) {
+        return SDL_THREAD_UNKNOWN;
+    }
+
     return (SDL_ThreadState)SDL_GetAtomicInt(&thread->state);
 }
 
 void SDL_DetachThread(SDL_Thread *thread)
 {
-    if (!thread) {
+    if (!ThreadValid(thread)) {
         return;
     }
 
     // Grab dibs if the state is alive+joinable.
-    if (SDL_CompareAndSwapAtomicInt(&thread->state, SDL_THREAD_STATE_ALIVE, SDL_THREAD_STATE_DETACHED)) {
+    if (SDL_CompareAndSwapAtomicInt(&thread->state, SDL_THREAD_ALIVE, SDL_THREAD_DETACHED)) {
         SDL_SYS_DetachThread(thread);
     } else {
         // all other states are pretty final, see where we landed.
-        const int thread_state = SDL_GetAtomicInt(&thread->state);
-        if ((thread_state == SDL_THREAD_STATE_DETACHED) || (thread_state == SDL_THREAD_STATE_CLEANED)) {
+        SDL_ThreadState thread_state = SDL_GetThreadState(thread);
+        if (thread_state == SDL_THREAD_DETACHED) {
             return; // already detached (you shouldn't call this twice!)
-        } else if (thread_state == SDL_THREAD_STATE_ZOMBIE) {
+        } else if (thread_state == SDL_THREAD_COMPLETE) {
             SDL_WaitThread(thread, NULL); // already done, clean it up.
-        } else {
-            SDL_assert(0 && "Unexpected thread state");
         }
     }
 }

src/thread/SDL_thread_c.h

@@ -50,7 +50,7 @@ struct SDL_Thread
     SDL_ThreadID threadid;
     SYS_ThreadHandle handle;
     int status;
-    SDL_AtomicInt state; /* SDL_THREAD_STATE_* */
+    SDL_AtomicInt state; /* SDL_ThreadState */
     SDL_error errbuf;
     char *name;
     size_t stacksize; // 0 for default, >0 for user-specified stack size.

src/thread/n3ds/SDL_systhread.c

@@ -126,7 +126,7 @@ void SDL_SYS_WaitThread(SDL_Thread *thread)
       Detached threads can be waited on, but should NOT be cleaned manually
       as it would result in a fatal error.
     */
-    if (R_SUCCEEDED(res) && SDL_GetAtomicInt(&thread->state) != SDL_THREAD_STATE_DETACHED) {
+    if (R_SUCCEEDED(res) && SDL_GetThreadState(thread) != SDL_THREAD_DETACHED) {
         threadFree(thread->handle);
     }
 }