[Lldb-commits] [lldb] [lldb] Handle signals in a separate thread in the driver (PR #134956)

[Jonas Devlieghere via lldb-commits]

https://github.com/JDevlieghere updated https://github.com/llvm/llvm-project/pull/134956

>From 3943112f5182065eed43d72e2960ed166737b237 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Tue, 8 Apr 2025 17:33:41 -0700
Subject: [PATCH] [lldb] Handle signals in a separate thread in the driver

Handle signals in a separate thread in the driver so that we can stop
worrying about signal safety of functions in libLLDB that may get called
from a signal handler.
---
 lldb/tools/driver/CMakeLists.txt |   2 +
 lldb/tools/driver/Driver.cpp     | 117 ++++++++++++++++++++++---------
 2 files changed, 84 insertions(+), 35 deletions(-)

diff --git a/lldb/tools/driver/CMakeLists.txt b/lldb/tools/driver/CMakeLists.txt
index 89884ecd0601b..8b4aa92f96f0d 100644
--- a/lldb/tools/driver/CMakeLists.txt
+++ b/lldb/tools/driver/CMakeLists.txt
@@ -22,6 +22,8 @@ add_lldb_tool(lldb
 
   LINK_LIBS
     liblldb
+    lldbHost
+    lldbUtility
 
   LINK_COMPONENTS
     Option
diff --git a/lldb/tools/driver/Driver.cpp b/lldb/tools/driver/Driver.cpp
index 15cb0134fec8e..2557c08c0e396 100644
--- a/lldb/tools/driver/Driver.cpp
+++ b/lldb/tools/driver/Driver.cpp
@@ -19,7 +19,9 @@
 #include "lldb/API/SBStringList.h"
 #include "lldb/API/SBStructuredData.h"
 #include "lldb/Host/Config.h"
-
+#include "lldb/Host/MainLoop.h"
+#include "lldb/Host/MainLoopBase.h"
+#include "lldb/Utility/Status.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/InitLLVM.h"
@@ -50,6 +52,9 @@
 
 using namespace lldb;
 using namespace llvm;
+using lldb_private::MainLoop;
+using lldb_private::MainLoopBase;
+using lldb_private::Status;
 
 namespace {
 using namespace llvm::opt;
@@ -636,15 +641,11 @@ void Driver::UpdateWindowSize() {
   }
 }
 
-void sigwinch_handler(int signo) {
-  if (g_driver != nullptr)
-    g_driver->UpdateWindowSize();
-}
-
+#ifdef _WIN32
 void sigint_handler(int signo) {
-#ifdef _WIN32 // Restore handler as it is not persistent on Windows
+  // 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) {
     if (!g_interrupt_sent.test_and_set()) {
@@ -656,30 +657,6 @@ void sigint_handler(int signo) {
 
   _exit(signo);
 }
-
-#ifndef _WIN32
-static void sigtstp_handler(int signo) {
-  if (g_driver != nullptr)
-    g_driver->GetDebugger().SaveInputTerminalState();
-
-  // Unblock the signal and remove our handler.
-  sigset_t set;
-  sigemptyset(&set);
-  sigaddset(&set, signo);
-  pthread_sigmask(SIG_UNBLOCK, &set, nullptr);
-  signal(signo, SIG_DFL);
-
-  // Now re-raise the signal. We will immediately suspend...
-  raise(signo);
-  // ... and resume after a SIGCONT.
-
-  // Now undo the modifications.
-  pthread_sigmask(SIG_BLOCK, &set, nullptr);
-  signal(signo, sigtstp_handler);
-
-  if (g_driver != nullptr)
-    g_driver->GetDebugger().RestoreInputTerminalState();
-}
 #endif
 
 static void printHelp(LLDBOptTable &table, llvm::StringRef tool_name) {
@@ -787,11 +764,75 @@ int main(int argc, char const *argv[]) {
   // Setup LLDB signal handlers once the debugger has been initialized.
   SBDebugger::PrintDiagnosticsOnError();
 
+#if defined(_WIN32)
+  //  FIXME: Support signals in MainLoopWindows and handle SIGINT in the signal
+  //  loop below.
   signal(SIGINT, sigint_handler);
-#if !defined(_WIN32)
+#else
   signal(SIGPIPE, SIG_IGN);
-  signal(SIGWINCH, sigwinch_handler);
-  signal(SIGTSTP, sigtstp_handler);
+
+  // Handle signals in a MainLoop running on a separate thread.
+  MainLoop signal_loop;
+  Status signal_status;
+
+  auto sigint_handler = signal_loop.RegisterSignal(
+      SIGINT,
+      [&](MainLoopBase &loop) {
+        static std::atomic_flag g_interrupt_sent = ATOMIC_FLAG_INIT;
+        if (g_driver) {
+          if (!g_interrupt_sent.test_and_set()) {
+            g_driver->GetDebugger().DispatchInputInterrupt();
+            g_interrupt_sent.clear();
+            return;
+          }
+        }
+        loop.RequestTermination();
+        _exit(SIGINT);
+      },
+      signal_status);
+  assert(sigint_handler && signal_status.Success());
+
+  auto sigwinch_handler = signal_loop.RegisterSignal(
+      SIGWINCH,
+      [&](MainLoopBase &) {
+        if (g_driver)
+          g_driver->UpdateWindowSize();
+      },
+      signal_status);
+  assert(sigwinch_handler && signal_status.Success());
+
+  auto sigtstp_handler = signal_loop.RegisterSignal(
+      SIGTSTP,
+      [&](MainLoopBase &) {
+        if (g_driver)
+          g_driver->GetDebugger().SaveInputTerminalState();
+
+        struct sigaction old_action;
+        struct sigaction new_action;
+
+        memset(&new_action, 0, sizeof(new_action));
+        new_action.sa_handler = SIG_DFL;
+        new_action.sa_flags = SA_SIGINFO;
+        sigemptyset(&new_action.sa_mask);
+        sigaddset(&new_action.sa_mask, SIGTSTP);
+
+        int ret = sigaction(SIGTSTP, &new_action, &old_action);
+        UNUSED_IF_ASSERT_DISABLED(ret);
+        assert(ret == 0 && "sigaction failed");
+
+        raise(SIGTSTP);
+
+        ret = sigaction(SIGTSTP, &old_action, nullptr);
+        UNUSED_IF_ASSERT_DISABLED(ret);
+        assert(ret == 0 && "sigaction failed");
+
+        if (g_driver)
+          g_driver->GetDebugger().RestoreInputTerminalState();
+      },
+      signal_status);
+  assert(sigtstp_handler && signal_status.Success());
+
+  std::thread signal_thread([&] { signal_loop.Run(); });
 #endif
 
   int exit_code = 0;
@@ -824,5 +865,11 @@ int main(int argc, char const *argv[]) {
     future.wait();
   }
 
+#if !defined(_WIN32)
+  signal_loop.AddPendingCallback(
+      [](MainLoopBase &loop) { loop.RequestTermination(); });
+  signal_thread.join();
+#endif
+
   return exit_code;
 }