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

[via lldb-commits]

Author: Jonas Devlieghere
Date: 2025-04-10T11:04:05-07:00
New Revision: 6493345c5ab96f60ab5ee38272fb6635f2083318

URL: https://github.com/llvm/llvm-project/commit/6493345c5ab96f60ab5ee38272fb6635f2083318
DIFF: https://github.com/llvm/llvm-project/commit/6493345c5ab96f60ab5ee38272fb6635f2083318.diff

LOG: [lldb] Handle signals in a separate thread in the driver (#134956)

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.

Added: 
    

Modified: 
    lldb/tools/driver/CMakeLists.txt
    lldb/tools/driver/Driver.cpp

Removed: 
    


################################################################################
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..e19fded051941 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,12 @@ void Driver::UpdateWindowSize() {
   }
 }
 
-void sigwinch_handler(int signo) {
-  if (g_driver != nullptr)
-    g_driver->UpdateWindowSize();
-}
-
 void sigint_handler(int signo) {
-#ifdef _WIN32 // Restore handler as it is not persistent on Windows
+#ifdef _WIN32
+  // 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()) {
@@ -657,31 +659,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) {
   std::string usage_str = tool_name.str() + " [options]";
   table.printHelp(llvm::outs(), usage_str.c_str(), "LLDB", false);
@@ -787,11 +764,53 @@ 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.
   signal(SIGINT, sigint_handler);
 #if !defined(_WIN32)
   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 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 = {};
+        new_action.sa_handler = SIG_DFL;
+        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 +843,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;
 }