From 45a1a992349d8556e740d1e4ba9086cc04544a0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deuc=D0=B5?= <shurd@sasktel.net> Date: Thu, 18 Jan 2024 12:14:00 -0500 Subject: [PATCH] Fix rwlock_unlock() and rwlock_rdlock() recursion. It would be nice to use TLS for the recusrion counter, but on Windows, it looks like TLS in threads created after the object is created is uninitialized, so we would need some kind of thread start hook thingie which just seems like too much work. Instead, rely on the old standby single-linked list to track. --- src/xpdev/rwlockwrap.c | 76 ++++++++++++++++++++++++++++++++++++++++-- src/xpdev/rwlockwrap.h | 9 +++++ src/xpdev/wraptest.c | 3 ++ 3 files changed, 85 insertions(+), 3 deletions(-) diff --git a/src/xpdev/rwlockwrap.c b/src/xpdev/rwlockwrap.c index cdd8f40a37..22450bc16c 100644 --- a/src/xpdev/rwlockwrap.c +++ b/src/xpdev/rwlockwrap.c @@ -6,6 +6,32 @@ #elif defined(_WIN32) +#include <stdlib.h> + +static struct rwlock_reader_thread * +find_self(rwlock_t *lock, struct rwlock_reader_thread ***prev) +{ + DWORD self = GetCurrentThreadId(); + struct rwlock_reader_thread *ret; + + if (prev) + *prev = &lock->rthreads; + for (ret = NULL; ret; ret = ret->next) { + if (ret->id == self) + return ret; + if (prev) { + *prev = &ret->next; + } + } + ret = calloc(1, sizeof(*ret)); + if (ret == NULL) + return ret; + ret->next = lock->rthreads; + ret->id = self; + lock->rthreads = ret; + return ret; +} + BOOL rwlock_init(rwlock_t *lock) { @@ -15,14 +41,22 @@ rwlock_init(rwlock_t *lock) lock->writers = 0; lock->writers_waiting = 0; lock->writer = (DWORD)-1; + lock->rthreads = NULL; return TRUE; } BOOL rwlock_rdlock(rwlock_t *lock) { + struct rwlock_reader_thread *rc; + EnterCriticalSection(&lock->lk); - while(lock->writers || lock->writers_waiting) { + rc = find_self(lock, NULL); + if (rc == NULL) { + LeaveCriticalSection(&lock->lk); + return FALSE; + } + while(rc->count == 0 && (lock->writers || lock->writers_waiting)) { LeaveCriticalSection(&lock->lk); // Wait for current writer to release EnterCriticalSection(&lock->wlk); @@ -40,12 +74,14 @@ rwlock_rdlock(rwlock_t *lock) } else { lock->readers++; + rc->count++; LeaveCriticalSection(&lock->lk); LeaveCriticalSection(&lock->wlk); return TRUE; } } lock->readers++; + rc->count++; LeaveCriticalSection(&lock->lk); return TRUE; } @@ -54,9 +90,16 @@ BOOL rwlock_tryrdlock(rwlock_t *lock) { BOOL ret = FALSE; + struct rwlock_reader_thread *rc; EnterCriticalSection(&lock->lk); + rc = find_self(lock, NULL); + if (rc == NULL) { + LeaveCriticalSection(&lock->lk); + return FALSE; + } if (lock->writers == 0 && lock->writers_waiting == 0) { + rc->count++; lock->readers++; ret = TRUE; } @@ -109,6 +152,9 @@ BOOL rwlock_unlock(rwlock_t *lock) { BOOL ret = FALSE; + struct rwlock_reader_thread *rc; + struct rwlock_reader_thread **prev; + EnterCriticalSection(&lock->lk); if (lock->writers) { if (lock->writer == GetCurrentThreadId()) { @@ -121,12 +167,36 @@ rwlock_unlock(rwlock_t *lock) return FALSE; } if (lock->readers) { - lock->readers--; - return TRUE; + rc = find_self(lock, NULL); + if (rc && rc->count) { + rc->count--; + lock->readers--; + if (rc->count == 0) { + *prev = rc->next; + free(rc); + } + LeaveCriticalSection(&lock->lk); + return TRUE; + } } + LeaveCriticalSection(&lock->lk); return FALSE; } +BOOL +rwlock_destory(rwlock_t *lock) +{ + EnterCriticalSection(&lock->lk); + if (lock->readers || lock->writers || lock->writers_waiting || lock->rthreads) { + LeaveCriticalSection(&lock->lk); + return FALSE; + } + LeaveCriticalSection(&lock->lk); + DeleteCriticalSection(&lock->lk); + DeleteCriticalSection(&lock->wlk); + return TRUE; +} + #elif defined(__unix__) // All macros... diff --git a/src/xpdev/rwlockwrap.h b/src/xpdev/rwlockwrap.h index 12cea2e14b..279eab9908 100644 --- a/src/xpdev/rwlockwrap.h +++ b/src/xpdev/rwlockwrap.h @@ -12,6 +12,7 @@ typedef pthread_rwlock_t rwlock_t; #define rwlock_wrlock(lock) (pthread_rwlock_wrlock(lock) == 0) #define rwlock_trywrlock(lock) (pthread_rwlock_trywrlock(lock) == 0) #define rwlock_unlock(lock) (pthread_rwlock_unlock(lock) == 0) +#define rwlock_destroy(lock) (pthread_rwlock_destroy(lock) == 0) #elif defined(__BORLANDC__) @@ -22,6 +23,12 @@ typedef pthread_rwlock_t rwlock_t; #include "gen_defs.h" // For windows.h and BOOL!! :( #include "threadwrap.h" +struct rwlock_reader_thread { + struct rwlock_reader_thread *next; + DWORD id; + unsigned count; +}; + typedef struct { CRITICAL_SECTION lk; // Protects access to all elements CRITICAL_SECTION wlk; // Locked by an active writer @@ -29,6 +36,7 @@ typedef struct { unsigned writers; unsigned writers_waiting; DWORD writer; + struct rwlock_reader_thread *rthreads; } rwlock_t; BOOL rwlock_init(rwlock_t *lock); @@ -37,6 +45,7 @@ BOOL rwlock_tryrdlock(rwlock_t *lock); BOOL rwlock_wrlock(rwlock_t *lock); BOOL rwlock_trywrlock(rwlock_t *lock); BOOL rwlock_unlock(rwlock_t *lock); +BOOL rwlock_destory(rwlock_t *lock); #else #error Not implemented diff --git a/src/xpdev/wraptest.c b/src/xpdev/wraptest.c index 121afa6756..465183b081 100644 --- a/src/xpdev/wraptest.c +++ b/src/xpdev/wraptest.c @@ -79,6 +79,9 @@ int main() continue; } rwlock_wrlock_thread(&lock); + if (!rwlock_destroy(&lock)) { + printf("Unable to destroy rwlock\n"); + } } while(0); for(i=0;i<3;i++) { -- GitLab