Removed the need for SDL_CreateTLS()

This eliminates the tap dancing needed for allocating TLS slots, we'll automatically allocate them as needed, in a thread-safe way.

build-scripts/SDL_migration.cocci

@@ -2630,11 +2630,6 @@ typedef SDL_cond, SDL_Condition;
 + SDL_WINDOW_HIGH_PIXEL_DENSITY
 @@
 @@
-- SDL_TLSCreate
-+ SDL_CreateTLS
-  (...)
-@@
-@@
 - SDL_TLSGet
 + SDL_GetTLS
   (...)

docs/README-migration.md

@@ -1886,13 +1886,17 @@ SDL_CreateThreadWithStackSize has been replaced with SDL_CreateThreadWithPropert
 
 SDL_CreateThread and SDL_CreateThreadWithProperties now take beginthread/endthread function pointers on all platforms (ignoring them on most), and have been replaced with macros that hide this detail on all platforms. This works the same as before at the source code level, but the actual function signature that is called in SDL has changed. The library's exported symbol is SDL_CreateThreadRuntime, and looking for "SDL_CreateThread" in the DLL/Shared Library/Dylib will fail. You should not call this directly, but instead always use the macro!
 
+SDL_GetTLS() and SDL_SetTLS() take a pointer to a TLS ID, and will automatically initialize it in a thread-safe way as needed.
+
 The following functions have been renamed:
 * SDL_TLSCleanup() => SDL_CleanupTLS()
-* SDL_TLSCreate() => SDL_CreateTLS()
 * SDL_TLSGet() => SDL_GetTLS()
 * SDL_TLSSet() => SDL_SetTLS()
 * SDL_ThreadID() => SDL_GetCurrentThreadID()
 
+The following functions have been removed:
+* SDL_TLSCreate() - TLS IDs are automatically allocated as needed.
+
 The following symbols have been renamed:
 * SDL_threadID => SDL_ThreadID
 

include/SDL3/SDL_oldnames.h

@@ -624,7 +624,6 @@
 
 /* ##SDL_thread.h */
 #define SDL_TLSCleanup SDL_CleanupTLS
-#define SDL_TLSCreate SDL_CreateTLS
 #define SDL_TLSGet SDL_GetTLS
 #define SDL_TLSSet SDL_SetTLS
 #define SDL_threadID SDL_ThreadID
@@ -1241,7 +1240,6 @@
 
 /* ##SDL_thread.h */
 #define SDL_TLSCleanup SDL_TLSCleanup_renamed_SDL_CleanupTLS
-#define SDL_TLSCreate SDL_TLSCreate_renamed_SDL_CreateTLS
 #define SDL_TLSGet SDL_TLSGet_renamed_SDL_GetTLS
 #define SDL_TLSSet SDL_TLSSet_renamed_SDL_SetTLS
 #define SDL_threadID SDL_threadID_renamed_SDL_ThreadID

include/SDL3/SDL_thread.h

@@ -73,18 +73,17 @@ typedef struct SDL_Thread SDL_Thread;
 typedef Uint64 SDL_ThreadID;
 
 /**
- * Thread local storage ID values.
+ * Thread local storage ID.
  *
  * 0 is the invalid ID. An app can create these and then set data for these
  * IDs that is unique to each thread.
  *
  * \since This datatype is available since SDL 3.0.0.
  *
- * \sa SDL_CreateTLS
  * \sa SDL_GetTLS
  * \sa SDL_SetTLS
  */
-typedef Uint32 SDL_TLSID;
+typedef SDL_AtomicInt SDL_TLSID;
 
 /**
  * The SDL thread priority.
@@ -462,34 +461,20 @@ extern SDL_DECLSPEC void SDLCALL SDL_WaitThread(SDL_Thread * thread, int *status
  */
 extern SDL_DECLSPEC void SDLCALL SDL_DetachThread(SDL_Thread * thread);
 
-/**
- * Create a piece of thread-local storage.
- *
- * This creates an identifier that is globally visible to all threads but
- * refers to data that is thread-specific.
- *
- * \returns the newly created thread local storage identifier or 0 on error.
- *
- * \since This function is available since SDL 3.0.0.
- *
- * \sa SDL_GetTLS
- * \sa SDL_SetTLS
- */
-extern SDL_DECLSPEC SDL_TLSID SDLCALL SDL_CreateTLS(void);
-
 /**
  * Get the current thread's value associated with a thread local storage ID.
  *
- * \param id the thread local storage ID.
+ * \param id a pointer to the thread local storage ID, may not be NULL.
  * \returns the value associated with the ID for the current thread or NULL if
  *          no value has been set; call SDL_GetError() for more information.
  *
+ * \threadsafety It is safe to call this function from any thread.
+ *
  * \since This function is available since SDL 3.0.0.
  *
  * \sa SDL_SetTLS
  */
-extern SDL_DECLSPEC void * SDLCALL SDL_GetTLS(SDL_TLSID id);
+extern SDL_DECLSPEC void * SDLCALL SDL_GetTLS(SDL_TLSID *id);
-
 
 /**
  * The callback used to cleanup data passed to SDL_SetTLS.
@@ -507,28 +492,34 @@ typedef void (SDLCALL *SDL_TLSDestructorCallback)(void *value);
 /**
  * Set the current thread's value associated with a thread local storage ID.
  *
+ * If the thread local storage ID is not initialized (the value is 0), a new ID will be created in a thread-safe way, so all calls using a pointer to the same ID will refer to the same local storage.
+ *
  * Note that replacing a value from a previous call to this function on the
  * same thread does _not_ call the previous value's destructor!
  *
  * `destructor` can be NULL; it is assumed that `value` does not need to be
  * cleaned up if so.
  *
- * \param id the thread local storage ID.
+ * \param id a pointer to the thread local storage ID, may not be NULL.
  * \param value the value to associate with the ID for the current thread.
  * \param destructor a function called when the thread exits, to free the
- *                   value. Can be NULL.
+ *                   value, may be NULL.
  * \returns 0 on success or a negative error code on failure; call
  *          SDL_GetError() for more information.
  *
+ * \threadsafety It is safe to call this function from any thread.
+ *
  * \since This function is available since SDL 3.0.0.
  *
  * \sa SDL_GetTLS
  */
-extern SDL_DECLSPEC int SDLCALL SDL_SetTLS(SDL_TLSID id, const void *value, SDL_TLSDestructorCallback destructor);
+extern SDL_DECLSPEC int SDLCALL SDL_SetTLS(SDL_TLSID *id, const void *value, SDL_TLSDestructorCallback destructor);
 
 /**
  * Cleanup all TLS data for this thread.
  *
+ * \threadsafety It is safe to call this function from any thread.
+ *
  * \since This function is available since SDL 3.0.0.
  */
 extern SDL_DECLSPEC void SDLCALL SDL_CleanupTLS(void);

src/dynapi/SDL_dynapi.sym

@@ -71,7 +71,6 @@ SDL3_0.0.0 {
     SDL_CreateSurfaceFrom;
     SDL_CreateSurfacePalette;
     SDL_CreateSystemCursor;
-    SDL_CreateTLS;
     SDL_CreateTexture;
     SDL_CreateTextureFromSurface;
     SDL_CreateTextureWithProperties;

src/dynapi/SDL_dynapi_overrides.h

@@ -96,7 +96,6 @@
 #define SDL_CreateSurfaceFrom SDL_CreateSurfaceFrom_REAL
 #define SDL_CreateSurfacePalette SDL_CreateSurfacePalette_REAL
 #define SDL_CreateSystemCursor SDL_CreateSystemCursor_REAL
-#define SDL_CreateTLS SDL_CreateTLS_REAL
 #define SDL_CreateTexture SDL_CreateTexture_REAL
 #define SDL_CreateTextureFromSurface SDL_CreateTextureFromSurface_REAL
 #define SDL_CreateTextureWithProperties SDL_CreateTextureWithProperties_REAL

src/dynapi/SDL_dynapi_procs.h

@@ -116,7 +116,6 @@ SDL_DYNAPI_PROC(SDL_Surface*,SDL_CreateSurface,(int a, int b, SDL_PixelFormat c)
 SDL_DYNAPI_PROC(SDL_Surface*,SDL_CreateSurfaceFrom,(int a, int b, SDL_PixelFormat c, void *d, int e),(a,b,c,d,e),return)
 SDL_DYNAPI_PROC(SDL_Palette*,SDL_CreateSurfacePalette,(SDL_Surface *a),(a),return)
 SDL_DYNAPI_PROC(SDL_Cursor*,SDL_CreateSystemCursor,(SDL_SystemCursor a),(a),return)
-SDL_DYNAPI_PROC(SDL_TLSID,SDL_CreateTLS,(void),(),return)
 SDL_DYNAPI_PROC(SDL_Texture*,SDL_CreateTexture,(SDL_Renderer *a, SDL_PixelFormat b, int c, int d, int e),(a,b,c,d,e),return)
 SDL_DYNAPI_PROC(SDL_Texture*,SDL_CreateTextureFromSurface,(SDL_Renderer *a, SDL_Surface *b),(a,b),return)
 SDL_DYNAPI_PROC(SDL_Texture*,SDL_CreateTextureWithProperties,(SDL_Renderer *a, SDL_PropertiesID b),(a,b),return)
@@ -484,7 +483,7 @@ SDL_DYNAPI_PROC(SDL_Palette*,SDL_GetSurfacePalette,(SDL_Surface *a),(a),return)
 SDL_DYNAPI_PROC(SDL_PropertiesID,SDL_GetSurfaceProperties,(SDL_Surface *a),(a),return)
 SDL_DYNAPI_PROC(int,SDL_GetSystemRAM,(void),(),return)
 SDL_DYNAPI_PROC(SDL_SystemTheme,SDL_GetSystemTheme,(void),(),return)
-SDL_DYNAPI_PROC(void*,SDL_GetTLS,(SDL_TLSID a),(a),return)
+SDL_DYNAPI_PROC(void*,SDL_GetTLS,(SDL_TLSID *a),(a),return)
 SDL_DYNAPI_PROC(int,SDL_GetTextInputArea,(SDL_Window *a, SDL_Rect *b, int *c),(a,b,c),return)
 SDL_DYNAPI_PROC(int,SDL_GetTextureAlphaMod,(SDL_Texture *a, Uint8 *b),(a,b),return)
 SDL_DYNAPI_PROC(int,SDL_GetTextureAlphaModFloat,(SDL_Texture *a, float *b),(a,b),return)
@@ -792,7 +791,7 @@ SDL_DYNAPI_PROC(int,SDL_SetSurfaceColorMod,(SDL_Surface *a, Uint8 b, Uint8 c, Ui
 SDL_DYNAPI_PROC(int,SDL_SetSurfaceColorspace,(SDL_Surface *a, SDL_Colorspace b),(a,b),return)
 SDL_DYNAPI_PROC(int,SDL_SetSurfacePalette,(SDL_Surface *a, SDL_Palette *b),(a,b),return)
 SDL_DYNAPI_PROC(int,SDL_SetSurfaceRLE,(SDL_Surface *a, SDL_bool b),(a,b),return)
-SDL_DYNAPI_PROC(int,SDL_SetTLS,(SDL_TLSID a, const void *b, SDL_TLSDestructorCallback c),(a,b,c),return)
+SDL_DYNAPI_PROC(int,SDL_SetTLS,(SDL_TLSID *a, const void *b, SDL_TLSDestructorCallback c),(a,b,c),return)
 SDL_DYNAPI_PROC(int,SDL_SetTextInputArea,(SDL_Window *a, const SDL_Rect *b, int c),(a,b,c),return)
 SDL_DYNAPI_PROC(int,SDL_SetTextureAlphaMod,(SDL_Texture *a, Uint8 b),(a,b),return)
 SDL_DYNAPI_PROC(int,SDL_SetTextureAlphaModFloat,(SDL_Texture *a, float b),(a,b),return)

src/thread/SDL_thread.c

@@ -29,34 +29,37 @@
 /* The storage is local to the thread, but the IDs are global for the process */
 
 static SDL_AtomicInt SDL_tls_allocated;
+static SDL_AtomicInt SDL_tls_id;
 
 void SDL_InitTLSData(void)
 {
     SDL_SYS_InitTLSData();
 }
 
-SDL_TLSID SDL_CreateTLS(void)
+void *SDL_GetTLS(SDL_TLSID *id)
-{
-    static SDL_AtomicInt SDL_tls_id;
-    return (SDL_TLSID)(SDL_AtomicIncRef(&SDL_tls_id) + 1);
-}
-
-void *SDL_GetTLS(SDL_TLSID id)
 {
     SDL_TLSData *storage;
+    int storage_index;
 
-    storage = SDL_SYS_GetTLSData();
+    if (id == NULL) {
-    if (!storage || id == 0 || id > storage->limit) {
+        SDL_InvalidParamError("id");
         return NULL;
     }
-    return storage->array[id - 1].data;
+
+    storage_index = SDL_AtomicGet(id) - 1;
+    storage = SDL_SYS_GetTLSData();
+    if (!storage || storage_index < 0 || storage_index >= storage->limit) {
+        return NULL;
+    }
+    return storage->array[storage_index].data;
 }
 
-int SDL_SetTLS(SDL_TLSID id, const void *value, SDL_TLSDestructorCallback destructor)
+int SDL_SetTLS(SDL_TLSID *id, const void *value, SDL_TLSDestructorCallback destructor)
 {
     SDL_TLSData *storage;
+    int storage_index;
 
-    if (id == 0) {
+    if (id == NULL) {
         return SDL_InvalidParamError("id");
     }
 
@@ -66,14 +69,27 @@ int SDL_SetTLS(SDL_TLSID id, const void *value, SDL_TLSDestructorCallback destru
      */
     SDL_InitTLSData();
 
+    /* Get the storage index associated with the ID in a thread-safe way */
+    storage_index = SDL_AtomicGet(id) - 1;
+    if (storage_index < 0) {
+        int new_id = (SDL_AtomicIncRef(&SDL_tls_id) + 1);
+
+        SDL_AtomicCompareAndSwap(id, 0, new_id);
+
+        /* If there was a race condition we'll have wasted an ID, but every thread
+         * will have the same storage index for this id.
+         */
+        storage_index = SDL_AtomicGet(id) - 1;
+    }
+
     /* Get the storage for the current thread */
     storage = SDL_SYS_GetTLSData();
-    if (!storage || (id > storage->limit)) {
+    if (!storage || storage_index >= storage->limit) {
         unsigned int i, oldlimit, newlimit;
         SDL_TLSData *new_storage;
 
         oldlimit = storage ? storage->limit : 0;
-        newlimit = (id + TLS_ALLOC_CHUNKSIZE);
+        newlimit = (storage_index + TLS_ALLOC_CHUNKSIZE);
         new_storage = (SDL_TLSData *)SDL_realloc(storage, sizeof(*storage) + (newlimit - 1) * sizeof(storage->array[0]));
         if (!new_storage) {
             return -1;
@@ -91,8 +107,8 @@ int SDL_SetTLS(SDL_TLSID id, const void *value, SDL_TLSDestructorCallback destru
         SDL_AtomicIncRef(&SDL_tls_allocated);
     }
 
-    storage->array[id - 1].data = SDL_const_cast(void *, value);
+    storage->array[storage_index].data = SDL_const_cast(void *, value);
-    storage->array[id - 1].destructor = destructor;
+    storage->array[storage_index].destructor = destructor;
     return 0;
 }
 
@@ -103,7 +119,7 @@ void SDL_CleanupTLS(void)
     /* Cleanup the storage for the current thread */
     storage = SDL_SYS_GetTLSData();
     if (storage) {
-        unsigned int i;
+        int i;
         for (i = 0; i < storage->limit; ++i) {
             if (storage->array[i].destructor) {
                 storage->array[i].destructor(storage->array[i].data);
@@ -261,42 +277,15 @@ SDL_error *SDL_GetErrBuf(SDL_bool create)
 #ifdef SDL_THREADS_DISABLED
     return SDL_GetStaticErrBuf();
 #else
-    static SDL_SpinLock tls_lock;
-    static SDL_bool tls_being_created;
     static SDL_TLSID tls_errbuf;
-    const SDL_error *ALLOCATION_IN_PROGRESS = (SDL_error *)-1;
     SDL_error *errbuf;
 
-    if (!tls_errbuf && !create) {
+    errbuf = (SDL_error *)SDL_GetTLS(&tls_errbuf);
-        return NULL;
-    }
-
-    /* tls_being_created is there simply to prevent recursion if SDL_CreateTLS() fails.
-       It also means it's possible for another thread to also use SDL_global_errbuf,
-       but that's very unlikely and hopefully won't cause issues.
-     */
-    if (!tls_errbuf && !tls_being_created) {
-        SDL_LockSpinlock(&tls_lock);
-        if (!tls_errbuf) {
-            SDL_TLSID slot;
-            tls_being_created = SDL_TRUE;
-            slot = SDL_CreateTLS();
-            tls_being_created = SDL_FALSE;
-            SDL_MemoryBarrierRelease();
-            tls_errbuf = slot;
-        }
-        SDL_UnlockSpinlock(&tls_lock);
-    }
-    if (!tls_errbuf) {
-        return SDL_GetStaticErrBuf();
-    }
-
-    SDL_MemoryBarrierAcquire();
-    errbuf = (SDL_error *)SDL_GetTLS(tls_errbuf);
-    if (errbuf == ALLOCATION_IN_PROGRESS) {
-        return SDL_GetStaticErrBuf();
-    }
     if (!errbuf) {
+        if (!create) {
+            return NULL;
+        }
+
         /* Get the original memory functions for this allocation because the lifetime
          * of the error buffer may span calls to SDL_SetMemoryFunctions() by the app
          */
@@ -304,17 +293,14 @@ SDL_error *SDL_GetErrBuf(SDL_bool create)
         SDL_free_func free_func;
         SDL_GetOriginalMemoryFunctions(NULL, NULL, &realloc_func, &free_func);
 
-        /* Mark that we're in the middle of allocating our buffer */
-        SDL_SetTLS(tls_errbuf, ALLOCATION_IN_PROGRESS, NULL);
         errbuf = (SDL_error *)realloc_func(NULL, sizeof(*errbuf));
         if (!errbuf) {
-            SDL_SetTLS(tls_errbuf, NULL, NULL);
             return SDL_GetStaticErrBuf();
         }
         SDL_zerop(errbuf);
         errbuf->realloc_func = realloc_func;
         errbuf->free_func = free_func;
-        SDL_SetTLS(tls_errbuf, errbuf, SDL_FreeErrBuf);
+        SDL_SetTLS(&tls_errbuf, errbuf, SDL_FreeErrBuf);
     }
     return errbuf;
 #endif /* SDL_THREADS_DISABLED */

src/thread/SDL_thread_c.h

@@ -78,7 +78,7 @@ extern void SDL_RunThread(SDL_Thread *thread);
 /* This is the system-independent thread local storage structure */
 typedef struct
 {
-    SDL_TLSID limit;
+    int limit;
     struct
     {
         void *data;

src/timer/windows/SDL_systimer.c

@@ -36,14 +36,11 @@ HANDLE SDL_GetWaitableTimer()
     static SDL_TLSID TLS_timer_handle;
     HANDLE timer;
 
-    if (!TLS_timer_handle) {
+    timer = SDL_GetTLS(&TLS_timer_handle);
-        TLS_timer_handle = SDL_CreateTLS();
-    }
-    timer = SDL_GetTLS(TLS_timer_handle);
     if (!timer) {
         timer = CreateWaitableTimerExW(NULL, NULL, CREATE_WAITABLE_TIMER_HIGH_RESOLUTION, TIMER_ALL_ACCESS);
         if (timer) {
-            SDL_SetTLS(TLS_timer_handle, timer, SDL_CleanupWaitableTimer);
+            SDL_SetTLS(&TLS_timer_handle, timer, SDL_CleanupWaitableTimer);
         }
     }
     return timer;

src/video/SDL_video.c

@@ -611,9 +611,6 @@ int SDL_VideoInit(const char *driver_name)
     _this->gl_config.dll_handle = NULL;
     SDL_GL_ResetAttributes();
 
-    _this->current_glwin_tls = SDL_CreateTLS();
-    _this->current_glctx_tls = SDL_CreateTLS();
-
     /* Initialize the video subsystem */
     if (_this->VideoInit(_this) < 0) {
         SDL_VideoQuit();
@@ -4794,8 +4791,8 @@ SDL_GLContext SDL_GL_CreateContext(SDL_Window *window)
     if (ctx) {
         _this->current_glwin = window;
         _this->current_glctx = ctx;
-        SDL_SetTLS(_this->current_glwin_tls, window, NULL);
+        SDL_SetTLS(&_this->current_glwin_tls, window, NULL);
-        SDL_SetTLS(_this->current_glctx_tls, ctx, NULL);
+        SDL_SetTLS(&_this->current_glctx_tls, ctx, NULL);
     }
     return ctx;
 }
@@ -4830,8 +4827,8 @@ int SDL_GL_MakeCurrent(SDL_Window *window, SDL_GLContext context)
     if (retval == 0) {
         _this->current_glwin = window;
         _this->current_glctx = context;
-        SDL_SetTLS(_this->current_glwin_tls, window, NULL);
+        SDL_SetTLS(&_this->current_glwin_tls, window, NULL);
-        SDL_SetTLS(_this->current_glctx_tls, context, NULL);
+        SDL_SetTLS(&_this->current_glctx_tls, context, NULL);
     }
     return retval;
 }
@@ -4842,7 +4839,7 @@ SDL_Window *SDL_GL_GetCurrentWindow(void)
         SDL_UninitializedVideo();
         return NULL;
     }
-    return (SDL_Window *)SDL_GetTLS(_this->current_glwin_tls);
+    return (SDL_Window *)SDL_GetTLS(&_this->current_glwin_tls);
 }
 
 SDL_GLContext SDL_GL_GetCurrentContext(void)
@@ -4851,7 +4848,7 @@ SDL_GLContext SDL_GL_GetCurrentContext(void)
         SDL_UninitializedVideo();
         return NULL;
     }
-    return (SDL_GLContext)SDL_GetTLS(_this->current_glctx_tls);
+    return (SDL_GLContext)SDL_GetTLS(&_this->current_glctx_tls);
 }
 
 SDL_EGLDisplay SDL_EGL_GetCurrentEGLDisplay(void)

test/testthread.c

@@ -58,9 +58,9 @@ ThreadFunc(void *data)
 {
     SDL_ThreadPriority prio = SDL_THREAD_PRIORITY_NORMAL;
 
-    SDL_SetTLS(tls, "baby thread", NULL);
+    SDL_SetTLS(&tls, "baby thread", NULL);
     SDL_Log("Started thread %s: My thread id is %" SDL_PRIu64 ", thread data = %s\n",
-            (char *)data, SDL_GetCurrentThreadID(), (const char *)SDL_GetTLS(tls));
+            (char *)data, SDL_GetCurrentThreadID(), (const char *)SDL_GetTLS(&tls));
     while (alive) {
         SDL_Log("Thread '%s' is alive!\n", (char *)data);
 
@@ -132,10 +132,8 @@ int main(int argc, char *argv[])
         return 0;
     }
 
-    tls = SDL_CreateTLS();
+    SDL_SetTLS(&tls, "main thread", NULL);
-    SDL_assert(tls);
+    SDL_Log("Main thread data initially: %s\n", (const char *)SDL_GetTLS(&tls));
-    SDL_SetTLS(tls, "main thread", NULL);
-    SDL_Log("Main thread data initially: %s\n", (const char *)SDL_GetTLS(tls));
 
     alive = 1;
     thread = SDL_CreateThread(ThreadFunc, "One", "#1");
@@ -148,7 +146,7 @@ int main(int argc, char *argv[])
     alive = 0;
     SDL_WaitThread(thread, NULL);
 
-    SDL_Log("Main thread data finally: %s\n", (const char *)SDL_GetTLS(tls));
+    SDL_Log("Main thread data finally: %s\n", (const char *)SDL_GetTLS(&tls));
 
     alive = 1;
     (void)signal(SIGTERM, killed);