[Lldb-commits] [lldb] e823449 - [lldb][debugserver] Synchronize interrupt and resume signals (#131073)
via lldb-commits
lldb-commits at lists.llvm.org
Thu Mar 13 13:54:17 PDT 2025
Author: Jonas Devlieghere
Date: 2025-03-13T13:54:13-07:00
New Revision: e823449f66acb39ce5a11dde6283ffd11731fe6a
URL: https://github.com/llvm/llvm-project/commit/e823449f66acb39ce5a11dde6283ffd11731fe6a
DIFF: https://github.com/llvm/llvm-project/commit/e823449f66acb39ce5a11dde6283ffd11731fe6a.diff
LOG: [lldb][debugserver] Synchronize interrupt and resume signals (#131073)
This PR fixes a race condition in debugserver where the main thread
calls MachProcess::Interrupt, setting `m_sent_interrupt_signo` while the
exception monitoring thread is checking the value of the variable.
I was on the fence between introducing a new mutex and reusing the
existing exception mutex. With the notable exception of
MachProcess::Interrupt, all the other places where we were already
locking this mutex before accessing the variable. I renamed the mutex to
make it clear that it's now protecting more than the exception messages.
Jason, while investigating a real issue, had a suspicion there was race
condition related to interrupts and I was able to narrow it down by
building debugserver with TSan.
Added:
Modified:
lldb/tools/debugserver/source/MacOSX/MachProcess.h
lldb/tools/debugserver/source/MacOSX/MachProcess.mm
Removed:
################################################################################
diff --git a/lldb/tools/debugserver/source/MacOSX/MachProcess.h b/lldb/tools/debugserver/source/MacOSX/MachProcess.h
index db673693a1b21..ec0a13b482267 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachProcess.h
+++ b/lldb/tools/debugserver/source/MacOSX/MachProcess.h
@@ -427,7 +427,7 @@ class MachProcess {
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
- PThreadEvent m_profile_events; // Used for the profile thread cancellable wait
+ PThreadEvent m_profile_events; // Used for the profile thread cancellable wait
DNBThreadResumeActions m_thread_actions; // The thread actions for the current
// MachProcess::Resume() call
MachException::Message::collection m_exception_messages; // A collection of
@@ -435,8 +435,8 @@ class MachProcess {
// caught when
// listening to the
// exception port
- PThreadMutex m_exception_messages_mutex; // Multithreaded protection for
- // m_exception_messages
+ PThreadMutex 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
diff --git a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
index a2179bf2f91e5..4742beeb6a4a3 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -528,7 +528,7 @@ static bool FBSAddEventDataToOptions(NSMutableDictionary *options,
m_profile_data_mutex(PTHREAD_MUTEX_RECURSIVE), m_profile_data(),
m_profile_events(0, eMachProcessProfileCancel), m_thread_actions(),
m_exception_messages(),
- m_exception_messages_mutex(PTHREAD_MUTEX_RECURSIVE), m_thread_list(),
+ 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(),
@@ -1338,8 +1338,11 @@ 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_messages_mutex);
+ PTHREAD_MUTEX_LOCKER(locker, m_exception_and_signal_mutex);
m_exception_messages.clear();
+ m_sent_interrupt_signo = 0;
+ m_auto_resume_signo = 0;
+
}
m_activities.Clear();
StopProfileThread();
@@ -1575,6 +1578,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);
if (m_sent_interrupt_signo == 0) {
m_sent_interrupt_signo = SIGSTOP;
if (Signal(m_sent_interrupt_signo)) {
@@ -1728,7 +1732,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_messages_mutex);
+ PTHREAD_MUTEX_LOCKER(locker, m_exception_and_signal_mutex);
ReplyToAllExceptions();
}
@@ -1854,7 +1858,7 @@ static uint64_t bits(uint64_t value, uint32_t msbit, uint32_t lsbit) {
}
void MachProcess::ReplyToAllExceptions() {
- PTHREAD_MUTEX_LOCKER(locker, m_exception_messages_mutex);
+ PTHREAD_MUTEX_LOCKER(locker, m_exception_and_signal_mutex);
if (!m_exception_messages.empty()) {
MachException::Message::iterator pos;
MachException::Message::iterator begin = m_exception_messages.begin();
@@ -1888,7 +1892,7 @@ static uint64_t bits(uint64_t value, uint32_t msbit, uint32_t lsbit) {
}
}
void MachProcess::PrivateResume() {
- PTHREAD_MUTEX_LOCKER(locker, m_exception_messages_mutex);
+ PTHREAD_MUTEX_LOCKER(locker, m_exception_and_signal_mutex);
m_auto_resume_signo = m_sent_interrupt_signo;
if (m_auto_resume_signo)
@@ -2290,7 +2294,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_messages_mutex);
+ PTHREAD_MUTEX_LOCKER(locker, m_exception_and_signal_mutex);
if (m_exception_messages.empty())
m_task.Suspend();
@@ -2304,7 +2308,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_messages_mutex);
+ PTHREAD_MUTEX_LOCKER(locker, m_exception_and_signal_mutex);
DNBLogThreadedIf(LOG_EXCEPTIONS, "%s: %llu exception messages.",
__PRETTY_FUNCTION__, (uint64_t)m_exception_messages.size());
bool auto_resume = false;
More information about the lldb-commits
mailing list