From f33d37976cab056e7fe3d86f275095782a5234e9 Mon Sep 17 00:00:00 2001
From: Romain Vimont <rom@rom1v.com>
Date: Sat, 26 Jun 2021 15:29:08 +0200
Subject: [PATCH] Fix assertion race condition in debug mode

Commit 21d206f360535047e16cc4ea7de889a55bd9444d 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.
---
 app/src/util/thread.c | 17 +++++++++++------
 app/src/util/thread.h |  6 ++++--
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/app/src/util/thread.c b/app/src/util/thread.c
index a0a99f20..de05365d 100644
--- a/app/src/util/thread.c
+++ b/app/src/util/thread.c
@@ -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;
diff --git a/app/src/util/thread.h b/app/src/util/thread.h
index d23e1432..dd3a630e 100644
--- a/app/src/util/thread.h
+++ b/app/src/util/thread.h
@@ -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;