jingham created this revision.
jingham added a reviewer: clayborg.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

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.

Note, you have to get fairly unlucky because when lldb times out a packet it
then sends a qEcho, and tries to get back in sync.  That adds some extra delay
which might give the detach a chance to succeed.  I've had occasional 
mysterious 
reports of Detach failing like this - only under Xcode which is currently the 
only client
I know of of the profiling info for years, but didn't get to chasing it down 
till now.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75004

Files:
  lldb/test/API/macosx/profile_vrs_detach/Makefile
  lldb/test/API/macosx/profile_vrs_detach/TestDetachVrsProfile.py
  lldb/test/API/macosx/profile_vrs_detach/main.c
  lldb/tools/debugserver/source/MacOSX/MachProcess.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.mm

Index: lldb/tools/debugserver/source/MacOSX/MachProcess.mm
===================================================================
--- lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ 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 @@
       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 @@
     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 @@
   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 @@
       // 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;
 }
Index: lldb/tools/debugserver/source/MacOSX/MachProcess.h
===================================================================
--- lldb/tools/debugserver/source/MacOSX/MachProcess.h
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.h
@@ -338,9 +338,16 @@
     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 @@
       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
Index: lldb/test/API/macosx/profile_vrs_detach/main.c
===================================================================
--- /dev/null
+++ 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;
+}
Index: lldb/test/API/macosx/profile_vrs_detach/TestDetachVrsProfile.py
===================================================================
--- /dev/null
+++ 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")
Index: lldb/test/API/macosx/profile_vrs_detach/Makefile
===================================================================
--- /dev/null
+++ lldb/test/API/macosx/profile_vrs_detach/Makefile
@@ -0,0 +1,4 @@
+C_SOURCES := main.c
+CFLAGS_EXTRAS := -std=c99
+
+include Makefile.rules
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to