winxp threads: detect SRWLock emulation reentrancy

We lack SRWLock on Windows XP, so we use CRITICAL_SECTION to emulate it
there.  SRWLock is non-recursive, but CRITICAL_SECTION will happily
allow itself to be acquired multiple times by the same thread.

We need to detect if our second acquire succeeded because of the
recursive nature of CRITICAL_SECTION.  In the case of a _lock()
operation, we would normally have deadlocked, so abort.  In the case of
a _trylock() operation, we need to ensure that FALSE is properly
returned.

Problem caught by Matthias Clasen and Chun-wei Fan.

https://bugzilla.gnome.org/show_bug.cgi?id=660096

diff --git a/glib/gthread-win32.c b/glib/gthread-win32.c
index f1533c7..2517c4b 100644 (file)
--- a/glib/gthread-win32.c
+++ b/glib/gthread-win32.c
@@ -645,6 +645,7 @@ typedef struct
 {
   CRITICAL_SECTION  writer_lock;
   gboolean          ever_shared;    /* protected by writer_lock */
+  gboolean          writer_locked;  /* protected by writer_lock */
 
   /* below is only ever touched if ever_shared becomes true */
   CRITICAL_SECTION  atomicity;
@@ -695,6 +696,7 @@ g_thread_xp_get_srwlock (GThreadSRWLock * volatile *lock)
         g_thread_abort (errno, "malloc");
 
       InitializeCriticalSection (&result->writer_lock);
+      result->writer_locked = FALSE;
       result->ever_shared = FALSE;
       *lock = result;
 
@@ -711,6 +713,12 @@ g_thread_xp_AcquireSRWLockExclusive (gpointer mutex)
 
   EnterCriticalSection (&lock->writer_lock);
 
+  /* CRITICAL_SECTION is reentrant, but SRWLock is not.
+   * Detect the deadlock that would occur on later Windows version.
+   */
+  g_assert (!lock->writer_locked);
+  lock->writer_locked = TRUE;
+
   if (lock->ever_shared)
     {
       GThreadXpWaiter *waiter = NULL;
@@ -735,6 +743,17 @@ g_thread_xp_TryAcquireSRWLockExclusive (gpointer mutex)
   if (!TryEnterCriticalSection (&lock->writer_lock))
     return FALSE;
 
+  /* CRITICAL_SECTION is reentrant, but SRWLock is not.
+   * Ensure that this properly returns FALSE (as SRWLock would).
+   */
+  if G_UNLIKELY (lock->writer_locked)
+    {
+      LeaveCriticalSection (&lock->writer_lock);
+      return FALSE;
+    }
+
+  lock->writer_locked = TRUE;
+
   if (lock->ever_shared)
     {
       gboolean available;
@@ -758,6 +777,8 @@ g_thread_xp_ReleaseSRWLockExclusive (gpointer mutex)
 {
   GThreadSRWLock *lock = *(GThreadSRWLock * volatile *) mutex;
 
+  lock->writer_locked = FALSE;
+
   /* We need this until we fix some weird parts of GLib that try to
    * unlock freshly-allocated mutexes.
    */
@@ -789,6 +810,9 @@ g_thread_xp_AcquireSRWLockShared (gpointer mutex)
 
   EnterCriticalSection (&lock->writer_lock);
 
+  /* See g_thread_xp_AcquireSRWLockExclusive */
+  g_assert (!lock->writer_locked);
+
   g_thread_xp_srwlock_become_reader (lock);
 
   LeaveCriticalSection (&lock->writer_lock);
@@ -802,6 +826,13 @@ g_thread_xp_TryAcquireSRWLockShared (gpointer mutex)
   if (!TryEnterCriticalSection (&lock->writer_lock))
     return FALSE;
 
+  /* See g_thread_xp_AcquireSRWLockExclusive */
+  if G_UNLIKELY (lock->writer_locked)
+    {
+      LeaveCriticalSection (&lock->writer_lock);
+      return FALSE;
+    }
+
   g_thread_xp_srwlock_become_reader (lock);
 
   LeaveCriticalSection (&lock->writer_lock);