[glib] winxp threads: fix some condition variable races



commit 541693f42d462f114055fd05d1ac536e0c6f46f5
Author: Ryan Lortie <desrt desrt ca>
Date:   Mon Dec 19 17:35:31 2011 -0500

    winxp threads: fix some condition variable races
    
    There are some races in the condition variable emulation code for
    Windows XP with respect to timeouts while waiting.
    
    First, in the event of a timeout, we never remove the waiter from the
    condition variable.  This can cause crashes later.  That problem was
    found by Rodrigo Rivas Costa.
    
    Second, if the waiting thread times out and exits just as we were about
    to call SetEvent() on its waiter event, we could end up trying to access
    the waiter after it was closed/freed.  We need to hold on to the lock a
    little bit longer to ensure that that's not possible.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=666551

 glib/gthread-win32.c |   32 ++++++++++++++++++++++++++------
 1 files changed, 26 insertions(+), 6 deletions(-)
---
diff --git a/glib/gthread-win32.c b/glib/gthread-win32.c
index 29065b5..3ec908d 100644
--- a/glib/gthread-win32.c
+++ b/glib/gthread-win32.c
@@ -520,8 +520,9 @@ static DWORD            g_thread_xp_waiter_tls;
 typedef struct _GThreadXpWaiter GThreadXpWaiter;
 struct _GThreadXpWaiter
 {
-  HANDLE                    event;
-  volatile GThreadXpWaiter *next;
+  HANDLE                     event;
+  volatile GThreadXpWaiter  *next;
+  volatile GThreadXpWaiter **my_owner;
 };
 
 static GThreadXpWaiter *
@@ -539,6 +540,7 @@ g_thread_xp_waiter_get (void)
       waiter->event = CreateEvent (0, FALSE, FALSE, NULL);
       if (waiter->event == NULL)
         g_thread_abort (GetLastError (), "CreateEvent");
+      waiter->my_owner = NULL;
 
       TlsSetValue (g_thread_xp_waiter_tls, waiter);
     }
@@ -848,6 +850,7 @@ g_thread_xp_SleepConditionVariableSRW (gpointer cond,
   waiter->next = NULL;
 
   EnterCriticalSection (&g_thread_xp_lock);
+  waiter->my_owner = cv->last_ptr;
   *cv->last_ptr = waiter;
   cv->last_ptr = &waiter->next;
   LeaveCriticalSection (&g_thread_xp_lock);
@@ -857,9 +860,19 @@ g_thread_xp_SleepConditionVariableSRW (gpointer cond,
 
   if (status != WAIT_TIMEOUT && status != WAIT_OBJECT_0)
     g_thread_abort (GetLastError (), "WaitForSingleObject");
-
   g_mutex_lock (mutex);
 
+  if (status == WAIT_TIMEOUT)
+    {
+      EnterCriticalSection (&g_thread_xp_lock);
+      if (waiter->my_owner)
+        {
+          *waiter->my_owner = waiter->next;
+          waiter->my_owner = NULL;
+        }
+      LeaveCriticalSection (&g_thread_xp_lock);
+    }
+
   return status == WAIT_OBJECT_0;
 }
 
@@ -870,17 +883,21 @@ g_thread_xp_WakeConditionVariable (gpointer cond)
   volatile GThreadXpWaiter *waiter;
 
   EnterCriticalSection (&g_thread_xp_lock);
+
   waiter = cv->first;
   if (waiter != NULL)
     {
       cv->first = waiter->next;
-      if (cv->first == NULL)
+      if (cv->first != NULL)
+        cv->first->my_owner = &cv->first;
+      else
         cv->last_ptr = &cv->first;
     }
-  LeaveCriticalSection (&g_thread_xp_lock);
 
   if (waiter != NULL)
     SetEvent (waiter->event);
+
+  LeaveCriticalSection (&g_thread_xp_lock);
 }
 
 static void __stdcall
@@ -890,10 +907,10 @@ g_thread_xp_WakeAllConditionVariable (gpointer cond)
   volatile GThreadXpWaiter *waiter;
 
   EnterCriticalSection (&g_thread_xp_lock);
+
   waiter = cv->first;
   cv->first = NULL;
   cv->last_ptr = &cv->first;
-  LeaveCriticalSection (&g_thread_xp_lock);
 
   while (waiter != NULL)
     {
@@ -901,8 +918,11 @@ g_thread_xp_WakeAllConditionVariable (gpointer cond)
 
       next = waiter->next;
       SetEvent (waiter->event);
+      waiter->my_owner = NULL;
       waiter = next;
     }
+
+  LeaveCriticalSection (&g_thread_xp_lock);
 }
 
 /* {{{2 XP Setup */



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]