From 2a60fdd9dfebced68f9bd862344e9f43700731b9 Mon Sep 17 00:00:00 2001 From: Hayodea Hekol Date: Mon, 29 Sep 2025 14:37:16 -0400 Subject: [PATCH] Qutex: currOwner should use registered sh_ptr to Lockvoker This ensures that the Lockvoker object we access from currOwner remains valid past the lifetime of the Lockvoker object that gets copied and invoked by boost::asio --- include/asynchronousContinuation.h | 2 +- include/asynchronousContinuationChainLink.h | 2 +- include/lockerAndInvokerBase.h | 2 +- include/qutex.h | 5 +++-- include/serializedAsynchronousContinuation.h | 2 +- smocore/qutex.cpp | 10 +++++++--- smocore/serializedAsynchronousContinuation.cpp | 15 ++++++++------- 7 files changed, 22 insertions(+), 16 deletions(-) diff --git a/include/asynchronousContinuation.h b/include/asynchronousContinuation.h index 10e725f..60ca47d 100644 --- a/include/asynchronousContinuation.h +++ b/include/asynchronousContinuation.h @@ -68,7 +68,7 @@ public: // Implement the virtual method from AsynchronousContinuationChainLink virtual std::shared_ptr - getCallersContinuation() override + getCallersContinuationShPtr() override { return originalCallback.callerContinuation; } public: diff --git a/include/asynchronousContinuationChainLink.h b/include/asynchronousContinuationChainLink.h index 63fb1b9..3e1ac74 100644 --- a/include/asynchronousContinuationChainLink.h +++ b/include/asynchronousContinuationChainLink.h @@ -20,7 +20,7 @@ public: virtual ~AsynchronousContinuationChainLink() = default; virtual std::shared_ptr - getCallersContinuation() = 0; + getCallersContinuationShPtr() = 0; }; } // namespace smo diff --git a/include/lockerAndInvokerBase.h b/include/lockerAndInvokerBase.h index f0991c1..3e11e0b 100644 --- a/include/lockerAndInvokerBase.h +++ b/include/lockerAndInvokerBase.h @@ -37,7 +37,7 @@ public: * @param qutex The Qutex to get the iterator for * @return Iterator pointing to this lockvoker in the Qutex's queue */ - virtual List::iterator getLockvokerIteratorForQutex(Qutex& qutex) = 0; + virtual List::iterator getLockvokerIteratorForQutex(Qutex& qutex) const = 0; /** * @brief Awaken this lockvoker by posting it to its io_service diff --git a/include/qutex.h b/include/qutex.h index eeb533c..c64d94c 100644 --- a/include/qutex.h +++ b/include/qutex.h @@ -90,13 +90,14 @@ public: void release(); #ifdef CONFIG_ENABLE_DEBUG_LOCKS - LockerAndInvokerBase* getCurrOwner() const { return currOwner; } + std::shared_ptr getCurrOwner() const + { return currOwner; } #endif public: #ifdef CONFIG_ENABLE_DEBUG_LOCKS std::string name; - LockerAndInvokerBase* currOwner; + std::shared_ptr currOwner; #endif SpinLock lock; LockerAndInvokerBase::List queue; diff --git a/include/serializedAsynchronousContinuation.h b/include/serializedAsynchronousContinuation.h index 5b80d08..7f02155 100644 --- a/include/serializedAsynchronousContinuation.h +++ b/include/serializedAsynchronousContinuation.h @@ -154,7 +154,7 @@ public: * @return Iterator pointing to this lockvoker in the Qutex's queue */ LockerAndInvokerBase::List::iterator - getLockvokerIteratorForQutex(Qutex& qutex) override + getLockvokerIteratorForQutex(Qutex& qutex) const override { return serializedContinuation.requiredLocks.getLockUsageDesc( qutex).second; diff --git a/smocore/qutex.cpp b/smocore/qutex.cpp index 0939771..d504c39 100644 --- a/smocore/qutex.cpp +++ b/smocore/qutex.cpp @@ -44,7 +44,9 @@ bool Qutex::tryAcquire( { isOwned = true; #ifdef CONFIG_ENABLE_DEBUG_LOCKS - currOwner = const_cast(&tryingLockvoker); + // Use the stored iterator from the LockSet + auto it = tryingLockvoker.getLockvokerIteratorForQutex(*this); + currOwner = *it; #endif lock.release(); return true; @@ -59,7 +61,7 @@ bool Qutex::tryAcquire( { isOwned = true; #ifdef CONFIG_ENABLE_DEBUG_LOCKS - currOwner = const_cast(&tryingLockvoker); + currOwner = queue.front(); #endif ret = true; } @@ -96,7 +98,9 @@ bool Qutex::tryAcquire( // Not found in rear portion - must be in top X%, so succeed isOwned = true; #ifdef CONFIG_ENABLE_DEBUG_LOCKS - currOwner = const_cast(&tryingLockvoker); + // Use the stored iterator from the LockSet + auto it = tryingLockvoker.getLockvokerIteratorForQutex(*this); + currOwner = *it; #endif lock.release(); return true; diff --git a/smocore/serializedAsynchronousContinuation.cpp b/smocore/serializedAsynchronousContinuation.cpp index 4f0038b..47446d4 100644 --- a/smocore/serializedAsynchronousContinuation.cpp +++ b/smocore/serializedAsynchronousContinuation.cpp @@ -27,9 +27,9 @@ SerializedAsynchronousContinuation * lockvoker. */ for (std::shared_ptr currContin = - this->serializedContinuation.getCallersContinuation(); + this->serializedContinuation.getCallersContinuationShPtr(); currContin != nullptr; - currContin = currContin->getCallersContinuation()) + currContin = currContin->getCallersContinuationShPtr()) { auto serializedCont = std::dynamic_pointer_cast< SerializedAsynchronousContinuation>(currContin); @@ -94,13 +94,14 @@ SerializedAsynchronousContinuation * Hence, we have a gridlock. */ - LockerAndInvokerBase* foreignOwnerPtr = firstFailedQutex.getCurrOwner(); + std::shared_ptr foreignOwnerShPtr = + firstFailedQutex.getCurrOwner(); // If no current owner, can't be a gridlock - if (foreignOwnerPtr == nullptr) + if (foreignOwnerShPtr == nullptr) { return false; } // Use reference for the rest of the function for safety. - LockerAndInvokerBase& foreignOwner = *foreignOwnerPtr; + LockerAndInvokerBase &foreignOwner = *foreignOwnerShPtr; /* For each lock in the foreign owner's LockSet, check if we hold it * in any of our previous continuations (excluding our most immediate one) @@ -128,9 +129,9 @@ SerializedAsynchronousContinuation * should eventually be able to acquire that lock. */ for (std::shared_ptr currContin = - this->serializedContinuation.getCallersContinuation(); + this->serializedContinuation.getCallersContinuationShPtr(); currContin != nullptr; - currContin = currContin->getCallersContinuation()) + currContin = currContin->getCallersContinuationShPtr()) { auto serializedCont = std::dynamic_pointer_cast< SerializedAsynchronousContinuation>(currContin);