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; _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
