diff --git a/sc2/ChangeLog b/sc2/ChangeLog index ad3320e86..3834a22c7 100644 --- a/sc2/ChangeLog +++ b/sc2/ChangeLog @@ -1,4 +1,6 @@ Changes towards version 0.3: +- Restored the CondBank to actually use condition variables properly + (resolves a race condition under OpenBSD) - Removed aspects of the legacy graphics code that are never used or that are redundant. More 'C-like' use of the PRIMITIVE datatype. - Fix various graphics glitches during dialog. Especially Spathi Eye,and ZFP diff --git a/sc2/src/sc2code/libs/graphics/tfb_draw.c b/sc2/src/sc2code/libs/graphics/tfb_draw.c index 099c2401f..5e886436a 100644 --- a/sc2/src/sc2code/libs/graphics/tfb_draw.c +++ b/sc2/src/sc2code/libs/graphics/tfb_draw.c @@ -197,14 +197,14 @@ void TFB_DrawScreen_WaitForSignal (void) { TFB_DrawCommand DrawCommand; + int channel; DrawCommand.Type = TFB_DRAWCOMMANDTYPE_SENDSIGNAL; - // We need to lock the mutex before enqueueing the DC to prevent races - LockSignalMutex (); Lock_DCQ (1); + channel = FindSignalChannel (); TFB_BatchReset (); TFB_EnqueueDrawCommand(&DrawCommand); Unlock_DCQ(); - WaitForSignal (); + WaitForSignal (channel); } void diff --git a/sc2/src/sc2code/libs/threadlib.h b/sc2/src/sc2code/libs/threadlib.h index fc12fee62..89347bb66 100644 --- a/sc2/src/sc2code/libs/threadlib.h +++ b/sc2/src/sc2code/libs/threadlib.h @@ -137,13 +137,14 @@ typedef void *CondVar; extern CondVar CreateCondVar (void); extern void DestroyCondVar (CondVar); extern void WaitCondVar (CondVar); +extern void WaitProtectedCondVar (CondVar, Mutex); extern void SignalCondVar (CondVar); extern void BroadcastCondVar (CondVar); extern DWORD CurrentThreadID (void); -extern void LockSignalMutex(void); -extern void WaitForSignal (void); +extern int FindSignalChannel (); +extern void WaitForSignal (int); extern void SignalThread (DWORD); #endif /* _THREADLIB_H */ diff --git a/sc2/src/sc2code/libs/threads/condbank.c b/sc2/src/sc2code/libs/threads/condbank.c index 692703fe5..577568c9d 100644 --- a/sc2/src/sc2code/libs/threads/condbank.c +++ b/sc2/src/sc2code/libs/threads/condbank.c @@ -30,12 +30,12 @@ #define CONDVAR_BANK_SIZE 10 static Mutex bank_mutex; -static int wait = 0, signal = 0; static struct { - Semaphore var; - DWORD id; - int used; + CondVar var; + DWORD id; + int used; + Mutex control; } bank[CONDVAR_BANK_SIZE]; void @@ -45,10 +45,9 @@ init_cond_bank () bank_mutex = CreateMutex (); for (i = 0; i < CONDVAR_BANK_SIZE; i++) { - char str[20]; - sprintf (str, "bank sem %d", i); - bank[i].var = CreateSemaphore (0, str); + bank[i].var = CreateCondVar (); bank[i].id = bank[i].used = 0; + bank[i].control = CreateMutex (); } } @@ -59,33 +58,37 @@ uninit_cond_bank () for (i = 0; i < CONDVAR_BANK_SIZE; i++) { DestroyCondVar (bank[i].var); + DestroyMutex (bank[i].control); } DestroyMutex (bank_mutex); } -void -LockSignalMutex () -{ - LockMutex (bank_mutex); -} -void -WaitForSignal () +int +FindSignalChannel () { int i; - int index = -1; - DWORD me = CurrentThreadID (); + + LockMutex (bank_mutex); for (i = 0; i < CONDVAR_BANK_SIZE; i++) { if (!bank[i].used) { - index = i; - break; + LockMutex (bank[i].control); + return i; } } - if (index == -1) + return -1; +} + +void +WaitForSignal (int i) +{ + DWORD me = CurrentThreadID (); + + if (i == -1) { /* The bank is full! */ - fprintf(stderr, "Condvar bank is full, %ul is waiting on DCQ.", me); + fprintf(stderr, "Condvar bank is full, %lu is waiting on DCQ.\n", me); UnlockMutex (bank_mutex); WaitCondVar (RenderingCond); } @@ -93,12 +96,14 @@ WaitForSignal () { bank[i].used = 1; bank[i].id = me; - // Initialize the Semaphore to a value of '0' in case it isn't already - while (SemaphoreValue (bank[i].var)) - SetSemaphore (bank[i].var); UnlockMutex (bank_mutex); - // Block on Semaphore until it is cleared by the Signal - SetSemaphore (bank[i].var); + // fprintf (stderr, "Thread %lu waiting on cond var %d (control: %p)\n", me, i, bank[i].control); + WaitProtectedCondVar (bank[i].var, bank[i].control); + // fprintf (stderr, "Thread %lu signaled via cond var %d\n", me, i); + UnlockMutex (bank[i].control); + LockMutex (bank_mutex); + bank[i].used = bank[i].id = 0; + UnlockMutex (bank_mutex); } } @@ -111,14 +116,15 @@ SignalThread (DWORD id) { if (bank[i].used && bank[i].id == id) { - bank[i].id = bank[i].used = 0; - ResetSemaphoreOwner (bank[i].var); - ClearSemaphore (bank[i].var); - break; + UnlockMutex (bank_mutex); + // fprintf (stderr, "Blocking on var %d's control: %p\n", i, bank[i].control); + LockMutex (bank[i].control); + // fprintf (stderr, "Signaling var %d, thread %lu, control %p\n", i, id, bank[i].control); + SignalCondVar (bank[i].var); + UnlockMutex (bank[i].control); + return; } } - if (i == CONDVAR_BANK_SIZE) - fprintf (stderr, "Warning: Couldn't find thread to signal!\n"); + fprintf (stderr, "Warning: Couldn't find thread to signal!\n"); UnlockMutex (bank_mutex); } - diff --git a/sc2/src/sc2code/libs/threads/sdl/sdlthreads.h b/sc2/src/sc2code/libs/threads/sdl/sdlthreads.h index deef2c8a4..029bbdd1e 100644 --- a/sc2/src/sc2code/libs/threads/sdl/sdlthreads.h +++ b/sc2/src/sc2code/libs/threads/sdl/sdlthreads.h @@ -91,6 +91,8 @@ typedef SDL_cond *NativeCondVar; extern void SDLWrapper_WaitCondVar (CondVar candvar); #define NativeWaitCondVar(condvar) \ SDLWrapper_WaitCondVar ((condvar)) +#define NativeWaitProtectedCondVar(condvar, mutex) \ + SDL_CondWait ((condvar), (mutex)) #define NativeSignalCondVar(condvar) \ SDL_CondSignal ((condvar)) #define NativeBroadcastCondVar(condvar) \ diff --git a/sc2/src/sc2code/libs/threads/thrcommon.c b/sc2/src/sc2code/libs/threads/thrcommon.c index 89d3bc915..a48d79e05 100644 --- a/sc2/src/sc2code/libs/threads/thrcommon.c +++ b/sc2/src/sc2code/libs/threads/thrcommon.c @@ -608,27 +608,39 @@ CreateCondVar () { return NativeCreateCondVar (); } -void DestroyCondVar (CondVar cv) + +void +DestroyCondVar (CondVar cv) { - NativeDestroyCondVar (cv); + NativeDestroyCondVar ((CondVar)cv); } -void WaitCondVar (CondVar cv) +void +WaitCondVar (CondVar cv) { - NativeWaitCondVar (cv); + NativeWaitCondVar ((NativeCondVar)cv); } -void SignalCondVar (CondVar cv) +void +WaitProtectedCondVar (CondVar cv, Mutex m) { - NativeSignalCondVar (cv); + NativeWaitProtectedCondVar ((NativeCondVar)cv, (NativeMutex)m); } -void BroadcastCondVar (CondVar cv) +void +SignalCondVar (CondVar cv) { - NativeBroadcastCondVar (cv); + NativeSignalCondVar ((NativeCondVar)cv); } -DWORD CurrentThreadID () +void +BroadcastCondVar (CondVar cv) +{ + NativeBroadcastCondVar ((NativeCondVar)cv); +} + +DWORD +CurrentThreadID () { return (DWORD)NativeThreadID (); }