Fix assertion race condition in debug mode

Commit 21d206f360 added mutex assertions.

However, the "locker" variable to trace the locker thread id was read
and written by several threads without any protection, so it was racy.

Reported by TSAN.
This commit is contained in:
Romain Vimont 2021-06-26 15:29:08 +02:00
parent 7dca5078e7
commit f33d37976c
2 changed files with 15 additions and 8 deletions

View file

@ -31,7 +31,7 @@ sc_mutex_init(sc_mutex *mutex) {
mutex->mutex = sdl_mutex;
#ifndef NDEBUG
mutex->locker = 0;
atomic_init(&mutex->locker, 0);
#endif
return true;
}
@ -52,7 +52,8 @@ sc_mutex_lock(sc_mutex *mutex) {
abort();
}
mutex->locker = sc_thread_get_id();
atomic_store_explicit(&mutex->locker, sc_thread_get_id(),
memory_order_relaxed);
#else
(void) r;
#endif
@ -62,7 +63,7 @@ void
sc_mutex_unlock(sc_mutex *mutex) {
#ifndef NDEBUG
assert(sc_mutex_held(mutex));
mutex->locker = 0;
atomic_store_explicit(&mutex->locker, 0, memory_order_relaxed);
#endif
int r = SDL_UnlockMutex(mutex->mutex);
#ifndef NDEBUG
@ -83,7 +84,9 @@ sc_thread_get_id(void) {
#ifndef NDEBUG
bool
sc_mutex_held(struct sc_mutex *mutex) {
return mutex->locker == sc_thread_get_id();
sc_thread_id locker_id =
atomic_load_explicit(&mutex->locker, memory_order_relaxed);
return locker_id == sc_thread_get_id();
}
#endif
@ -112,7 +115,8 @@ sc_cond_wait(sc_cond *cond, sc_mutex *mutex) {
abort();
}
mutex->locker = sc_thread_get_id();
atomic_store_explicit(&mutex->locker, sc_thread_get_id(),
memory_order_relaxed);
#else
(void) r;
#endif
@ -127,7 +131,8 @@ sc_cond_timedwait(sc_cond *cond, sc_mutex *mutex, uint32_t ms) {
abort();
}
mutex->locker = sc_thread_get_id();
atomic_store_explicit(&mutex->locker, sc_thread_get_id(),
memory_order_relaxed);
#endif
assert(r == 0 || r == SDL_MUTEX_TIMEDOUT);
return r == 0;

View file

@ -3,6 +3,7 @@
#include "common.h"
#include <stdatomic.h>
#include <stdbool.h>
#include <stdint.h>
@ -12,7 +13,8 @@ typedef struct SDL_mutex SDL_mutex;
typedef struct SDL_cond SDL_cond;
typedef int sc_thread_fn(void *);
typedef unsigned int sc_thread_id;
typedef unsigned sc_thread_id;
typedef atomic_uint sc_atomic_thread_id;
typedef struct sc_thread {
SDL_Thread *thread;
@ -21,7 +23,7 @@ typedef struct sc_thread {
typedef struct sc_mutex {
SDL_mutex *mutex;
#ifndef NDEBUG
sc_thread_id locker;
sc_atomic_thread_id locker;
#endif
} sc_mutex;