thread: Put all important SDL_CreateThread internal data into SDL_Thr…

…ead. This avoids the need to malloc something extra, use a semaphore, etc, and fixes Emscripten with pthreads support, which might not spin up a web worker until after SDL_CreateThread returns and thus can't wait on a semaphore at this point in any case. Fixes Bugzilla #5064.
libsdl-org · Mar 27, 2020 · 46bb47c · 46bb47c
1 parent abdc5cb
commit 46bb47c
Show file tree

Hide file tree

Showing 8 changed files with 44 additions and 97 deletions.
diff --git a/src/thread/SDL_systhread.h b/src/thread/SDL_systhread.h
@@ -33,11 +33,11 @@
    on success.
 */
 #ifdef SDL_PASSED_BEGINTHREAD_ENDTHREAD
-extern int SDL_SYS_CreateThread(SDL_Thread * thread, void *args,
+extern int SDL_SYS_CreateThread(SDL_Thread * thread,
                                 pfnSDL_CurrentBeginThread pfnBeginThread,
                                 pfnSDL_CurrentEndThread pfnEndThread);
 #else
-extern int SDL_SYS_CreateThread(SDL_Thread * thread, void *args);
+extern int SDL_SYS_CreateThread(SDL_Thread * thread);
 #endif
 
 /* This function does any necessary setup in the child thread */

diff --git a/src/thread/SDL_thread.c b/src/thread/SDL_thread.c
@@ -258,22 +258,12 @@ SDL_GetErrBuf(void)
 }
 
 
-/* Arguments and callback to setup and run the user thread function */
-typedef struct
-{
-    int (SDLCALL * func) (void *);
-    void *data;
-    SDL_Thread *info;
-    SDL_sem *wait;
-} thread_args;
-
 void
-SDL_RunThread(void *data)
+SDL_RunThread(SDL_Thread *thread)
 {
-    thread_args *args = (thread_args *) data;
-    int (SDLCALL * userfunc) (void *) = args->func;
-    void *userdata = args->data;
-    SDL_Thread *thread = args->info;
+    void *userdata = thread->userdata;
+    int (SDLCALL * userfunc) (void *) = thread->userfunc;
+
     int *statusloc = &thread->status;
 
     /* Perform any system-dependent setup - this function may not fail */
@@ -282,9 +272,6 @@ SDL_RunThread(void *data)
     /* Get the thread id */
     thread->threadid = SDL_ThreadID();
 
-    /* Wake up the parent thread */
-    SDL_SemPost(args->wait);
-
     /* Run the function */
     *statusloc = userfunc(userdata);
 
@@ -325,16 +312,14 @@ SDL_CreateThreadWithStackSize(int (SDLCALL * fn) (void *),
 #endif
 {
     SDL_Thread *thread;
-    thread_args *args;
     int ret;
 
     /* Allocate memory for the thread info structure */
-    thread = (SDL_Thread *) SDL_malloc(sizeof(*thread));
+    thread = (SDL_Thread *) SDL_calloc(1, sizeof(*thread));
     if (thread == NULL) {
         SDL_OutOfMemory();
-        return (NULL);
+        return NULL;
     }
-    SDL_zerop(thread);
     thread->status = -1;
     SDL_AtomicSet(&thread->state, SDL_THREAD_STATE_ALIVE);
 
@@ -344,57 +329,29 @@ SDL_CreateThreadWithStackSize(int (SDLCALL * fn) (void *),
         if (thread->name == NULL) {
             SDL_OutOfMemory();
             SDL_free(thread);
-            return (NULL);
-        }
-    }
-
-    /* Set up the arguments for the thread */
-    args = (thread_args *) SDL_malloc(sizeof(*args));
-    if (args == NULL) {
-        SDL_OutOfMemory();
-        if (thread->name) {
-            SDL_free(thread->name);
+            return NULL;
         }
-        SDL_free(thread);
-        return (NULL);
-    }
-    args->func = fn;
-    args->data = data;
-    args->info = thread;
-    args->wait = SDL_CreateSemaphore(0);
-    if (args->wait == NULL) {
-        if (thread->name) {
-            SDL_free(thread->name);
-        }
-        SDL_free(thread);
-        SDL_free(args);
-        return (NULL);
     }
 
+    thread->userfunc = fn;
+    thread->userdata = data;
     thread->stacksize = stacksize;
 
     /* Create the thread and go! */
 #ifdef SDL_PASSED_BEGINTHREAD_ENDTHREAD
-    ret = SDL_SYS_CreateThread(thread, args, pfnBeginThread, pfnEndThread);
+    ret = SDL_SYS_CreateThread(thread, pfnBeginThread, pfnEndThread);
 #else
-    ret = SDL_SYS_CreateThread(thread, args);
+    ret = SDL_SYS_CreateThread(thread);
 #endif
-    if (ret >= 0) {
-        /* Wait for the thread function to use arguments */
-        SDL_SemWait(args->wait);
-    } else {
+    if (ret < 0) {
         /* Oops, failed.  Gotta free everything */
-        if (thread->name) {
-            SDL_free(thread->name);
-        }
+        SDL_free(thread->name);
         SDL_free(thread);
         thread = NULL;
     }
-    SDL_DestroySemaphore(args->wait);
-    SDL_free(args);
 
     /* Everything is running now */
-    return (thread);
+    return thread;
 }
 
 #ifdef SDL_PASSED_BEGINTHREAD_ENDTHREAD

diff --git a/src/thread/SDL_thread_c.h b/src/thread/SDL_thread_c.h
@@ -60,11 +60,14 @@ struct SDL_Thread
     SDL_error errbuf;
     char *name;
     size_t stacksize;  /* 0 for default, >0 for user-specified stack size. */
+    int (SDLCALL * userfunc) (void *);
+    void *userdata;
     void *data;
+    void *endfunc;  /* only used on some platforms. */
 };
 
 /* This is the function called to run a thread */
-extern void SDL_RunThread(void *data);
+extern void SDL_RunThread(SDL_Thread *thread);
 
 /* This is the system-independent thread local storage structure */
 typedef struct {

diff --git a/src/thread/generic/SDL_systhread.c b/src/thread/generic/SDL_systhread.c
@@ -27,12 +27,12 @@
 
 #ifdef SDL_PASSED_BEGINTHREAD_ENDTHREAD
 int
-SDL_SYS_CreateThread(SDL_Thread * thread, void *args,
+SDL_SYS_CreateThread(SDL_Thread * thread,
                      pfnSDL_CurrentBeginThread pfnBeginThread,
                      pfnSDL_CurrentEndThread pfnEndThread)
 #else
 int
-SDL_SYS_CreateThread(SDL_Thread * thread, void *args)
+SDL_SYS_CreateThread(SDL_Thread * thread)
 #endif /* SDL_PASSED_BEGINTHREAD_ENDTHREAD */
 {
     return SDL_SetError("Threads are not supported on this platform");

diff --git a/src/thread/psp/SDL_systhread.c b/src/thread/psp/SDL_systhread.c
@@ -37,11 +37,11 @@
 
 static int ThreadEntry(SceSize args, void *argp)
 {
-    SDL_RunThread(*(void **) argp);
+    SDL_RunThread(*(SDL_Thread **) argp);
     return 0;
 }
 
-int SDL_SYS_CreateThread(SDL_Thread *thread, void *args)
+int SDL_SYS_CreateThread(SDL_Thread *thread)
 {
     SceKernelThreadInfo status;
     int priority = 32;
@@ -59,7 +59,7 @@ int SDL_SYS_CreateThread(SDL_Thread *thread, void *args)
         return SDL_SetError("sceKernelCreateThread() failed");
     }
 
-    sceKernelStartThread(thread->handle, 4, &args);
+    sceKernelStartThread(thread->handle, 4, &thread);
     return 0;
 }
 

diff --git a/src/thread/pthread/SDL_systhread.c b/src/thread/pthread/SDL_systhread.c
@@ -76,7 +76,7 @@ RunThread(void *data)
 #ifdef __ANDROID__
     Android_JNI_SetupThread();
 #endif
-    SDL_RunThread(data);
+    SDL_RunThread((SDL_Thread *) data);
     return NULL;
 }
 
@@ -88,7 +88,7 @@ static SDL_bool checked_setname = SDL_FALSE;
 static int (*ppthread_setname_np)(pthread_t, const char*) = NULL;
 #endif
 int
-SDL_SYS_CreateThread(SDL_Thread * thread, void *args)
+SDL_SYS_CreateThread(SDL_Thread * thread)
 {
     pthread_attr_t type;
 
@@ -117,7 +117,7 @@ SDL_SYS_CreateThread(SDL_Thread * thread, void *args)
     }
 
     /* Create the thread and go! */
-    if (pthread_create(&thread->handle, &type, RunThread, args) != 0) {
+    if (pthread_create(&thread->handle, &type, RunThread, thread) != 0) {
         return SDL_SetError("Not enough resources to create thread");
     }
 

diff --git a/src/thread/stdcpp/SDL_systhread.cpp b/src/thread/stdcpp/SDL_systhread.cpp
@@ -40,16 +40,16 @@ extern "C" {
 static void
 RunThread(void *args)
 {
-    SDL_RunThread(args);
+    SDL_RunThread((SDL_Thread *) args);
 }
 
 extern "C"
 int
-SDL_SYS_CreateThread(SDL_Thread * thread, void *args)
+SDL_SYS_CreateThread(SDL_Thread * thread)
 {
     try {
         // !!! FIXME: no way to set a thread stack size here.
-        std::thread cpp_thread(RunThread, args);
+        std::thread cpp_thread(RunThread, thread);
         thread->handle = (void *) new std::thread(std::move(cpp_thread));
         return 0;
     } catch (std::system_error & ex) {

diff --git a/src/thread/windows/SDL_systhread.c b/src/thread/windows/SDL_systhread.c
@@ -74,23 +74,16 @@ typedef void (__cdecl * pfnSDL_CurrentEndThread) (unsigned code);
 #endif /* !SDL_PASSED_BEGINTHREAD_ENDTHREAD */
 
 
-typedef struct ThreadStartParms
-{
-    void *args;
-    pfnSDL_CurrentEndThread pfnCurrentEndThread;
-} tThreadStartParms, *pThreadStartParms;
-
 static DWORD
 RunThread(void *data)
 {
-    pThreadStartParms pThreadParms = (pThreadStartParms) data;
-    pfnSDL_CurrentEndThread pfnEndThread = pThreadParms->pfnCurrentEndThread;
-    void *args = pThreadParms->args;
-    SDL_free(pThreadParms);
-    SDL_RunThread(args);
-    if (pfnEndThread != NULL)
+    SDL_Thread *thread = (SDL_Thread *) data;
+    pfnSDL_CurrentEndThread pfnEndThread = (pfnSDL_CurrentEndThread) thread->endfunc;
+    SDL_RunThread(thread);
+    if (pfnEndThread != NULL) {
         pfnEndThread(0);
-    return (0);
+    }
+    return 0;
 }
 
 static DWORD WINAPI
@@ -107,46 +100,40 @@ RunThreadViaBeginThreadEx(void *data)
 
 #ifdef SDL_PASSED_BEGINTHREAD_ENDTHREAD
 int
-SDL_SYS_CreateThread(SDL_Thread * thread, void *args,
+SDL_SYS_CreateThread(SDL_Thread * thread,
                      pfnSDL_CurrentBeginThread pfnBeginThread,
                      pfnSDL_CurrentEndThread pfnEndThread)
 {
 #elif defined(__CYGWIN__) || defined(__WINRT__)
 int
-SDL_SYS_CreateThread(SDL_Thread * thread, void *args)
+SDL_SYS_CreateThread(SDL_Thread * thread)
 {
     pfnSDL_CurrentBeginThread pfnBeginThread = NULL;
     pfnSDL_CurrentEndThread pfnEndThread = NULL;
 #else
 int
-SDL_SYS_CreateThread(SDL_Thread * thread, void *args)
+SDL_SYS_CreateThread(SDL_Thread * thread)
 {
     pfnSDL_CurrentBeginThread pfnBeginThread = (pfnSDL_CurrentBeginThread)_beginthreadex;
     pfnSDL_CurrentEndThread pfnEndThread = (pfnSDL_CurrentEndThread)_endthreadex;
 #endif /* SDL_PASSED_BEGINTHREAD_ENDTHREAD */
-    pThreadStartParms pThreadParms =
-        (pThreadStartParms) SDL_malloc(sizeof(tThreadStartParms));
     const DWORD flags = thread->stacksize ? STACK_SIZE_PARAM_IS_A_RESERVATION : 0;
-    if (!pThreadParms) {
-        return SDL_OutOfMemory();
-    }
+
     /* Save the function which we will have to call to clear the RTL of calling app! */
-    pThreadParms->pfnCurrentEndThread = pfnEndThread;
-    /* Also save the real parameters we have to pass to thread function */
-    pThreadParms->args = args;
+    thread->endfunc = pfnEndThread;
 
     /* thread->stacksize == 0 means "system default", same as win32 expects */
     if (pfnBeginThread) {
         unsigned threadid = 0;
         thread->handle = (SYS_ThreadHandle)
             ((size_t) pfnBeginThread(NULL, (unsigned int) thread->stacksize,
                                      RunThreadViaBeginThreadEx,
-                                     pThreadParms, flags, &threadid));
+                                     thread, flags, &threadid));
     } else {
         DWORD threadid = 0;
         thread->handle = CreateThread(NULL, thread->stacksize,
                                       RunThreadViaCreateThread,
-                                      pThreadParms, flags, &threadid);
+                                      thread, flags, &threadid);
     }
     if (thread->handle == NULL) {
         return SDL_SetError("Not enough resources to create thread");