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