[Lldb-commits] [lldb] 3cd13c4 - Fix a race between lldb's packet timeout and the profile thread's usleep.
Jim Ingham via lldb-commits
lldb-commits at lists.llvm.org
Tue Feb 25 11:17:36 PST 2020
Author: Jim Ingham
Date: 2020-02-25T11:17:08-08:00
New Revision: 3cd13c4624b5900c884b691b72d0ca053433f6fe
URL: https://github.com/llvm/llvm-project/commit/3cd13c4624b5900c884b691b72d0ca053433f6fe
DIFF: https://github.com/llvm/llvm-project/commit/3cd13c4624b5900c884b691b72d0ca053433f6fe.diff
LOG: Fix a race between lldb's packet timeout and the profile thread's usleep.
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
Added:
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
Modified:
lldb/tools/debugserver/source/MacOSX/MachProcess.h
lldb/tools/debugserver/source/MacOSX/MachProcess.mm
Removed:
################################################################################
diff --git a/lldb/test/API/macosx/profile_vrs_detach/Makefile b/lldb/test/API/macosx/profile_vrs_detach/Makefile
new file mode 100644
index 000000000000..695335e068c0
--- /dev/null
+++ b/lldb/test/API/macosx/profile_vrs_detach/Makefile
@@ -0,0 +1,4 @@
+C_SOURCES := main.c
+CFLAGS_EXTRAS := -std=c99
+
+include Makefile.rules
diff --git a/lldb/test/API/macosx/profile_vrs_detach/TestDetachVrsProfile.py b/lldb/test/API/macosx/profile_vrs_detach/TestDetachVrsProfile.py
new file mode 100644
index 000000000000..31a9bce78cdc
--- /dev/null
+++ b/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")
diff --git a/lldb/test/API/macosx/profile_vrs_detach/main.c b/lldb/test/API/macosx/profile_vrs_detach/main.c
new file mode 100644
index 000000000000..6bcd3342441b
--- /dev/null
+++ b/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;
+}
diff --git a/lldb/tools/debugserver/source/MacOSX/MachProcess.h b/lldb/tools/debugserver/source/MacOSX/MachProcess.h
index 378715985268..ec3a852ad839 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachProcess.h
+++ b/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
diff --git a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
index e4ec6129f70c..f6134e48a048 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ b/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;
}
More information about the lldb-commits
mailing list