Threading: Detect recursive locks in debug builds

src/common-tests/threading_tests.cpp

@@ -20,7 +20,12 @@ TEST(ThreadingMutex, TryLock)
 {
   Threading::Mutex mutex;
   EXPECT_TRUE(mutex.try_lock());
-  EXPECT_FALSE(mutex.try_lock());
+
+  bool acquired_in_thread = true;
+  std::thread thread([&]() { acquired_in_thread = mutex.try_lock(); });
+  thread.join();
+  EXPECT_FALSE(acquired_in_thread);
+
   mutex.unlock();
   EXPECT_TRUE(mutex.try_lock());
   mutex.unlock();
@@ -32,13 +37,11 @@ TEST(ThreadingMutex, StandardLockWrappers)
 
   {
     const std::lock_guard lock(mutex);
-    EXPECT_FALSE(mutex.try_lock());
   }
 
   {
     std::unique_lock lock(mutex);
     EXPECT_TRUE(lock.owns_lock());
-    EXPECT_FALSE(mutex.try_lock());
   }
 
   EXPECT_TRUE(mutex.try_lock());

src/common/threading.cpp

@@ -68,11 +68,31 @@ LOG_CHANNEL(Threading);
 #endif
 
 #ifdef _WIN32
+namespace {
 union FileTimeU64Union
 {
   FILETIME filetime;
   u64 u64time;
 };
+} // namespace
+#endif
+
+#ifdef THREADING_DEBUG_CHECKS
+
+static uintptr_t GetCurrentThreadIdentifier()
+{
+#ifdef _WIN32
+  return static_cast<uintptr_t>(GetCurrentThreadId());
+#elif defined(__APPLE__)
+  u64 thread_id;
+  [[maybe_unused]] const int result = pthread_threadid_np(nullptr, &thread_id);
+  DebugAssert(result == 0);
+  return static_cast<uintptr_t>(thread_id);
+#else
+  return static_cast<uintptr_t>(gettid());
+#endif
+}
+
 #endif
 
 #ifdef __APPLE__
@@ -599,27 +619,51 @@ Threading::Mutex::~Mutex()
 
 void Threading::Mutex::lock()
 {
+#ifdef THREADING_DEBUG_CHECKS
+  DebugAssert(m_owner_thread_id.load(std::memory_order_relaxed) != GetCurrentThreadIdentifier());
+#endif
+
 #ifdef _WIN32
   AcquireSRWLockExclusive(NATIVE_MUTEX_PTR(m_data));
 #else
   [[maybe_unused]] const int result = pthread_mutex_lock(NATIVE_MUTEX_PTR(m_data));
   DebugAssert(result == 0);
 #endif
+
+#ifdef THREADING_DEBUG_CHECKS
+  m_owner_thread_id.store(GetCurrentThreadIdentifier(), std::memory_order_relaxed);
+#endif
 }
 
 bool Threading::Mutex::try_lock()
 {
+#ifdef THREADING_DEBUG_CHECKS
+  DebugAssert(m_owner_thread_id.load(std::memory_order_relaxed) != GetCurrentThreadIdentifier());
+#endif
+
 #ifdef _WIN32
-  return TryAcquireSRWLockExclusive(NATIVE_MUTEX_PTR(m_data)) != FALSE;
+  const bool success = (TryAcquireSRWLockExclusive(NATIVE_MUTEX_PTR(m_data)) != FALSE);
 #else
   const int result = pthread_mutex_trylock(NATIVE_MUTEX_PTR(m_data));
   DebugAssert(result == 0 || result == EBUSY);
-  return result == 0;
+  const bool success = (result == 0);
 #endif
+
+#ifdef THREADING_DEBUG_CHECKS
+  if (success)
+    m_owner_thread_id.store(GetCurrentThreadIdentifier(), std::memory_order_relaxed);
+#endif
+
+  return success;
 }
 
 void Threading::Mutex::unlock()
 {
+#ifdef THREADING_DEBUG_CHECKS
+  DebugAssert(m_owner_thread_id.load(std::memory_order_relaxed) == GetCurrentThreadIdentifier());
+  m_owner_thread_id.store(0, std::memory_order_relaxed);
+#endif
+
 #ifdef _WIN32
   ReleaseSRWLockExclusive(NATIVE_MUTEX_PTR(m_data));
 #else
@@ -679,6 +723,11 @@ void Threading::ConditionVariable::notify_all()
 
 void Threading::ConditionVariable::Wait(Mutex& mutex)
 {
+#ifdef THREADING_DEBUG_CHECKS
+  DebugAssert(mutex.m_owner_thread_id.load(std::memory_order_relaxed) == GetCurrentThreadIdentifier());
+  mutex.m_owner_thread_id.store(0, std::memory_order_relaxed);
+#endif
+
 #ifdef _WIN32
   [[maybe_unused]] const BOOL result =
     SleepConditionVariableSRW(NATIVE_CONDITION_VARIABLE_PTR(m_data), NATIVE_MUTEX_PTR(mutex.m_data), INFINITE, 0);
@@ -688,6 +737,10 @@ void Threading::ConditionVariable::Wait(Mutex& mutex)
     pthread_cond_wait(NATIVE_CONDITION_VARIABLE_PTR(m_data), NATIVE_MUTEX_PTR(mutex.m_data));
   DebugAssert(result == 0);
 #endif
+
+#ifdef THREADING_DEBUG_CHECKS
+  mutex.m_owner_thread_id.store(GetCurrentThreadIdentifier(), std::memory_order_relaxed);
+#endif
 }
 
 u64 Threading::GetThreadCpuTime()

src/common/threading.h

@@ -8,6 +8,10 @@
 #include <atomic>
 #include <functional>
 
+#if defined(_DEBUG) || defined(_DEVEL)
+#define THREADING_DEBUG_CHECKS
+#endif
+
 namespace Threading {
 extern u64 GetThreadCpuTime();
 extern u64 GetThreadTicksPerSecond();
@@ -155,6 +159,10 @@ private:
 #if !defined(_WIN32)
   alignas(void*) u8 m_data[NATIVE_STORAGE_SIZE];
 #endif
+
+#ifdef THREADING_DEBUG_CHECKS
+  std::atomic<uintptr_t> m_owner_thread_id = 0;
+#endif
 };
 
 // --------------------------------------------------------------------------------------