Fixed up the CondBank so that it uses Condition Variables properly.

git-svn-id: svn://svn.code.sf.net/p/sc2/code/trunk@851 8092fc87-c524-0410-9efc-e669fe64eaf9

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

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

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 int  FindSignalChannel ();
-extern void WaitForSignal (void);
+extern void WaitForSignal (int);
 extern void SignalThread (DWORD);
 
 #endif  /* _THREADLIB_H */

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;
+	CondVar   var;
-	DWORD   id;
+	DWORD     id;
-	int     used;
+	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];
+		bank[i].var = CreateCondVar ();
-		sprintf (str, "bank sem %d", i);
-		bank[i].var = CreateSemaphore (0, str); 
 		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
+int
-LockSignalMutex ()
+FindSignalChannel ()
-{
-	LockMutex (bank_mutex);
-}
-void
-WaitForSignal ()
 {
 	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;
+			LockMutex (bank[i].control);
-			break;
+			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
+		// fprintf (stderr, "Thread %lu waiting on cond var %d (control: %p)\n", me, i, bank[i].control);
-		SetSemaphore (bank[i].var);
+		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;
+			UnlockMutex (bank_mutex);
-			ResetSemaphoreOwner (bank[i].var);
+			// fprintf (stderr, "Blocking on var %d's control: %p\n", i, bank[i].control);
-			ClearSemaphore (bank[i].var);
+			LockMutex (bank[i].control);
-			break;
+			// 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);
 }
-

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) \

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 ();
 }