Skip to content

[debugserver] Migrate PThreadEvent away from PThreadMutex (NFC) #137554

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 28, 2025
Merged

[debugserver] Migrate PThreadEvent away from PThreadMutex (NFC) #137554

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 36 additions & 40 deletions lldb/tools/debugserver/source/PThreadEvent.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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;
Expand All @@ -39,15 +39,15 @@ 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;
}
}

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;
}
Expand All @@ -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
Expand All @@ -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();
}
}

Expand 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();
}
}

Expand 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
Expand All @@ -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
Expand Down
11 changes: 4 additions & 7 deletions lldb/tools/debugserver/source/PThreadEvent.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@

#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>
#include <mutex>

class PThreadEvent {
public:
Expand Down Expand Up @@ -45,11 +45,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;
Expand Down
Loading