Skip to content

Commit c6617e5

Browse files
authored
[linux] handle Ctrl-C interrupt packet when inferior is already stopped
## Purpose Properly support interruption from the debugger via Ctrl-C (\x03) packet when no thread in the inferior process is running ## Overview * Implement a `sendInterrupt` method in the `POSIX::Process` class that can be used to fork a short-lived process whenever an interrupt packet is received. The forked process immediately exits, generating a process exit event. * Implement a `checkInterrupt` method in the `POSIX::Process` class that compares a provided pid against the pid that was memoized on the last `sendInterrupt` call. If they match, it clears the the pid and returns true. * Wire-up `sendInterrupt` to the `Linux::Process:interrupt` method. There was previously no Linux override for this `Process` method, so there's a bit of additional scaffolding for this. * Wire-up `checkInterrupt` to the `Linux::Process::wait` method and properly handle the interrupt. * Enable two lldb tests on Android that were previously failing due to this bug. NOTE: This PR only addresses Linux and Android, but the bug most likely impacts Darwin and FreeBSD platforms as well. The implementations of `sendInterrupt` and `checkInterrupt` are in `POSIX::Process` so they can be reused on these platforms. ## Problem Details The ds2 main thread blocks indefinitely on a `::waitpid()` call from `Linux::Process::wait()`. In this scenario, if the inferior process never generates any process events, ds2 is effectively hung. However, the debug server needs to always be able to be interrupted with a Ctrl-C (\x03) packet from the debugger. Ds2 already properly handles this command (and only this command) off of the main thread attempting to stop the inferior process with a `kill()` call. However, the `kill()` call is a noop if all threads in the inferior process are already stopped. In this scenario the lldb <-> ds2 connection is effectively hung. ## Validation The re-enabled tests now pass consistently locally on Android and Linux.
1 parent a55e03a commit c6617e5

File tree

5 files changed

+116
-2
lines changed

5 files changed

+116
-2
lines changed

Headers/DebugServer2/Target/Linux/Process.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ class Process : public POSIX::ELFProcess {
2525
ErrorCode attach(int waitStatus) override;
2626

2727
public:
28+
ErrorCode interrupt() override;
2829
ErrorCode terminate() override;
2930
bool isAlive() const override;
3031

Headers/DebugServer2/Target/POSIX/Process.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
#include "DebugServer2/Host/ProcessSpawner.h"
1515
#include "DebugServer2/Target/ProcessBase.h"
1616

17+
#include <mutex>
18+
1719
namespace ds2 {
1820
namespace Target {
1921
namespace POSIX {
@@ -22,10 +24,20 @@ class Process : public ds2::Target::ProcessBase {
2224
protected:
2325
std::set<int> _passthruSignals;
2426

27+
// state used by sendInterrupt and checkInterrupt
28+
struct {
29+
std::mutex mutex;
30+
ProcessId pid = 0;
31+
} _interruptState;
32+
2533
protected:
2634
ErrorCode initialize(ProcessId pid, uint32_t flags) override;
2735
virtual ErrorCode attach(int waitStatus) = 0;
2836

37+
protected:
38+
ErrorCode sendInterrupt();
39+
bool checkInterrupt(ThreadId tid, int waitStatus);
40+
2941
public:
3042
ErrorCode detach() override;
3143
ErrorCode interrupt() override;

Sources/Target/Linux/Process.cpp

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,29 @@ ErrorCode Process::wait() {
189189
DS2LOG(Debug, "tid %" PRI_PID " %s", tid, Stringify::WaitStatus(status));
190190

191191
auto threadIt = _threads.find(tid);
192+
if (super::checkInterrupt(tid, status)) {
193+
DS2ASSERT(threadIt == _threads.end());
194+
// We were explicitly interrupted. In this scenario, check the state of
195+
// all threads and resume a stopped one if none is running.
196+
bool running = false;
197+
Thread* stoppedThread = nullptr;
198+
enumerateThreads([&](Thread *thread) {
199+
if (stoppedThread == nullptr && thread->state() == Thread::kStopped)
200+
stoppedThread = thread;
201+
else if (thread->state() == Thread::kRunning)
202+
running = true;
203+
});
204+
205+
if (!running && stoppedThread != nullptr) {
206+
DS2LOG(Debug, "interrupted by %" PRI_PID "; resuming %" PRI_PID,
207+
tid, stoppedThread->tid());
208+
CHK(stoppedThread->resume());
209+
} else {
210+
DS2LOG(Debug, "interrupted by %" PRI_PID "; cleared and ignored", tid);
211+
}
212+
213+
goto continue_waiting;
214+
}
192215

193216
if (threadIt == _threads.end()) {
194217
// If we don't know about this thread yet, but it has a WIFEXITED() or a
@@ -345,6 +368,16 @@ ErrorCode Process::wait() {
345368
return kSuccess;
346369
}
347370

371+
ErrorCode Process::interrupt() {
372+
// Unconditionally send an interrupt here even if sending kill(STOP) to the
373+
// inferior will generate waitpid() events. Because this method can be called
374+
// off of the main thread, it is potentially unsafe to inspect process state
375+
// here. Instead, state is inspected on receipt of the interrupt in
376+
// Linux::Process:wait(), which will ignore any unnecessary interrupts.
377+
CHK(super::sendInterrupt());
378+
return super::interrupt();
379+
}
380+
348381
ErrorCode Process::terminate() {
349382
ErrorCode error = super::terminate();
350383
if (error == kSuccess || error == kErrorProcessNotFound) {

Sources/Target/POSIX/Process.cpp

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
#include "DebugServer2/Target/Process.h"
1212
#include "DebugServer2/Core/BreakpointManager.h"
13+
#include "DebugServer2/Host/Platform.h"
1314
#include "DebugServer2/Host/POSIX/PTrace.h"
1415
#include "DebugServer2/Target/Thread.h"
1516
#include "DebugServer2/Utils/Log.h"
@@ -176,6 +177,75 @@ void Process::setSignalPass(int signo, bool set) {
176177
_passthruSignals.erase(signo);
177178
}
178179
}
180+
181+
// Used to interrupt calls to wait(2), waitpid(2), and waitid(2) that are
182+
// blocked waiting for inferior process events. Requried in the scenario where
183+
// the debug server's main thread is blocked waiting for events from an
184+
// inferior process whose threads are already in a stopped state. E.g.:
185+
// 1. All threads of the inferior process are in a stopped state.
186+
// 2. The main thread of the debug server is blocked in a waitpid() call
187+
// waiting for an event from the inferior process.
188+
// 2. The debug server process receives an asynchronous \x03 interrupt packet
189+
// from the debugger and attempts to interrupt the inferior by calling
190+
// kill(SIGSTOP).
191+
// 3. The kill(SIGSTOP) call has no effect on the inferior process because
192+
// every thread is already stopped.
193+
// 4. The call to waitpid() remains blocked indefinitely.
194+
//
195+
// In this situation, the debug server can use sendInterrupt in the following
196+
// way to interrupt the waitpid() call:
197+
// 1. In response to the interrupt packet from the debugger, the debug server
198+
// calls sendInterrupt() in addition to kill(SIGSTOP). This call forks a
199+
// short-lived child process and saves its pid. The child process
200+
// immediately exits, generating a thread exit event.
201+
// 2. The debug server's main thread now returns from waitpid(). It calls
202+
// checkInterrupt() with the results from waitpid(). The call compares the
203+
// pid returned by waitpid() with the pid stashed by sendInterrupt(), which
204+
// returns true if pid matches the one saved during the last call to
205+
// sendInterrupt().
206+
//
207+
// NOTE: There can be only one pending interrupt at a time. Subsequent calls
208+
// to sendInterrupt() without corresponding calls to checkInterrupt() are
209+
// noops.
210+
ErrorCode Process::sendInterrupt() {
211+
// Mutex guards against concurrent calls to checkInterrupt and sendInterrupt.
212+
// It is required to atomically fork the interrupting process and memoize it's
213+
// pid in the _pid field.
214+
std::lock_guard<std::mutex> lock(_interruptState.mutex);
215+
if (_interruptState.pid > 0) // There is already a pending interrupt
216+
return kSuccess;
217+
218+
const ProcessId pid = ::fork();
219+
if (pid < 0)
220+
return Host::Platform::TranslateError();
221+
222+
if (pid == 0) {
223+
DS2LOG(Debug, "forked process %" PRI_PID " to interrupt waiter",
224+
::getpid());
225+
// Exiting the forked thread will wake the waiting parent process with
226+
// WIFEXITED status.
227+
exit(0);
228+
}
229+
230+
_interruptState.pid = pid;
231+
return kSuccess;
232+
}
233+
234+
bool Process::checkInterrupt(ThreadId tid, int waitStatus) {
235+
{
236+
std::lock_guard<std::mutex> lock(_interruptState.mutex);
237+
if (_interruptState.pid <= 0) // no interrupt
238+
return false;
239+
240+
if (_interruptState.pid != tid || !WIFEXITED(waitStatus)) // tid does not match interrupt
241+
return false;
242+
243+
_interruptState.pid = 0; // clear interrupt
244+
}
245+
246+
DS2LOG(Debug, "received interrupt from process %" PRI_PID, tid);
247+
return true;
248+
}
179249
} // namespace POSIX
180250
} // namespace Target
181251
} // namespace ds2

Support/Testing/Excluded/ds2/android-x86_64.excluded

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,6 @@ TestDWIMPrint.TestCase.test_expressions_dwo
3131
TestExec.ExecTestCase.test_correct_thread_plan_state_before_exec
3232
TestExec.ExecTestCase.test_hitting_exec
3333
TestExec.ExecTestCase.test_skipping_exec
34-
TestExitDuringExpression.TestExitDuringExpression.test_exit_before_one_thread_no_unwind
35-
TestExitDuringExpression.TestExitDuringExpression.test_exit_before_one_thread_unwind
3634
TestExpressionInSyscall.ExprSyscallTestCase.test_setpgid_dwarf
3735
TestExpressionInSyscall.ExprSyscallTestCase.test_setpgid_dwo
3836
TestExprs.BasicExprCommandsTestCase.test_expr_commands_can_handle_quotes_dwarf

0 commit comments

Comments
 (0)