luc14n0/glib
1
0
Fork 0
mirror of https://gitlab.gnome.org/GNOME/glib.git synced 2025-04-01 21:33:09 +02:00
Code Issues Packages Projects Releases Wiki Activity

winxp threads: fix some condition variable races

Browse Source 
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
This commit is contained in:
Ryan Lortie 2011-12-19 17:35:31 -05:00
parent f3cf8c0ca8
commit 541693f42d
1 changed files with 26 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
glib/gthread-win32.c
View File

@ -520,8 +520,9 @@ static DWORD g_thread_xp_waiter_tls;
typedef struct _GThreadXpWaiter GThreadXpWaiter; typedef struct _GThreadXpWaiter GThreadXpWaiter;
struct _GThreadXpWaiter struct _GThreadXpWaiter
{ {
HANDLE event; HANDLE event;
volatile GThreadXpWaiter *next; volatile GThreadXpWaiter *next;
volatile GThreadXpWaiter **my_owner;
}; };
static GThreadXpWaiter * static GThreadXpWaiter *
@ -539,6 +540,7 @@ g_thread_xp_waiter_get (void)
waiter->event = CreateEvent (0, FALSE, FALSE, NULL); waiter->event = CreateEvent (0, FALSE, FALSE, NULL);
if (waiter->event == NULL) if (waiter->event == NULL)
g_thread_abort (GetLastError (), "CreateEvent"); g_thread_abort (GetLastError (), "CreateEvent");
waiter->my_owner = NULL;
TlsSetValue (g_thread_xp_waiter_tls, waiter); TlsSetValue (g_thread_xp_waiter_tls, waiter);
} }
@ -848,6 +850,7 @@ g_thread_xp_SleepConditionVariableSRW (gpointer cond,
waiter->next = NULL; waiter->next = NULL;
EnterCriticalSection (&g_thread_xp_lock); EnterCriticalSection (&g_thread_xp_lock);
waiter->my_owner = cv->last_ptr;
*cv->last_ptr = waiter; *cv->last_ptr = waiter;
cv->last_ptr = &waiter->next; cv->last_ptr = &waiter->next;
LeaveCriticalSection (&g_thread_xp_lock); LeaveCriticalSection (&g_thread_xp_lock);
@ -857,9 +860,19 @@ g_thread_xp_SleepConditionVariableSRW (gpointer cond,
if (status != WAIT_TIMEOUT && status != WAIT_OBJECT_0) if (status != WAIT_TIMEOUT && status != WAIT_OBJECT_0)
g_thread_abort (GetLastError (), "WaitForSingleObject"); g_thread_abort (GetLastError (), "WaitForSingleObject");
g_mutex_lock (mutex); 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; return status == WAIT_OBJECT_0;
} }
@ -870,17 +883,21 @@ g_thread_xp_WakeConditionVariable (gpointer cond)
volatile GThreadXpWaiter *waiter; volatile GThreadXpWaiter *waiter;
EnterCriticalSection (&g_thread_xp_lock); EnterCriticalSection (&g_thread_xp_lock);
waiter = cv->first; waiter = cv->first;
if (waiter != NULL) if (waiter != NULL)
{ {
cv->first = waiter->next; cv->first = waiter->next;
if (cv->first == NULL) if (cv->first != NULL)
cv->first->my_owner = &cv->first;
else
cv->last_ptr = &cv->first; cv->last_ptr = &cv->first;
} }
LeaveCriticalSection (&g_thread_xp_lock);
if (waiter != NULL) if (waiter != NULL)
SetEvent (waiter->event); SetEvent (waiter->event);
LeaveCriticalSection (&g_thread_xp_lock);
} }
static void __stdcall static void __stdcall
@ -890,10 +907,10 @@ g_thread_xp_WakeAllConditionVariable (gpointer cond)
volatile GThreadXpWaiter *waiter; volatile GThreadXpWaiter *waiter;
EnterCriticalSection (&g_thread_xp_lock); EnterCriticalSection (&g_thread_xp_lock);
waiter = cv->first; waiter = cv->first;
cv->first = NULL; cv->first = NULL;
cv->last_ptr = &cv->first; cv->last_ptr = &cv->first;
LeaveCriticalSection (&g_thread_xp_lock);
while (waiter != NULL) while (waiter != NULL)
{ {
@ -901,8 +918,11 @@ g_thread_xp_WakeAllConditionVariable (gpointer cond)
next = waiter->next; next = waiter->next;
SetEvent (waiter->event); SetEvent (waiter->event);
waiter->my_owner = NULL;
waiter = next; waiter = next;
} }
LeaveCriticalSection (&g_thread_xp_lock);
} }
/* {{{2 XP Setup */ /* {{{2 XP Setup */