From fdc594e963b52b1b554fbbc522b91937a39bb5dc Mon Sep 17 00:00:00 2001 From: Ryan Lortie Date: Mon, 26 Sep 2011 04:44:41 -0400 Subject: [PATCH] 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 --- glib/gthread-win32.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/glib/gthread-win32.c b/glib/gthread-win32.c index f1533c7..2517c4b 100644 --- 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); -- 2.7.4