From 702455687a240155200b792647d45f24ddd17f26 Mon Sep 17 00:00:00 2001 From: mcmartin Date: Fri, 9 Jan 2004 19:11:13 +0000 Subject: [PATCH] Thread creation and destruction is now handled by the main loop. Some modifications to the initialization code were also necessary to keep the system from deadlocking itself immediately; only subthreads may now call CreateThread or AssignTask. All threads are guaranteed to get a WaitThread () on them when they're finished now, which hopefully will stop thread ID leaking (bug #561). git-svn-id: svn://svn.code.sf.net/p/sc2/code/trunk@1311 8092fc87-c524-0410-9efc-e669fe64eaf9 --- sc2/ChangeLog | 3 + sc2/src/sc2code/libs/threadlib.h | 15 ++ sc2/src/sc2code/libs/threads/sdl/sdlthreads.c | 9 +- sc2/src/sc2code/libs/threads/sdl/sdlthreads.h | 2 + sc2/src/sc2code/libs/threads/thrcommon.c | 161 +++++++++++++++++- sc2/src/sc2code/starcon.c | 8 +- sc2/src/starcon2.c | 22 ++- 7 files changed, 212 insertions(+), 8 deletions(-) diff --git a/sc2/ChangeLog b/sc2/ChangeLog index 2df14b5c9..0da4a5b6d 100644 --- a/sc2/ChangeLog +++ b/sc2/ChangeLog @@ -1,4 +1,7 @@ Changes towards version 0.4: +- Thread code refactoring: only the main thread will actually spawn + threads, and thread IDs are properly recycled with SDL_WaitThead () + once they're done. (With luck, this will fix bug #561) -Michael - Sound code refactoring: core api is now virtualized, MixSDL is divided to generic mixer and driver entities - Mika - Optimized MixSDL mixing and resampling routines diff --git a/sc2/src/sc2code/libs/threadlib.h b/sc2/src/sc2code/libs/threadlib.h index 9b83cff4c..903bd1399 100644 --- a/sc2/src/sc2code/libs/threadlib.h +++ b/sc2/src/sc2code/libs/threadlib.h @@ -77,6 +77,11 @@ enum SYNC_CLASS_RESOURCE = (1 << 3) /* Involves system resources (_MemoryLock) */ }; +/* Note. NEVER call CreateThread from the main thread, or deadlocks + are guaranteed. Use StartThread instead (which doesn't wait around + for the main thread to actually create the thread and return + it). */ + #ifdef NAMED_SYNCHRO /* Logical OR of all classes we want to track. */ #define TRACK_CONTENTION_CLASSES (SYNC_CLASS_TOPLEVEL) @@ -84,6 +89,7 @@ enum /* Prototypes with the "name" field */ Thread CreateThread_Core (ThreadFunction func, void *data, SDWORD stackSize, const char *name); +void StartThread_Core (ThreadFunction func, void *data, SDWORD stackSize, const char *name); Semaphore CreateSemaphore_Core (DWORD initial, const char *name, DWORD syncClass); Mutex CreateMutex_Core (const char *name, DWORD syncClass); RecursiveMutex CreateRecursiveMutex_Core (const char *name, DWORD syncClass); @@ -93,6 +99,8 @@ CondVar CreateCondVar_Core (const char *name, DWORD syncClass); #define CreateThread(func, data, stackSize, name) \ CreateThread_Core ((func), (data), (stackSize), (name)) +#define StartThread(func, data, stackSize, name) \ + StartThread_Core ((func), (data), (stackSize), (name)) #define CreateSemaphore(initial, name, syncClass) \ CreateSemaphore_Core ((initial), (name), (syncClass)) #define CreateMutex(name, syncClass) \ @@ -106,6 +114,7 @@ CondVar CreateCondVar_Core (const char *name, DWORD syncClass); /* Prototypes without the "name" field. */ Thread CreateThread_Core (ThreadFunction func, void *data, SDWORD stackSize); +void StartThread_Core (ThreadFunction func, void *data, SDWORD stackSize); Semaphore CreateSemaphore_Core (DWORD initial); Mutex CreateMutex_Core (void); RecursiveMutex CreateRecursiveMutex_Core (void); @@ -117,6 +126,8 @@ CondVar CreateCondVar_Core (void); #define CreateThread(func, data, stackSize, name) \ CreateThread_Core ((func), (data), (stackSize)) +#define StartThread(func, data, stackSize, name) \ + StartThread_Core ((func), (data), (stackSize)) #define CreateSemaphore(initial, name, syncClass) \ CreateSemaphore_Core ((initial)) #define CreateMutex(name, syncClass) \ @@ -134,9 +145,13 @@ ThreadLocal *GetMyThreadLocal (void); void SleepThread (TimePeriod timePeriod); void SleepThreadUntil (TimeCount wakeTime); +void DestroyThread (Thread); void TaskSwitch (void); void WaitThread (Thread thread, int *status); +void FinishThread (Thread); +void ProcessThreadLifecycles (void); + #ifdef PROFILE_THREADS void PrintThreadsStats (void); #endif /* PROFILE_THREADS */ diff --git a/sc2/src/sc2code/libs/threads/sdl/sdlthreads.c b/sc2/src/sc2code/libs/threads/sdl/sdlthreads.c index 6cf82c5a8..de2f0a9ce 100644 --- a/sc2/src/sc2code/libs/threads/sdl/sdlthreads.c +++ b/sc2/src/sc2code/libs/threads/sdl/sdlthreads.c @@ -220,10 +220,17 @@ ThreadHelper (void *startInfo) { UnQueueThread (thread); DestroyThreadLocal (thread->localData); - HFree (thread); + FinishThread (thread); + /* Destroying the thread is the responsibility of ProcessThreadLifecycles() */ return result; } +void +DestroyThread_SDL (Thread t) +{ + HFree (t); +} + Thread CreateThread_SDL (ThreadFunction func, void *data, SDWORD stackSize #ifdef NAMED_SYNCHRO diff --git a/sc2/src/sc2code/libs/threads/sdl/sdlthreads.h b/sc2/src/sc2code/libs/threads/sdl/sdlthreads.h index 1a5f7fb5b..19b17a805 100644 --- a/sc2/src/sc2code/libs/threads/sdl/sdlthreads.h +++ b/sc2/src/sc2code/libs/threads/sdl/sdlthreads.h @@ -50,6 +50,7 @@ void SleepThread_SDL (TimeCount sleepTime); void SleepThreadUntil_SDL (TimeCount wakeTime); void TaskSwitch_SDL (void); void WaitThread_SDL (Thread thread, int *status); +void DestroyThread_SDL (Thread thread); void DestroyMutex_SDL (Mutex m); void LockMutex_SDL (Mutex m); @@ -79,6 +80,7 @@ int GetRecursiveMutexDepth_SDL (RecursiveMutex m); #define NativeSleepThreadUntil SleepThreadUntil_SDL #define NativeTaskSwitch TaskSwitch_SDL #define NativeWaitThread WaitThread_SDL +#define NativeDestroyThread DestroyThread_SDL #define NativeCreateMutex CreateMutex_SDL #define NativeDestroyMutex DestroyMutex_SDL diff --git a/sc2/src/sc2code/libs/threads/thrcommon.c b/sc2/src/sc2code/libs/threads/thrcommon.c index 7afb8bf01..255299b32 100644 --- a/sc2/src/sc2code/libs/threads/thrcommon.c +++ b/sc2/src/sc2code/libs/threads/thrcommon.c @@ -18,23 +18,140 @@ */ #include +#include #include "libs/threadlib.h" #include "libs/timelib.h" #include "libs/misc.h" #include "thrcommon.h" +#define LIFECYCLE_SIZE 8 +typedef struct { + ThreadFunction func; + void *data; + SDWORD stackSize; + Semaphore sem; + Thread value; +#ifdef NAMED_SYNCHRO + const char *name; +#endif +} SpawnRequest_struct; + +typedef SpawnRequest_struct *SpawnRequest; + +static Mutex lifecycleMutex; +static SpawnRequest pendingBirth[LIFECYCLE_SIZE]; +static Thread pendingDeath[LIFECYCLE_SIZE]; + void InitThreadSystem (void) { + int i; NativeInitThreadSystem (); + for (i = 0; i < LIFECYCLE_SIZE; i++) + { + pendingBirth[i] = NULL; + pendingDeath[i] = NULL; + } + lifecycleMutex = CreateMutex ("Thread Lifecycle Mutex", SYNC_CLASS_RESOURCE); } void UnInitThreadSystem (void) { NativeUnInitThreadSystem (); + DestroyMutex (lifecycleMutex); } +static Thread +FlagStartThread (SpawnRequest s) +{ + int i; + LockMutex (lifecycleMutex); + for (i = 0; i < LIFECYCLE_SIZE; i++) + { + if (pendingBirth[i] == NULL) + { + pendingBirth[i] = s; + UnlockMutex (lifecycleMutex); + if (s->sem) + { + Thread result; + SetSemaphore (s->sem); + DestroySemaphore (s->sem); + result = s->value; + HFree (s); + return result; + } + return NULL; + } + } + fprintf (stderr, "Thread Lifecycle array filled. This is a fatal error! Make LIFECYCLE_SIZE something larger than %d.\n", LIFECYCLE_SIZE); + exit (-1); +} + +void +FinishThread (Thread thread) +{ + int i; + LockMutex (lifecycleMutex); + for (i = 0; i < LIFECYCLE_SIZE; i++) + { + if (pendingDeath[i] == NULL) + { + pendingDeath[i] = thread; + UnlockMutex (lifecycleMutex); + return; + } + } + fprintf (stderr, "Thread Lifecycle array filled. This is a fatal error! Make LIFECYCLE_SIZE something larger than %d.\n", LIFECYCLE_SIZE); + exit (-1); +} + +/* Only call from main thread! */ +void +ProcessThreadLifecycles (void) +{ + int i; + LockMutex (lifecycleMutex); + for (i = 0; i < LIFECYCLE_SIZE; i++) + { + SpawnRequest s = pendingBirth[i]; + if (s != NULL) + { +#ifdef NAMED_SYNCHRO + s->value = NativeCreateThread (s->func, s->data, s->stackSize, s->name); +#else + s->value = NativeCreateThread (s->func, s->data, s->stackSize); +#endif + if (s->sem) + { + ClearSemaphore (s->sem); + /* The spawning thread's FlagStartThread will clean up s */ + } + else + { + /* The thread value has been lost to the game logic. We must + clean up s ourself. */ + HFree (s); + } + pendingBirth[i] = NULL; + } + } + + for (i = 0; i < LIFECYCLE_SIZE; i++) + { + Thread t = pendingDeath[i]; + if (t != NULL) + { + WaitThread (t, NULL); + pendingDeath[i] = NULL; + DestroyThread (t); + } + } + UnlockMutex (lifecycleMutex); +} + + /* The Create routines look different based on whether NAMED_SYNCHRO is defined or not. */ @@ -42,7 +159,25 @@ UnInitThreadSystem (void) Thread CreateThread_Core (ThreadFunction func, void *data, SDWORD stackSize, const char *name) { - return NativeCreateThread (func, data, stackSize, name); + SpawnRequest s = HMalloc(sizeof (SpawnRequest_struct)); + s->func = func; + s->data = data; + s->stackSize = stackSize; + s->name = name; + s->sem = CreateSemaphore (0, "SpawnRequest semaphore", SYNC_CLASS_RESOURCE); + return FlagStartThread (s); +} + +void +StartThread_Core (ThreadFunction func, void *data, SDWORD stackSize, const char *name) +{ + SpawnRequest s = HMalloc(sizeof (SpawnRequest_struct)); + s->func = func; + s->data = data; + s->stackSize = stackSize; + s->name = name; + s->sem = NULL; + FlagStartThread (s); } Mutex @@ -74,7 +209,23 @@ CreateCondVar_Core (const char *name, DWORD syncClass) Thread CreateThread_Core (ThreadFunction func, void *data, SDWORD stackSize) { - return NativeCreateThread (func, data, stackSize); + SpawnRequest s = HMalloc(sizeof (SpawnRequest_struct)); + s->func = func; + s->data = data; + s->stackSize = stackSize; + s->sem = CreateSemaphore (0, "SpawnRequest semaphore", SYNC_CLASS_RESOURCE); + return FlagStartThread (s); +} + +void +StartThread_Core (ThreadFunction func, void *data, SDWORD stackSize) +{ + SpawnRequest s = HMalloc(sizeof (SpawnRequest_struct)); + s->func = func; + s->data = data; + s->stackSize = stackSize; + s->sem = NULL; + FlagStartThread (s); } Mutex @@ -102,6 +253,12 @@ CreateCondVar_Core (void) } #endif +void +DestroyThread (Thread t) +{ + NativeDestroyThread (t); +} + ThreadLocal * CreateThreadLocal (void) { diff --git a/sc2/src/sc2code/starcon.c b/sc2/src/sc2code/starcon.c index 5f70befb7..49ca15780 100644 --- a/sc2/src/sc2code/starcon.c +++ b/sc2/src/sc2code/starcon.c @@ -849,6 +849,9 @@ EventHandler (BYTE selector) #define DEBUG_PSYTRON 0 +/* TODO: Remove these declarations once threading is gone. */ +extern int snddriver, soundflags; + int Starcon2Main(void* blah) { #if DEBUG_PSYTRON || CREATE_JOURNAL @@ -882,7 +885,10 @@ while (--ac > 0) } } #endif //DEBUG_PSYTRON || CREATE_JOURNAL - + +/* TODO: Put initAudio back in main where it belongs once threading is gone */ +initAudio (snddriver, soundflags); + if (LoadKernel (0,0)) { extern BOOLEAN StartGame (void); diff --git a/sc2/src/starcon2.c b/sc2/src/starcon2.c index 74c46123b..adac72a02 100644 --- a/sc2/src/starcon2.c +++ b/sc2/src/starcon2.c @@ -44,6 +44,10 @@ // Including this is actually necessary on OSX. #endif +/* TODO: Remove these (making them local to main() ) once threading is + gone. */ + +int snddriver, soundflags; static int Check_PC_3DO_opt (const char *value, DWORD mask, const char *opt) @@ -66,9 +70,7 @@ int main (int argc, char *argv[]) { int gfxdriver = TFB_GFXDRIVER_SDL_PURE; - int snddriver = audio_DRIVER_MIXSDL; int gfxflags = 0; - int soundflags = audio_QUALITY_MEDIUM; int width = 640, height = 480, bpp = 16; int vol; int val; @@ -120,6 +122,14 @@ main (int argc, char *argv[]) {0, 0, 0, 0} }; + /* TODO: Once threading is gone, these become local variables + again. In the meantime, they must be global so that + initAudio (in StarCon2Main) can see them. initAudio needed + to be moved there because calling AssignTask in the main + thread doesn't work */ + snddriver = audio_DRIVER_MIXSDL; + soundflags = audio_QUALITY_MEDIUM; + fprintf (stderr, "The Ur-Quan Masters v%d.%d%s (compiled %s %s)\n" "This software comes with ABSOLUTELY NO WARRANTY;\n" "for details see the included 'COPYING' file.\n\n", @@ -357,14 +367,18 @@ main (int argc, char *argv[]) init_communication (); if (gammaset) TFB_SetGamma (gamma); - initAudio (snddriver, soundflags); + /* TODO: Once threading is gone, restore initAudio here. + initAudio calls AssignTask, which currently blocks on + ProcessThreadLifecycles... */ + // initAudio (snddriver, soundflags); TFB_InitInput (TFB_INPUTDRIVER_SDL, 0); - AssignTask (Starcon2Main, 1024, "Starcon2Main"); + StartThread (Starcon2Main, NULL, 1024, "Starcon2Main"); for (;;) { TFB_ProcessEvents (); + ProcessThreadLifecycles (); TFB_FlushGraphics (); }