[Lldb-commits] [lldb] [lldb] Handle SIGINT via the MainLoop signal thread (on POSIX) (#195959) (PR #196687)

[via lldb-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

<details>
<summary>Changes</summary>

The driver's async SIGINT handler called
SBDebugger::DispatchInputInterrupt directly. That is not async-signal-safe and can lead to a crash.

Register SIGINT with the existing signal-thread MainLoop instead so DispatchInputInterrupt runs in normal thread context. The Windows path is unchanged and keeps the legacy async handler.

While DispatchInputInterrupt runs, the callback temporarily installs SIG_DFL so a second Ctrl-C still hard-terminates the process, preserving the escape hatch users rely on when the debugger is unresponsive.

Moving SIGINT off the main thread means a Ctrl-C no longer interrupts blocking syscalls there (e.g. a Python REPL waiting on input or sleeping), so Python never observes the queued interrupt and KeyboardInterrupt is not raised. To restore that behavior, after dispatching the interrupt the callback re-raises SIGINT on the main thread via pthread_kill; the resulting EINTR lets Python pick up the pending interrupt. A skip flag suppresses the re-entry that this self-send produces. Because the callback only ever runs on the signal thread, the flag and the captured main-thread id live in the lambda's captures and need no synchronization.

rdar://158218595

---
Full diff: https://github.com/llvm/llvm-project/pull/196687.diff


1 Files Affected:

- (modified) lldb/tools/driver/Driver.cpp (+56-4) 


``````````diff
diff --git a/lldb/tools/driver/Driver.cpp b/lldb/tools/driver/Driver.cpp
index d47d3daf1c3fc..e58286f9ff41e 100644
--- a/lldb/tools/driver/Driver.cpp
+++ b/lldb/tools/driver/Driver.cpp
@@ -43,6 +43,9 @@
 #include <clocale>
 #include <csignal>
 #include <future>
+#ifndef _WIN32
+#include <pthread.h>
+#endif
 #include <string>
 #include <thread>
 #include <utility>
@@ -651,11 +654,10 @@ void Driver::UpdateWindowSize() {
   }
 }
 
-void sigint_handler(int signo) {
 #ifdef _WIN32
+void sigint_handler(int signo) {
   // Restore handler as it is not persistent on Windows.
   signal(SIGINT, sigint_handler);
-#endif
 
   static std::atomic_flag g_interrupt_sent = ATOMIC_FLAG_INIT;
   if (g_driver != nullptr) {
@@ -668,6 +670,7 @@ void sigint_handler(int signo) {
 
   _exit(signo);
 }
+#endif
 
 static void printHelp(LLDBOptTable &table, llvm::StringRef tool_name) {
   std::string usage_str = tool_name.str() + " [options]";
@@ -781,15 +784,64 @@ int main(int argc, char const *argv[]) {
   // Setup LLDB signal handlers once the debugger has been initialized.
   SBDebugger::PrintDiagnosticsOnError();
 
-  //  FIXME: Migrate the SIGINT handler to be handled by the signal loop below.
+#ifdef _WIN32
   signal(SIGINT, sigint_handler);
-#if !defined(_WIN32)
+#else
   signal(SIGPIPE, SIG_IGN);
 
+  // Capture the main thread's id so the signal thread can target it.
+  pthread_t main_thread = pthread_self();
+
+  // Set when the signal thread sends itself a SIGINT to wake the main thread.
+  // The next callback invocation observes this flag and skips the work. A
+  // plain bool is sufficient because the callback only ever runs on the
+  // signal thread; it lives outside the lambda because MainLoopPosix copies
+  // the callback on every dispatch, which would discard in-lambda state.
+  bool skip_next_sigint = false;
+
   // Handle signals in a MainLoop running on a separate thread.
   MainLoop signal_loop;
   Status signal_status;
 
+  auto sigint_handler = signal_loop.RegisterSignal(
+      SIGINT,
+      [&, main_thread](MainLoopBase &) {
+        // Skip the self-sent wakeup SIGINT queued at the end of the previous
+        // invocation.
+        if (std::exchange(skip_next_sigint, false))
+          return;
+
+        // Temporarily restore the default disposition so that a second SIGINT
+        // delivered while DispatchInputInterrupt is running hard-terminates
+        // the process. This preserves the "double Ctrl-C to force exit"
+        // escape hatch users rely on when the debugger is unresponsive.
+        struct sigaction old_action;
+        struct sigaction new_action = {};
+        new_action.sa_handler = SIG_DFL;
+        sigemptyset(&new_action.sa_mask);
+
+        int ret = sigaction(SIGINT, &new_action, &old_action);
+        UNUSED_IF_ASSERT_DISABLED(ret);
+        assert(ret == 0 && "sigaction failed");
+
+        if (g_driver)
+          g_driver->GetDebugger().DispatchInputInterrupt();
+
+        ret = sigaction(SIGINT, &old_action, nullptr);
+        UNUSED_IF_ASSERT_DISABLED(ret);
+        assert(ret == 0 && "sigaction failed");
+
+        // Wake the main thread so any blocking syscall (e.g. the Python REPL
+        // waiting on input or sleeping) returns with EINTR. This lets Python
+        // observe the pending interrupt queued by DispatchInputInterrupt and
+        // raise KeyboardInterrupt. Flag the resulting callback invocation so
+        // it's skipped rather than re-running DispatchInputInterrupt.
+        skip_next_sigint = true;
+        pthread_kill(main_thread, SIGINT);
+      },
+      signal_status);
+  assert(sigint_handler && signal_status.Success());
+
   auto sigwinch_handler = signal_loop.RegisterSignal(
       SIGWINCH,
       [&](MainLoopBase &) {

``````````

</details>


https://github.com/llvm/llvm-project/pull/196687