https://github.com/JDevlieghere created https://github.com/llvm/llvm-project/pull/137554
The debugserver code predates modern C++, but with C++11 and later there's no need to have something like PThreadMutex. This migrates PThreadEvent away from PThreadMutex in preparation for removing it. >From 8424780dc5739458ba512a9778d6cd32986e144e Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <jo...@devlieghere.com> Date: Sun, 27 Apr 2025 14:51:22 -0700 Subject: [PATCH] [debugserver] Migrate PThreadEvent away from PThreadMutex (NFC) The debugserver code predates modern C++, but with C++11 and later there's no need to have something like PThreadMutex. This migrates PThreadEvent away from PThreadMutex in preparation for removing it. --- .../tools/debugserver/source/PThreadEvent.cpp | 76 +++++++++---------- lldb/tools/debugserver/source/PThreadEvent.h | 8 +- 2 files changed, 38 insertions(+), 46 deletions(-) diff --git a/lldb/tools/debugserver/source/PThreadEvent.cpp b/lldb/tools/debugserver/source/PThreadEvent.cpp index f9166a1b63d06..e12070c05eccd 100644 --- a/lldb/tools/debugserver/source/PThreadEvent.cpp +++ b/lldb/tools/debugserver/source/PThreadEvent.cpp @@ -15,8 +15,8 @@ #include <cerrno> PThreadEvent::PThreadEvent(uint32_t bits, uint32_t validBits) - : m_mutex(), m_set_condition(), m_reset_condition(), m_bits(bits), - m_validBits(validBits), m_reset_ack_mask(0) { + : m_mutex(), m_set_condition(), m_bits(bits), m_validBits(validBits), + m_reset_ack_mask(0) { // DNBLogThreadedIf(LOG_EVENTS, "%p PThreadEvent::%s (0x%8.8x, 0x%8.8x)", // this, __FUNCTION__, bits, validBits); } @@ -27,7 +27,7 @@ PThreadEvent::~PThreadEvent() { uint32_t PThreadEvent::NewEventBit() { // DNBLogThreadedIf(LOG_EVENTS, "%p %s", this, LLVM_PRETTY_FUNCTION); - PTHREAD_MUTEX_LOCKER(locker, m_mutex); + std::lock_guard<std::mutex> guard(m_mutex); uint32_t mask = 1; while (mask & m_validBits) mask <<= 1; @@ -39,7 +39,7 @@ void PThreadEvent::FreeEventBits(const uint32_t mask) { // DNBLogThreadedIf(LOG_EVENTS, "%p PThreadEvent::%s (0x%8.8x)", this, // __FUNCTION__, mask); if (mask) { - PTHREAD_MUTEX_LOCKER(locker, m_mutex); + std::lock_guard<std::mutex> guard(m_mutex); m_bits &= ~mask; m_validBits &= ~mask; } @@ -47,7 +47,7 @@ void PThreadEvent::FreeEventBits(const uint32_t mask) { uint32_t PThreadEvent::GetEventBits() const { // DNBLogThreadedIf(LOG_EVENTS, "%p %s", this, LLVM_PRETTY_FUNCTION); - PTHREAD_MUTEX_LOCKER(locker, m_mutex); + std::lock_guard<std::mutex> guard(m_mutex); uint32_t bits = m_bits; return bits; } @@ -56,7 +56,7 @@ uint32_t PThreadEvent::GetEventBits() const { void PThreadEvent::ReplaceEventBits(const uint32_t bits) { // DNBLogThreadedIf(LOG_EVENTS, "%p PThreadEvent::%s (0x%8.8x)", this, // __FUNCTION__, bits); - PTHREAD_MUTEX_LOCKER(locker, m_mutex); + std::lock_guard<std::mutex> guard(m_mutex); // Make sure we have some bits and that they aren't already set... if (m_bits != bits) { // Figure out which bits are changing @@ -65,7 +65,7 @@ void PThreadEvent::ReplaceEventBits(const uint32_t bits) { m_bits = bits; // If any new bits are set, then broadcast if (changed_bits & m_bits) - m_set_condition.Broadcast(); + m_set_condition.notify_all(); } } @@ -77,14 +77,14 @@ void PThreadEvent::SetEvents(const uint32_t mask) { // __FUNCTION__, mask); // Make sure we have some bits to set if (mask) { - PTHREAD_MUTEX_LOCKER(locker, m_mutex); + std::lock_guard<std::mutex> guard(m_mutex); // Save the old event bit state so we can tell if things change uint32_t old = m_bits; // Set the all event bits that are set in 'mask' m_bits |= mask; // Broadcast only if any extra bits got set. if (old != m_bits) - m_set_condition.Broadcast(); + m_set_condition.notify_all(); } } @@ -93,18 +93,27 @@ void PThreadEvent::ResetEvents(const uint32_t mask) { // DNBLogThreadedIf(LOG_EVENTS, "%p PThreadEvent::%s (0x%8.8x)", this, // __FUNCTION__, mask); if (mask) { - PTHREAD_MUTEX_LOCKER(locker, m_mutex); - - // Save the old event bit state so we can tell if things change - uint32_t old = m_bits; + std::lock_guard<std::mutex> guard(m_mutex); // Clear the all event bits that are set in 'mask' m_bits &= ~mask; - // Broadcast only if any extra bits got reset. - if (old != m_bits) - m_reset_condition.Broadcast(); } } +static std::chrono::nanoseconds ToDuration(timespec ts) { + auto duration = + std::chrono::seconds{ts.tv_sec} + std::chrono::nanoseconds{ts.tv_nsec}; + return std::chrono::duration_cast<std::chrono::nanoseconds>(duration); +} + +static std::chrono::time_point<std::chrono::system_clock, + std::chrono::nanoseconds> +ToTimePoint(timespec ts) { + return std::chrono::time_point<std::chrono::system_clock, + std::chrono::nanoseconds>{ + std::chrono::duration_cast<std::chrono::system_clock::duration>( + ToDuration(ts))}; +} + // Wait until 'timeout_abstime' for any events that are set in // 'mask'. If 'timeout_abstime' is NULL, then wait forever. uint32_t @@ -113,30 +122,17 @@ PThreadEvent::WaitForEventsImpl(const uint32_t mask, std::function<bool()> predicate) const { // DNBLogThreadedIf(LOG_EVENTS, "%p PThreadEvent::%s (0x%8.8x, %p)", this, // __FUNCTION__, mask, timeout_abstime); - - int err = 0; - - // pthread_cond_timedwait() or pthread_cond_wait() will atomically - // unlock the mutex and wait for the condition to be set. When either - // function returns, they will re-lock the mutex. We use an auto lock/unlock - // class (PThreadMutex::Locker) to allow us to return at any point in this - // function and not have to worry about unlocking the mutex. - PTHREAD_MUTEX_LOCKER(locker, m_mutex); - - // Check the predicate and the error code. The functions below do not return - // EINTR so that's not something we need to handle. - while (!predicate() && err == 0) { - if (timeout_abstime) { - // Wait for condition to get broadcast, or for a timeout. If we get - // a timeout we will drop out of the loop on the next iteration and we - // will recompute the mask in case of a race between the condition and the - // timeout. - err = ::pthread_cond_timedwait(m_set_condition.Condition(), - m_mutex.Mutex(), timeout_abstime); - } else { - // Wait for condition to get broadcast. - err = ::pthread_cond_wait(m_set_condition.Condition(), m_mutex.Mutex()); - } + std::unique_lock<std::mutex> lock(m_mutex); + + if (timeout_abstime) { + // Wait for condition to get broadcast, or for a timeout. If we get + // a timeout we will drop out of the loop on the next iteration and we + // will recompute the mask in case of a race between the condition and the + // timeout. + m_set_condition.wait_until(lock, ToTimePoint(*timeout_abstime), predicate); + } else { + // Wait for condition to get broadcast. + m_set_condition.wait(lock, predicate); } // Either the predicate passed, we hit the specified timeout (ETIMEDOUT) or we diff --git a/lldb/tools/debugserver/source/PThreadEvent.h b/lldb/tools/debugserver/source/PThreadEvent.h index 5a8a7dd207493..f11f311164561 100644 --- a/lldb/tools/debugserver/source/PThreadEvent.h +++ b/lldb/tools/debugserver/source/PThreadEvent.h @@ -13,7 +13,6 @@ #ifndef LLDB_TOOLS_DEBUGSERVER_SOURCE_PTHREADEVENT_H #define LLDB_TOOLS_DEBUGSERVER_SOURCE_PTHREADEVENT_H #include "PThreadCondition.h" -#include "PThreadMutex.h" #include <cstdint> #include <ctime> #include <functional> @@ -45,11 +44,8 @@ class PThreadEvent { const struct timespec *timeout_abstime = NULL) const; protected: - // pthread condition and mutex variable to control access and allow - // blocking between the main thread and the spotlight index thread. - mutable PThreadMutex m_mutex; - mutable PThreadCondition m_set_condition; - mutable PThreadCondition m_reset_condition; + mutable std::mutex m_mutex; + mutable std::condition_variable m_set_condition; uint32_t m_bits; uint32_t m_validBits; uint32_t m_reset_ack_mask; _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits