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
This commit is contained in:
Jim Ingham 2020-02-21 16:45:29 -08:00
parent 481b1c8380
commit 3cd13c4624
5 changed files with 127 additions and 11 deletions

View File

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

View File

@ -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")

View File

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

View File

@ -338,9 +338,16 @@ private:
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 @@ private:
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

View File

@ -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 @@ MachProcess::MachProcess()
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 @@ void MachProcess::Clear(bool detaching) {
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 @@ void MachProcess::SetEnableAsyncProfiling(bool enable, uint64_t interval_usec,
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 @@ void *MachProcess::ProfileThread(void *arg) {
// 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;
}