From 831305b5dfa48b3a6f49af19e280f2e687866f25 Mon Sep 17 00:00:00 2001 From: Stenzek Date: Tue, 22 Sep 2026 01:14:01 +1000 Subject: [PATCH] Threading: Detect recursive locks in debug builds --- src/common-tests/threading_tests.cpp | 9 +++-- src/common/threading.cpp | 57 +++++++++++++++++++++++++++- src/common/threading.h | 8 ++++ 3 files changed, 69 insertions(+), 5 deletions(-) diff --git a/src/common-tests/threading_tests.cpp b/src/common-tests/threading_tests.cpp index 85adba12d..b51a4eba5 100644 --- a/src/common-tests/threading_tests.cpp +++ b/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()); diff --git a/src/common/threading.cpp b/src/common/threading.cpp index 2f5dbdcff..af3fa7cad 100644 --- a/src/common/threading.cpp +++ b/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(GetCurrentThreadId()); +#elif defined(__APPLE__) + u64 thread_id; + [[maybe_unused]] const int result = pthread_threadid_np(nullptr, &thread_id); + DebugAssert(result == 0); + return static_cast(thread_id); +#else + return static_cast(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() diff --git a/src/common/threading.h b/src/common/threading.h index 1215ed552..ae01421e2 100644 --- a/src/common/threading.h +++ b/src/common/threading.h @@ -8,6 +8,10 @@ #include #include +#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 m_owner_thread_id = 0; +#endif }; // --------------------------------------------------------------------------------------