From 092a0954a00229e6e37bbad5077b6c8451006f97 Mon Sep 17 00:00:00 2001 From: Hayodea Hakol Date: Mon, 22 Sep 2025 20:45:36 -0400 Subject: [PATCH] Locking: Add basic reactive deadlock detection foundation We added a timestamp to each Lockvoker so that we can detect when a lockvoker has been in a qutex for "too long", where "too long" is defined arbitrarily as 500ms. Next we're going to change the way we create callbacks to enable us to more explicitly access the sh_ptr via the callback object. --- CMakeLists.txt | 18 +++++--- include/config.h.in | 5 ++- include/qutex.h | 13 ++++-- include/serializedAsynchronousContinuation.h | 47 ++++++++++++-------- smocore/CMakeLists.txt | 1 + 5 files changed, 53 insertions(+), 31 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index b9968ee..5554d68 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -3,6 +3,7 @@ project(salmanoff VERSION 0.00.004 LANGUAGES CXX) include(CMakeDependentOption) include(${CMAKE_CURRENT_SOURCE_DIR}/cmake/DAPSS.cmake) +include(${CMAKE_CURRENT_SOURCE_DIR}/cmake/DebugOpts.cmake) # Set C++ standard set(CMAKE_CXX_STANDARD 20) @@ -27,16 +28,19 @@ math(EXPR MIND_VOSCILLATOR_FREQ_MS "1000 / ${MIND_VOSCILLATOR_PERIOD_MS}") option(WORLD_USE_BODY_THREAD "Use body thread for world component instead of separate world thread" OFF) -# Qutex deadlock detection configuration -set(DEBUG_CONFIG_QUTEX_DEADLOCK_TIMEOUT_MS 500 CACHE STRING - "Timeout in milliseconds for deadlock detection in qutex system") -if(NOT DEBUG_CONFIG_QUTEX_DEADLOCK_TIMEOUT_MS GREATER 0) - message(FATAL_ERROR "DEBUG_CONFIG_QUTEX_DEADLOCK_TIMEOUT_MS must be a positive integer > 0") -endif() - # Test configuration option(ENABLE_TESTS "Enable building tests" OFF) +# Set the debug locks variable for config.h +if(ENABLE_DEBUG_LOCKS) + set(CONFIG_ENABLE_DEBUG_LOCKS TRUE) +else() + set(CONFIG_ENABLE_DEBUG_LOCKS FALSE) +endif() + +# Set the timeout variable for config.h +set(CONFIG_DEBUG_QUTEX_DEADLOCK_TIMEOUT_MS ${DEBUG_QUTEX_DEADLOCK_TIMEOUT_MS}) + # Configure config.h configure_file( ${CMAKE_CURRENT_SOURCE_DIR}/include/config.h.in diff --git a/include/config.h.in b/include/config.h.in index 1ab4536..3e01e3b 100644 --- a/include/config.h.in +++ b/include/config.h.in @@ -12,8 +12,9 @@ /* World thread configuration */ #cmakedefine WORLD_USE_BODY_THREAD -/* Qutex deadlock detection configuration */ -#define DEBUG_CONFIG_QUTEX_DEADLOCK_TIMEOUT_MS @DEBUG_CONFIG_QUTEX_DEADLOCK_TIMEOUT_MS@ +/* Debug locking configuration */ +#cmakedefine CONFIG_ENABLE_DEBUG_LOCKS +#cmakedefine CONFIG_DEBUG_QUTEX_DEADLOCK_TIMEOUT_MS @DEBUG_QUTEX_DEADLOCK_TIMEOUT_MS@ /* Cross-compilation configuration */ #cmakedefine CMAKE_CROSSCOMPILING diff --git a/include/qutex.h b/include/qutex.h index ad82c8e..7a50573 100644 --- a/include/qutex.h +++ b/include/qutex.h @@ -1,6 +1,7 @@ #ifndef QUTEX_H #define QUTEX_H +#include #include #include #include @@ -24,8 +25,12 @@ public: /** * @brief Constructor */ - Qutex(const std::string &_name) - : isOwned(false), name(_name) + Qutex([[maybe_unused]] const std::string &_name) + : +#ifdef CONFIG_ENABLE_DEBUG_LOCKS + name(_name), +#endif + isOwned(false) {} /** @@ -85,10 +90,12 @@ public: void release(); public: +#ifdef CONFIG_ENABLE_DEBUG_LOCKS + std::string name; +#endif SpinLock lock; LockerAndInvokerBase::List queue; bool isOwned; - std::string name; }; } // namespace smo diff --git a/include/serializedAsynchronousContinuation.h b/include/serializedAsynchronousContinuation.h index e146c9c..f4a372d 100644 --- a/include/serializedAsynchronousContinuation.h +++ b/include/serializedAsynchronousContinuation.h @@ -1,6 +1,7 @@ #ifndef SERIALIZED_ASYNCHRONOUS_CONTINUATION_H #define SERIALIZED_ASYNCHRONOUS_CONTINUATION_H +#include #include #include #include @@ -63,10 +64,12 @@ public: const std::shared_ptr& target, InvocationTargetT invocationTarget) : LockerAndInvokerBase(&serializedContinuation), +#ifdef CONFIG_ENABLE_DEBUG_LOCKS + creationTimestamp(std::chrono::steady_clock::now()), +#endif serializedContinuation(serializedContinuation), target(target), - invocationTarget(std::move(invocationTarget)), - creationTimestamp(std::chrono::steady_clock::now()) + invocationTarget(std::move(invocationTarget)) { firstWake(); } @@ -85,27 +88,19 @@ public: } Qutex *firstFailedQutexPtr = nullptr; - bool isDeadlockLikely = isDeadlockLikely(); + bool deadlockLikely = isDeadlockLikely(); if (!serializedContinuation.requiredLocks.tryAcquireOrBackOff( - *this, (isDeadlockLikely ? &firstFailedQutexPtr : nullptr))) + *this, (deadlockLikely ? &firstFailedQutexPtr : nullptr))) { // Just allow this lockvoker to be dropped from its io_service. allowAwakening(); - - if (!isDeadlockLikely) + if (!deadlockLikely) { return; } - Qutex &firstFailedQutex = *firstFailedQutexPtr; - - std::cerr << __func__ << ": Deadlock likely: " - << "Lockvoker has been waiting for " - << std::chrono::duration_cast( - std::chrono::steady_clock::now() - creationTimestamp) - .count() - << "ms, failed on qutex @" << &firstFailedQutex - << " (" << firstFailedQutex.name << ")" << std::endl; - +#ifdef CONFIG_ENABLE_DEBUG_LOCKS + handleLikelyDeadlock(*firstFailedQutexPtr); +#endif return; } @@ -177,21 +172,35 @@ public: awaken(true); } - // Check if CONFIG_QUTEX_DEADLOCK_TIMEOUT_MS has elapsed since creation + // Has CONFIG_DEBUG_QUTEX_DEADLOCK_TIMEOUT_MS elapsed since creation? bool isDeadlockLikely() const { +#ifdef CONFIG_ENABLE_DEBUG_LOCKS auto now = std::chrono::steady_clock::now(); auto elapsed = std::chrono::duration_cast( now - creationTimestamp); - return elapsed.count() >= DEBUG_CONFIG_QUTEX_DEADLOCK_TIMEOUT_MS; + return elapsed.count() >= CONFIG_DEBUG_QUTEX_DEADLOCK_TIMEOUT_MS; +#else + return false; +#endif } + /** + * @brief Handle a likely deadlock situation by logging debug information + * @param firstFailedQutex The first qutex that failed acquisition + */ +#ifdef CONFIG_ENABLE_DEBUG_LOCKS + void handleLikelyDeadlock(Qutex& firstFailedQutex); +#endif + private: +#ifdef CONFIG_ENABLE_DEBUG_LOCKS + std::chrono::steady_clock::time_point creationTimestamp; +#endif SerializedAsynchronousContinuation &serializedContinuation; InvocationTargetT invocationTarget; std::shared_ptr target; - std::chrono::steady_clock::time_point creationTimestamp; }; }; diff --git a/smocore/CMakeLists.txt b/smocore/CMakeLists.txt index 1fd9bf8..b2047c4 100644 --- a/smocore/CMakeLists.txt +++ b/smocore/CMakeLists.txt @@ -12,6 +12,7 @@ add_library(smocore STATIC qutex.cpp lockerAndInvokerBase.cpp lockSet.cpp + serializedAsynchronousContinuation.cpp # Body body/body.cpp