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.
This commit is contained in:
jimingham
2025-11-14 10:14:28 -08:00
committed by GitHub
parent 306f49a254
commit 420d56a394
3 changed files with 10 additions and 7 deletions

View File

@@ -1739,7 +1739,7 @@ bool MachProcess::Detach() {
ReplyToAllExceptions();
}
m_task.ShutDownExcecptionThread();
m_task.ShutDownExceptionThread();
// Detach from our process
errno = 0;

View File

@@ -68,7 +68,7 @@ public:
bool ExceptionPortIsValid() const;
kern_return_t SaveExceptionPortInfo();
kern_return_t RestoreExceptionPortInfo();
kern_return_t ShutDownExcecptionThread();
void ShutDownExceptionThread();
bool StartExceptionThread(
const RNBContext::IgnoredExceptions &ignored_exceptions, DNBError &err);

View File

@@ -145,10 +145,8 @@ bool MachTask::ExceptionPortIsValid() const {
//----------------------------------------------------------------------
void MachTask::Clear() {
// Do any cleanup needed for this task
if (m_exception_thread)
ShutDownExcecptionThread();
ShutDownExceptionThread();
m_task = TASK_NULL;
m_exception_thread = 0;
m_exception_port = MACH_PORT_NULL;
m_exec_will_be_suspended = false;
m_do_double_resume = false;
@@ -685,8 +683,11 @@ bool MachTask::StartExceptionThread(
return false;
}
kern_return_t MachTask::ShutDownExcecptionThread() {
void MachTask::ShutDownExceptionThread() {
DNBError err;
if (!m_exception_thread)
return;
err = RestoreExceptionPortInfo();
@@ -702,6 +703,8 @@ kern_return_t MachTask::ShutDownExcecptionThread() {
if (DNBLogCheckLogBit(LOG_TASK) || err.Fail())
err.LogThreaded("::pthread_join ( thread = %p, value_ptr = NULL)",
m_exception_thread);
m_exception_thread = nullptr;
// Deallocate our exception port that we used to track our child process
mach_port_t task_self = mach_task_self();
@@ -713,7 +716,7 @@ kern_return_t MachTask::ShutDownExcecptionThread() {
m_exec_will_be_suspended = false;
m_do_double_resume = false;
return err.Status();
return;
}
void *MachTask::ExceptionThread(void *arg) {