Skip to content

Commit 420d56a

Browse files
authored
Clean up MachTask.mm's handling of m_exception_thread. (#167994)
This was getting joined in ShutDownExcecptionThread (sic) but not cleared. So this function was not safe to call twice, since you aren't supposed to join a thread twice. Sadly, this was called in MachTask::Clear and MachProcess::Destroy, which are both called when you tell debugserver to detach. This didn't seem to cause problems IRL, but the most recent ASAN detects this as an error and calls ASAN::Die, which was causing all the tests that ran detach to fail. I fixed that by moving the clear & test for m_exception_thread to ShutDownExceptionThread. I also fixed the spelling of that routine. And that routine was claiming to return a kern_return_t which no one was checking. It actually returns a kern_return_t if there was a Mach failure and a Posix error if there was a join failure. Since there's really nothing you can do but exit if this fails, which is always what you are in the process of doing when you call this, and since we have already done all the useful logging in ShutDownExceptionThread, I just removed the return value.
1 parent 306f49a commit 420d56a

File tree

3 files changed

+10
-7
lines changed

3 files changed

+10
-7
lines changed

lldb/tools/debugserver/source/MacOSX/MachProcess.mm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1739,7 +1739,7 @@ static uint64_t bits(uint64_t value, uint32_t msbit, uint32_t lsbit) {
17391739
ReplyToAllExceptions();
17401740
}
17411741

1742-
m_task.ShutDownExcecptionThread();
1742+
m_task.ShutDownExceptionThread();
17431743

17441744
// Detach from our process
17451745
errno = 0;

lldb/tools/debugserver/source/MacOSX/MachTask.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ class MachTask {
6868
bool ExceptionPortIsValid() const;
6969
kern_return_t SaveExceptionPortInfo();
7070
kern_return_t RestoreExceptionPortInfo();
71-
kern_return_t ShutDownExcecptionThread();
71+
void ShutDownExceptionThread();
7272

7373
bool StartExceptionThread(
7474
const RNBContext::IgnoredExceptions &ignored_exceptions, DNBError &err);

lldb/tools/debugserver/source/MacOSX/MachTask.mm

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -145,10 +145,8 @@
145145
//----------------------------------------------------------------------
146146
void MachTask::Clear() {
147147
// Do any cleanup needed for this task
148-
if (m_exception_thread)
149-
ShutDownExcecptionThread();
148+
ShutDownExceptionThread();
150149
m_task = TASK_NULL;
151-
m_exception_thread = 0;
152150
m_exception_port = MACH_PORT_NULL;
153151
m_exec_will_be_suspended = false;
154152
m_do_double_resume = false;
@@ -685,8 +683,11 @@ static void get_threads_profile_data(DNBProfileDataScanType scanType,
685683
return false;
686684
}
687685

688-
kern_return_t MachTask::ShutDownExcecptionThread() {
686+
void MachTask::ShutDownExceptionThread() {
689687
DNBError err;
688+
689+
if (!m_exception_thread)
690+
return;
690691

691692
err = RestoreExceptionPortInfo();
692693

@@ -702,6 +703,8 @@ static void get_threads_profile_data(DNBProfileDataScanType scanType,
702703
if (DNBLogCheckLogBit(LOG_TASK) || err.Fail())
703704
err.LogThreaded("::pthread_join ( thread = %p, value_ptr = NULL)",
704705
m_exception_thread);
706+
707+
m_exception_thread = nullptr;
705708

706709
// Deallocate our exception port that we used to track our child process
707710
mach_port_t task_self = mach_task_self();
@@ -713,7 +716,7 @@ static void get_threads_profile_data(DNBProfileDataScanType scanType,
713716
m_exec_will_be_suspended = false;
714717
m_do_double_resume = false;
715718

716-
return err.Status();
719+
return;
717720
}
718721

719722
void *MachTask::ExceptionThread(void *arg) {

0 commit comments

Comments
 (0)