# HG changeset patch
# User sesse
# Date 1267718228 0
# Node ID 1001c606f94cdacef52bd53219955f73b45c8679
# Parent  e4e2bcd96a56cd87565be13acba4bb6bbe589c6e
Make emulated Win32 critical sections thread safe.

Earlier, cs->locked was accessed outside the mutex to get around
the problem that default pthread mutexes are not recursive
(ie., you cannot do a double-lock from the same thread), causing
a thread-safety problem, as both detected by Helgrind and showing
up in some multithreaded codecs.

The ideal solution here would be to simply use recursive pthread
mutexes, but there were concerns about reduced debuggability and
possibly portability. Thus, instead, rewrite the critical sections
to be a simple lock count (with owner) protected by a regular mutex.
Whenever a thread wants to enter the critical section and lock_count
is not 0, it sleeps on a special event that tells it when the
critical section is available.

diff -r e4e2bcd96a56 -r 1001c606f94c loader/win32.c
--- a/loader/win32.c	Thu Mar 04 15:56:34 2010 +0000
+++ b/loader/win32.c	Thu Mar 04 15:57:08 2010 +0000
@@ -350,7 +350,8 @@
 {
     pthread_t id;
     pthread_mutex_t mutex;
-    int locked;
+    pthread_cond_t unlocked;
+    int lock_count;
     long deadbeef;
 };
 
@@ -1331,7 +1332,8 @@
 	    return;
 	}
 	pthread_mutex_init(&cs->mutex, NULL);
-	cs->locked = 0;
+	pthread_cond_init(&cs->unlocked, NULL);
+	cs->lock_count = 0;
 	critsecs_list[i].cs_win = c;
 	critsecs_list[i].cs_unix = cs;
 	dbgprintf("InitializeCriticalSection -> itemno=%d, cs_win=%p, cs_unix=%p\n",
@@ -1342,7 +1344,8 @@
 	struct CRITSECT* cs = mreq_private(sizeof(struct CRITSECT) + sizeof(CRITICAL_SECTION),
 					   0, AREATYPE_CRITSECT);
 	pthread_mutex_init(&cs->mutex, NULL);
-	cs->locked=0;
+	pthread_cond_init(&cs->unlocked, NULL);
+	cs->lock_count = 0;
         cs->deadbeef = 0xdeadbeef;
 	*(void**)c = cs;
     }
@@ -1374,12 +1377,17 @@
 #endif
 	dbgprintf("Win32 Warning: Accessed uninitialized Critical Section (%p)!\n", c);
     }
-    if(cs->locked)
-	if(cs->id==pthread_self())
-	    return;
     pthread_mutex_lock(&(cs->mutex));
-    cs->locked=1;
-    cs->id=pthread_self();
+    if (cs->lock_count > 0 && cs->id == pthread_self()) {
+        cs->lock_count++;
+    } else {
+        while (cs->lock_count != 0) {
+            pthread_cond_wait(&(cs->unlocked), &(cs->mutex));
+        }
+        cs->lock_count = 1;
+        cs->id = pthread_self();
+    }
+    pthread_mutex_unlock(&(cs->mutex));
     return;
 }
 static void WINAPI expLeaveCriticalSection(CRITICAL_SECTION* c)
@@ -1396,13 +1404,16 @@
 	dbgprintf("Win32 Warning: Leaving uninitialized Critical Section %p!!\n", c);
 	return;
     }
-    if (cs->locked)
-    {
-	cs->locked=0;
-	pthread_mutex_unlock(&(cs->mutex));
+    pthread_mutex_lock(&(cs->mutex));
+    if (cs->lock_count == 0) {
+        dbgprintf("Win32 Warning: Unlocking unlocked Critical Section %p!!\n", c);
+    } else {
+        cs->lock_count--;
     }
-    else
-	dbgprintf("Win32 Warning: Unlocking unlocked Critical Section %p!!\n", c);
+    if (cs->lock_count == 0) {
+        pthread_cond_signal(&(cs->unlocked));
+    }
+    pthread_mutex_unlock(&(cs->mutex));
     return;
 }
 
@@ -1424,14 +1435,16 @@
 	return;
     }
 
-    if (cs->locked)
+    pthread_mutex_lock(&(cs->mutex));
+    if (cs->lock_count > 0)
     {
-	dbgprintf("Win32 Warning: Deleting unlocked Critical Section %p!!\n", c);
-	pthread_mutex_unlock(&(cs->mutex));
+       dbgprintf("Win32 Warning: Deleting locked Critical Section %p!!\n", c);
     }
+    pthread_mutex_unlock(&(cs->mutex));
 
 #ifndef GARBAGE
     pthread_mutex_destroy(&(cs->mutex));
+    pthread_cond_destroy(&(cs->unlocked));
     // released by GarbageCollector in my_relase otherwise
 #endif
     my_release(cs);