Skip to content

[debugserver] Migrate MachProcess away from PThreadMutex (NFC) #137553

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.

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
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 6 additions & 6 deletions lldb/tools/debugserver/source/MacOSX/MachProcess.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
#include "MachVMMemory.h"
#include "PThreadCondition.h"
#include "PThreadEvent.h"
#include "PThreadMutex.h"
#include "RNBContext.h"
#include "ThreadInfo.h"

Expand Down Expand Up @@ -413,7 +412,7 @@ class MachProcess {
uint32_t m_stop_count; // A count of many times have we stopped
pthread_t m_stdio_thread; // Thread ID for the thread that watches for child
// process stdio
PThreadMutex m_stdio_mutex; // Multithreaded protection for stdio
std::recursive_mutex m_stdio_mutex; // Multithreaded protection for stdio
std::string m_stdout_data;

bool m_profile_enabled; // A flag to indicate if profiling is enabled
Expand All @@ -423,7 +422,7 @@ class MachProcess {
m_profile_scan_type; // Indicates what needs to be profiled
pthread_t
m_profile_thread; // Thread ID for the thread that profiles the inferior
PThreadMutex
std::recursive_mutex
m_profile_data_mutex; // Multithreaded protection for profile info data
std::vector<std::string>
m_profile_data; // Profile data, must be protected by m_profile_data_mutex
Expand All @@ -435,15 +434,16 @@ class MachProcess {
// caught when
// listening to the
// exception port
PThreadMutex m_exception_and_signal_mutex; // Multithreaded protection for
// exceptions and signals.
std::recursive_mutex
m_exception_and_signal_mutex; // Multithreaded protection for
// exceptions and signals.

MachThreadList m_thread_list; // A list of threads that is maintained/updated
// after each stop
Genealogy m_activities; // A list of activities that is updated after every
// stop lazily
nub_state_t m_state; // The state of our process
PThreadMutex m_state_mutex; // Multithreaded protection for m_state
std::recursive_mutex m_state_mutex; // Multithreaded protection for m_state
PThreadEvent m_events; // Process related events in the child processes
// lifetime can be waited upon
PThreadEvent m_private_events; // Used to coordinate running and stopping the
Expand Down
48 changes: 23 additions & 25 deletions lldb/tools/debugserver/source/MacOSX/MachProcess.mm
Original file line number Diff line number Diff line change
Expand Up @@ -522,19 +522,17 @@ static bool FBSAddEventDataToOptions(NSMutableDictionary *options,
MachProcess::MachProcess()
: m_pid(0), m_cpu_type(0), m_child_stdin(-1), m_child_stdout(-1),
m_child_stderr(-1), m_path(), m_args(), m_task(this),
m_flags(eMachProcessFlagsNone), m_stdio_thread(0),
m_stdio_mutex(PTHREAD_MUTEX_RECURSIVE), m_stdout_data(),
m_profile_enabled(false), m_profile_interval_usec(0), m_profile_thread(0),
m_profile_data_mutex(PTHREAD_MUTEX_RECURSIVE), m_profile_data(),
m_flags(eMachProcessFlagsNone), m_stdio_thread(0), m_stdio_mutex(),
m_stdout_data(), m_profile_enabled(false), m_profile_interval_usec(0),
m_profile_thread(0), m_profile_data_mutex(), m_profile_data(),
m_profile_events(0, eMachProcessProfileCancel), m_thread_actions(),
m_exception_messages(),
m_exception_and_signal_mutex(PTHREAD_MUTEX_RECURSIVE), m_thread_list(),
m_activities(), m_state(eStateUnloaded),
m_state_mutex(PTHREAD_MUTEX_RECURSIVE), m_events(0, kAllEventsMask),
m_private_events(0, kAllEventsMask), m_breakpoints(), m_watchpoints(),
m_name_to_addr_callback(NULL), m_name_to_addr_baton(NULL),
m_image_infos_callback(NULL), m_image_infos_baton(NULL),
m_sent_interrupt_signo(0), m_auto_resume_signo(0), m_did_exec(false),
m_exception_messages(), m_exception_and_signal_mutex(), m_thread_list(),
m_activities(), m_state(eStateUnloaded), m_state_mutex(),
m_events(0, kAllEventsMask), m_private_events(0, kAllEventsMask),
m_breakpoints(), m_watchpoints(), m_name_to_addr_callback(NULL),
m_name_to_addr_baton(NULL), m_image_infos_callback(NULL),
m_image_infos_baton(NULL), m_sent_interrupt_signo(0),
m_auto_resume_signo(0), m_did_exec(false),
m_dyld_process_info_create(nullptr),
m_dyld_process_info_for_each_image(nullptr),
m_dyld_process_info_release(nullptr),
Expand Down Expand Up @@ -577,7 +575,7 @@ static bool FBSAddEventDataToOptions(NSMutableDictionary *options,

nub_state_t MachProcess::GetState() {
// If any other threads access this we will need a mutex for it
PTHREAD_MUTEX_LOCKER(locker, m_state_mutex);
std::lock_guard<std::recursive_mutex> guard(m_state_mutex);
return m_state;
}

Expand Down Expand Up @@ -1279,7 +1277,7 @@ static bool mach_header_validity_test(uint32_t magic, uint32_t cputype) {

// Scope for mutex locker
{
PTHREAD_MUTEX_LOCKER(locker, m_state_mutex);
std::lock_guard<std::recursive_mutex> guard(m_state_mutex);
const nub_state_t old_state = m_state;

if (old_state == eStateExited) {
Expand Down Expand Up @@ -1338,7 +1336,7 @@ static bool mach_header_validity_test(uint32_t magic, uint32_t cputype) {
m_stop_count = 0;
m_thread_list.Clear();
{
PTHREAD_MUTEX_LOCKER(locker, m_exception_and_signal_mutex);
std::lock_guard<std::recursive_mutex> guard(m_exception_and_signal_mutex);
m_exception_messages.clear();
m_sent_interrupt_signo = 0;
m_auto_resume_signo = 0;
Expand Down Expand Up @@ -1578,7 +1576,7 @@ static uint64_t bits(uint64_t value, uint32_t msbit, uint32_t lsbit) {
bool MachProcess::Interrupt() {
nub_state_t state = GetState();
if (IsRunning(state)) {
PTHREAD_MUTEX_LOCKER(locker, m_exception_and_signal_mutex);
std::lock_guard<std::recursive_mutex> guard(m_exception_and_signal_mutex);
if (m_sent_interrupt_signo == 0) {
m_sent_interrupt_signo = SIGSTOP;
if (Signal(m_sent_interrupt_signo)) {
Expand Down Expand Up @@ -1736,7 +1734,7 @@ static uint64_t bits(uint64_t value, uint32_t msbit, uint32_t lsbit) {
m_thread_actions.Append(thread_action);
m_thread_actions.SetDefaultThreadActionIfNeeded(eStateRunning, 0);

PTHREAD_MUTEX_LOCKER(locker, m_exception_and_signal_mutex);
std::lock_guard<std::recursive_mutex> guard(m_exception_and_signal_mutex);

ReplyToAllExceptions();
}
Expand Down Expand Up @@ -1862,7 +1860,7 @@ static uint64_t bits(uint64_t value, uint32_t msbit, uint32_t lsbit) {
}

void MachProcess::ReplyToAllExceptions() {
PTHREAD_MUTEX_LOCKER(locker, m_exception_and_signal_mutex);
std::lock_guard<std::recursive_mutex> guard(m_exception_and_signal_mutex);
if (!m_exception_messages.empty()) {
MachException::Message::iterator pos;
MachException::Message::iterator begin = m_exception_messages.begin();
Expand Down Expand Up @@ -1896,7 +1894,7 @@ static uint64_t bits(uint64_t value, uint32_t msbit, uint32_t lsbit) {
}
}
void MachProcess::PrivateResume() {
PTHREAD_MUTEX_LOCKER(locker, m_exception_and_signal_mutex);
std::lock_guard<std::recursive_mutex> guard(m_exception_and_signal_mutex);

m_auto_resume_signo = m_sent_interrupt_signo;
if (m_auto_resume_signo)
Expand Down Expand Up @@ -2298,7 +2296,7 @@ static uint64_t bits(uint64_t value, uint32_t msbit, uint32_t lsbit) {
// data has already been copied.
void MachProcess::ExceptionMessageReceived(
const MachException::Message &exceptionMessage) {
PTHREAD_MUTEX_LOCKER(locker, m_exception_and_signal_mutex);
std::lock_guard<std::recursive_mutex> guard(m_exception_and_signal_mutex);

if (m_exception_messages.empty())
m_task.Suspend();
Expand All @@ -2312,7 +2310,7 @@ static uint64_t bits(uint64_t value, uint32_t msbit, uint32_t lsbit) {

task_t MachProcess::ExceptionMessageBundleComplete() {
// We have a complete bundle of exceptions for our child process.
PTHREAD_MUTEX_LOCKER(locker, m_exception_and_signal_mutex);
std::lock_guard<std::recursive_mutex> guard(m_exception_and_signal_mutex);
DNBLogThreadedIf(LOG_EXCEPTIONS, "%s: %llu exception messages.",
__PRETTY_FUNCTION__, (uint64_t)m_exception_messages.size());
bool auto_resume = false;
Expand Down Expand Up @@ -2495,7 +2493,7 @@ static uint64_t bits(uint64_t value, uint32_t msbit, uint32_t lsbit) {
void MachProcess::AppendSTDOUT(char *s, size_t len) {
DNBLogThreadedIf(LOG_PROCESS, "MachProcess::%s (<%llu> %s) ...", __FUNCTION__,
(uint64_t)len, s);
PTHREAD_MUTEX_LOCKER(locker, m_stdio_mutex);
std::lock_guard<std::recursive_mutex> guard(m_stdio_mutex);
m_stdout_data.append(s, len);
m_events.SetEvents(eEventStdioAvailable);

Expand All @@ -2506,7 +2504,7 @@ static uint64_t bits(uint64_t value, uint32_t msbit, uint32_t lsbit) {
size_t MachProcess::GetAvailableSTDOUT(char *buf, size_t buf_size) {
DNBLogThreadedIf(LOG_PROCESS, "MachProcess::%s (&%p[%llu]) ...", __FUNCTION__,
static_cast<void *>(buf), (uint64_t)buf_size);
PTHREAD_MUTEX_LOCKER(locker, m_stdio_mutex);
std::lock_guard<std::recursive_mutex> guard(m_stdio_mutex);
size_t bytes_available = m_stdout_data.size();
if (bytes_available > 0) {
if (bytes_available > buf_size) {
Expand Down Expand Up @@ -2733,7 +2731,7 @@ static uint64_t bits(uint64_t value, uint32_t msbit, uint32_t lsbit) {

void MachProcess::SignalAsyncProfileData(const char *info) {
DNBLogThreadedIf(LOG_PROCESS, "MachProcess::%s (%s) ...", __FUNCTION__, info);
PTHREAD_MUTEX_LOCKER(locker, m_profile_data_mutex);
std::lock_guard<std::recursive_mutex> guard(m_profile_data_mutex);
m_profile_data.push_back(info);
m_events.SetEvents(eEventProfileDataAvailable);

Expand All @@ -2744,7 +2742,7 @@ static uint64_t bits(uint64_t value, uint32_t msbit, uint32_t lsbit) {
size_t MachProcess::GetAsyncProfileData(char *buf, size_t buf_size) {
DNBLogThreadedIf(LOG_PROCESS, "MachProcess::%s (&%p[%llu]) ...", __FUNCTION__,
static_cast<void *>(buf), (uint64_t)buf_size);
PTHREAD_MUTEX_LOCKER(locker, m_profile_data_mutex);
std::lock_guard<std::recursive_mutex> guard(m_profile_data_mutex);
if (m_profile_data.empty())
return 0;

Expand Down
Loading