[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 ea8bfa041c697a1f1726f5a92307f7d32d263e7d 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 |  1 +
 lldb/tools/driver/Driver.cpp     | 78 +++++++++++++++++++-------------
 2 files changed, 48 insertions(+), 31 deletions(-)

diff --git a/lldb/tools/driver/CMakeLists.txt b/lldb/tools/driver/CMakeLists.txt
index 89884ecd0601b..fba61199091ea 100644
--- a/lldb/tools/driver/CMakeLists.txt
+++ b/lldb/tools/driver/CMakeLists.txt
@@ -22,6 +22,7 @@ add_lldb_tool(lldb
 
   LINK_LIBS
     liblldb
+    lldbHost
 
   LINK_COMPONENTS
     Option
diff --git a/lldb/tools/driver/Driver.cpp b/lldb/tools/driver/Driver.cpp
index 15cb0134fec8e..36c5a9954c6ca 100644
--- a/lldb/tools/driver/Driver.cpp
+++ b/lldb/tools/driver/Driver.cpp
@@ -19,7 +19,8 @@
 #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 "llvm/ADT/StringRef.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/InitLLVM.h"
@@ -50,6 +51,8 @@
 
 using namespace lldb;
 using namespace llvm;
+using lldb_private::MainLoop;
+using lldb_private::MainLoopBase;
 
 namespace {
 using namespace llvm::opt;
@@ -89,6 +92,7 @@ static struct termios g_old_stdin_termios;
 static bool disable_color(const raw_ostream &OS) { return false; }
 
 static Driver *g_driver = nullptr;
+static MainLoop g_signal_loop;
 
 // In the Driver::MainLoop, we change the terminal settings.  This function is
 // added as an atexit handler to make sure we clean them up.
@@ -637,48 +641,50 @@ void Driver::UpdateWindowSize() {
 }
 
 void sigwinch_handler(int signo) {
-  if (g_driver != nullptr)
-    g_driver->UpdateWindowSize();
+  g_signal_loop.AddPendingCallback([](MainLoopBase &loop) {
+    if (g_driver != nullptr)
+      g_driver->UpdateWindowSize();
+  });
 }
 
 void sigint_handler(int signo) {
 #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()) {
-      g_driver->GetDebugger().DispatchInputInterrupt();
-      g_interrupt_sent.clear();
-      return;
+  g_signal_loop.AddPendingCallback([signo](MainLoopBase &loop) {
+    static std::atomic_flag g_interrupt_sent = ATOMIC_FLAG_INIT;
+    if (g_driver != nullptr) {
+      if (!g_interrupt_sent.test_and_set()) {
+        g_driver->GetDebugger().DispatchInputInterrupt();
+        g_interrupt_sent.clear();
+        return;
+      }
     }
-  }
 
-  _exit(signo);
+    loop.RequestTermination();
+    _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();
+  g_signal_loop.AddPendingCallback([signo](MainLoopBase &loop) {
+    if (g_driver != nullptr)
+      g_driver->GetDebugger().SaveInputTerminalState();
+
+    // Remove our handler.
+    signal(signo, SIG_DFL);
+
+    // Now re-raise the signal. We will immediately suspend...
+    raise(signo);
+    // ... and resume after a SIGCONT.
+
+    // Now undo the modifications.
+    signal(signo, sigtstp_handler);
+
+    if (g_driver != nullptr)
+      g_driver->GetDebugger().RestoreInputTerminalState();
+  });
 }
 #endif
 
@@ -794,6 +800,10 @@ int main(int argc, char const *argv[]) {
   signal(SIGTSTP, sigtstp_handler);
 #endif
 
+  // Run the signal handling MainLoop on a separate thread.
+  std::thread signal_thread([] { g_signal_loop.Run(); });
+  signal_thread.detach();
+
   int exit_code = 0;
   // Create a scope for driver so that the driver object will destroy itself
   // before SBDebugger::Terminate() is called.
@@ -824,5 +834,11 @@ int main(int argc, char const *argv[]) {
     future.wait();
   }
 
+  // Stop the signal handler thread. Do this after calling SBDebugger::Terminate
+  // so that impatient users can send a SIGSTOP if they don't want to wait for
+  // the background threads to exit cleanly.
+  g_signal_loop.AddPendingCallback(
+      [](MainLoopBase &loop) { loop.RequestTermination(); });
+
   return exit_code;
 }