[debugserver] Remove unnecessary sleep in MachProcess::AttachForDebug

[JDevlieghere]

Remove the unnecessary sleep in MachProcess::AttachForDebug. The preceding comment makes it seem like it's necessary for synchronization, though I don't believe that's the case (see below), and even if it were, sleeping is not a reliable way to achieve that.

The reason I don't believe it's necessary is because after we return, we synchronize with the exception thread on a state change. The latter will call and update the process state, which is exactly what we synchronize on. I was able to verify that this is the first time we change the process state: i.e., GetState doesn't return a different value before and after the sleep.

On top of that, there are 3 more places where we call ptrace attach (PosixSpawnChildForPTraceDebugging, SBLaunchForDebug, and BoardServiceLaunchForDebug) where we don't sleep.

rdar://163952037

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

Remove the unnecessary sleep in MachProcess::AttachForDebug. The preceding comment makes it seem like it's necessary for synchronization, though I don't believe that's the case (see below), and even if it were, sleeping is not a reliable way to achieve that.

The reason I don't believe it's necessary is because after we return, we synchronize with the exception thread on a state change. The latter will call and update the process state, which is exactly what we synchronize on. I was able to verify that this is the first time we change the process state: i.e., GetState doesn't return a different value before and after the sleep.

On top of that, there are 3 more places where we call ptrace attach (PosixSpawnChildForPTraceDebugging, SBLaunchForDebug, and BoardServiceLaunchForDebug) where we don't sleep.

rdar://163952037

---

Full diff: https://github.com/llvm/llvm-project/pull/166674.diff

1 Files Affected:

• (modified) lldb/tools/debugserver/source/MacOSX/MachProcess.mm (-6)

diff --git a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
index 3afaaa2f64c00..8df3f29a7e825 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -2853,12 +2853,6 @@ static uint64_t bits(uint64_t value, uint32_t msbit, uint32_t lsbit) {
 
     if (err.Success()) {
       m_flags |= eMachProcessFlagsAttached;
-      // Sleep a bit to let the exception get received and set our process
-      // status
-      // to stopped.
-      ::usleep(250000);
-      DNBLog("[LaunchAttach] (%d) Done napping after ptrace(PT_ATTACHEXC)'ing",
-             getpid());
       DNBLogThreadedIf(LOG_PROCESS, "successfully attached to pid %d", pid);
       return m_pid;
     } else {

[jasonmolenda]

Yes, agreed. If there ever was a need to sleep to allow something to synchronize, this number was picked fifteen years ago when the whole environment was an order of magnitude slower; it cannot be correct still.