From 46bb47cf04065bc77ae97d026e11e3255d06f4f8 Mon Sep 17 00:00:00 2001 From: "Ryan C. Gordon" Date: Thu, 26 Mar 2020 22:14:59 -0400 Subject: [PATCH] thread: Put all important SDL_CreateThread internal data into SDL_Thread. 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. --- src/thread/SDL_systhread.h | 4 +- src/thread/SDL_thread.c | 71 ++++++----------------------- src/thread/SDL_thread_c.h | 5 +- src/thread/generic/SDL_systhread.c | 4 +- src/thread/psp/SDL_systhread.c | 6 +-- src/thread/pthread/SDL_systhread.c | 6 +-- src/thread/stdcpp/SDL_systhread.cpp | 6 +-- src/thread/windows/SDL_systhread.c | 39 ++++++---------- 8 files changed, 44 insertions(+), 97 deletions(-) diff --git a/src/thread/SDL_systhread.h b/src/thread/SDL_systhread.h index 250a89b610427..77dc098457df9 100644 --- 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 index c53ddcd0ca64d..55e48c3bcb95d 100644 --- 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 index 9cc52af1aacd0..ac0071e1b0e6e 100644 --- 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 index fcba8d6dfeb63..3a4d6a22eaa0c 100644 --- 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 index c292afad450d2..789f9a0575343 100644 --- 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 index 0a4650a9bbeef..148c47406a954 100644 --- 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 index fc839d3bbc6f6..d1aac63f18a06 100644 --- 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 index a5e4e35a1ca6d..7e3269b2b03ff 100644 --- 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,33 +100,27 @@ 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) { @@ -141,12 +128,12 @@ SDL_SYS_CreateThread(SDL_Thread * thread, void *args) 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");