Fix a race between lldb's packet timeout and the profile thread's usl…

…eep.

The debugserver profile thread used to suspend itself between samples with
a usleep.  When you detach or kill, MachProcess::Clear would delay replying
to the incoming packet until pthread_join of the profile thread returned.
If you are unlucky or the suspend delay is long, it could take longer than
the packet timeout for pthread_join to return.  Then you would get an error
about detach not succeeding from lldb - even though in fact the detach was
successful...

I replaced the usleep with PThreadEvents entity.  Then we just call a timed
WaitForEventBits, and when debugserver wants to stop the profile thread, it
can set the event bit, and the sleep will exit immediately.

Differential Revision: https://reviews.llvm.org/D75004

lldb/test/API/macosx/profile_vrs_detach/Makefile

@@ -0,0 +1,4 @@
+C_SOURCES := main.c
+CFLAGS_EXTRAS := -std=c99
+
+include Makefile.rules

lldb/test/API/macosx/profile_vrs_detach/TestDetachVrsProfile.py

@@ -0,0 +1,76 @@
+"""
+debugserver used to block replying to the 'D' packet
+till it had joined the profiling thread.  If the profiling interval
+was too long, that would mean it would take longer than the packet
+timeout to reply, and the detach would time out.  Make sure that doesn't
+happen. 
+"""
+
+
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+import os
+import signal
+
+class TestDetachVrsProfile(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    NO_DEBUG_INFO_TESTCASE = True
+
+    @skipUnlessDarwin
+    @skipIfOutOfTreeDebugserver
+    def test_profile_and_detach(self):
+        """There can be many tests in a test case - describe this test here."""
+        self.build()
+        self.main_source_file = lldb.SBFileSpec("main.c")
+        self.do_profile_and_detach()
+
+    def do_profile_and_detach(self):
+        (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
+                                   "Set a breakpoint here", self.main_source_file)
+
+        interp = self.dbg.GetCommandInterpreter()
+        result = lldb.SBCommandReturnObject()
+
+        # First make sure we are getting async data.  Set a short interval, continue a bit and check:
+        interp.HandleCommand("process plugin packet send 'QSetEnableAsyncProfiling;enable:1;interval_usec:500000;scan_type=0x5;'", result)
+        self.assertTrue(result.Succeeded(), "process packet send failed: %s"%(result.GetError()))
+
+        # Run a bit to give us a change to collect profile data:
+        bkpt.SetIgnoreCount(1)
+        threads = lldbutil.continue_to_breakpoint(process, bkpt)
+        self.assertEqual(len(threads), 1, "Hit our breakpoint again.")
+        str = process.GetAsyncProfileData(1000)
+        self.assertTrue(len(str) > 0, "Got some profile data")
+
+        # Now make the profiling interval very long and try to detach.
+        interp.HandleCommand("process plugin packet send 'QSetEnableAsyncProfiling;enable:1;interval_usec:10000000;scan_type=0x5;'", result)
+        self.assertTrue(result.Succeeded(), "process packet send failed: %s"%(result.GetError()))
+        self.dbg.SetAsync(True)
+        listener = self.dbg.GetListener()
+
+        # We don't want to hit our breakpoint anymore.
+        bkpt.SetEnabled(False)
+
+        # Record our process pid so we can kill it since we are going to detach...
+        self.pid = process.GetProcessID()
+        def cleanup():
+            self.dbg.SetAsync(False)
+            os.kill(self.pid, signal.SIGKILL)
+        self.addTearDownHook(cleanup)
+
+        process.Continue()
+
+        event = lldb.SBEvent()
+        success = listener.WaitForEventForBroadcaster(0, process.GetBroadcaster(), event)
+        self.assertTrue(success, "Got an event which should be running.")
+        event_state = process.GetStateFromEvent(event)
+        self.assertEqual(event_state, lldb.eStateRunning, "Got the running event")
+
+        # Now detach:
+        error = process.Detach()
+        self.assertTrue(error.Success(), "Detached successfully")

lldb/test/API/macosx/profile_vrs_detach/main.c

@@ -0,0 +1,11 @@
+#include <stdio.h>
+
+int
+main()
+{
+  while (1) {
+    sleep(1); // Set a breakpoint here 
+    printf("I slept\n");
+  }
+  return 0;
+}

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

@@ -338,9 +338,16 @@ class MachProcess {
     eMachProcessFlagsUsingFBS = (1 << 3), // only read via ProcessUsingFrontBoard()
     eMachProcessFlagsBoardCalculated = (1 << 4)
   };
+
+  enum {
+    eMachProcessProfileNone = 0,
+    eMachProcessProfileCancel = (1 << 0)
+  };
+
   void Clear(bool detaching = false);
   void ReplyToAllExceptions();
   void PrivateResume();
+  void StopProfileThread();
 
   uint32_t Flags() const { return m_flags; }
   nub_state_t DoSIGSTOP(bool clear_bps_and_wps, bool allow_running,
@@ -375,7 +382,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  
   DNBThreadResumeActions m_thread_actions; // The thread actions for the current
                                            // MachProcess::Resume() call
   MachException::Message::collection m_exception_messages; // A collection of

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

@@ -25,11 +25,13 @@
 #include <sys/ptrace.h>
 #include <sys/stat.h>
 #include <sys/sysctl.h>
+#include <sys/time.h>
 #include <sys/types.h>
 #include <unistd.h>
 #include <uuid/uuid.h>
 
 #include <algorithm>
+#include <chrono>
 #include <map>
 
 #import <Foundation/Foundation.h>
@@ -485,6 +487,7 @@ static bool FBSAddEventDataToOptions(NSMutableDictionary *options,
       m_stdio_mutex(PTHREAD_MUTEX_RECURSIVE), m_stdout_data(),
       m_profile_enabled(false), m_profile_interval_usec(0), m_profile_thread(0),
       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_activities(), m_state(eStateUnloaded),
@@ -1294,10 +1297,7 @@ static bool FBSAddEventDataToOptions(NSMutableDictionary *options,
     m_exception_messages.clear();
   }
   m_activities.Clear();
-  if (m_profile_thread) {
-    pthread_join(m_profile_thread, NULL);
-    m_profile_thread = NULL;
-  }
+  StopProfileThread();
 }
 
 bool MachProcess::StartSTDIOThread() {
@@ -1316,11 +1316,19 @@ static bool FBSAddEventDataToOptions(NSMutableDictionary *options,
   if (m_profile_enabled && (m_profile_thread == NULL)) {
     StartProfileThread();
   } else if (!m_profile_enabled && m_profile_thread) {
-    pthread_join(m_profile_thread, NULL);
-    m_profile_thread = NULL;
+    StopProfileThread();
   }
 }
 
+void MachProcess::StopProfileThread() {
+  if (m_profile_thread == NULL)
+    return;
+  m_profile_events.SetEvents(eMachProcessProfileCancel);
+  pthread_join(m_profile_thread, NULL);
+  m_profile_thread = NULL;
+  m_profile_events.ResetEvents(eMachProcessProfileCancel);
+}
+
 bool MachProcess::StartProfileThread() {
   DNBLogThreadedIf(LOG_PROCESS, "MachProcess::%s ( )", __FUNCTION__);
   // Create the thread that profiles the inferior and reports back if enabled
@@ -2513,10 +2521,20 @@ static bool FBSAddEventDataToOptions(NSMutableDictionary *options,
       // Done. Get out of this thread.
       break;
     }
-
-    // A simple way to set up the profile interval. We can also use select() or
-    // dispatch timer source if necessary.
-    usleep(proc->ProfileInterval());
+    timespec ts;
+    {
+      using namespace std::chrono;
+      std::chrono::microseconds dur(proc->ProfileInterval());
+      const auto dur_secs = duration_cast<seconds>(dur);
+      const auto dur_usecs = dur % std::chrono::seconds(1);
+      DNBTimer::OffsetTimeOfDay(&ts, dur_secs.count(), 
+                                dur_usecs.count());
+    }
+    uint32_t bits_set = 
+        proc->m_profile_events.WaitForSetEvents(eMachProcessProfileCancel, &ts);
+    // If we got bits back, we were told to exit.  Do so.
+    if (bits_set & eMachProcessProfileCancel)
+      break;
   }
   return NULL;
 }