[Lldb-commits] [lldb] [lldb] Print a message when background tasks take a while to complete (PR #82799)

Jonas Devlieghere via lldb-commits lldb-commits at lists.llvm.org
Fri Feb 23 09:28:22 PST 2024


https://github.com/JDevlieghere created https://github.com/llvm/llvm-project/pull/82799

When terminating the debugger, we wait for all background tasks to complete. Given that there's no way to interrupt those treads, this can take a while. When that happens, the debugger appears to hang at exit.

The above situation is unfortunately not uncommon when background downloading of dSYMs is enabled (`symbols.auto-download background`). Even when calling dsymForUUID with a reasonable timeout, it can take a while to complete.

This patch improves the user experience by registering a callback and printing a message when we're waiting on the thread pool for more than a second. Both the timeout and the callback is configurable.

Fixing the underlying issue is a much harder problem. Neither C++ threads no pthread support interrupting threads, so it would have to be a cooperative interruption mechanism, similar to how we deal with this for HostThreads. If we go that route, we should do it at the thread pool level so that it applies to all background tasks.

A cooperative interruption mechanism alone solve the dsymForUUID. It's a blocking call to an external binary, so we'd have to monitor for interrupts and kill the execution from yet another thread.

>From 8ff01f8e7060ba46540503a48c9ec8bbefd2076c Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Fri, 23 Feb 2024 09:13:06 -0800
Subject: [PATCH] [lldb] Print a message when background tasks take a while to
 complete

When terminating the debugger, we wait for all background tasks to
complete. Given that there's no way to interrupt those treads, this can
take a while. When that happens, the debugger appears to hang at exit.

The above situation is unfortunately not uncommon when background
downloading of dSYMs is enabled (`symbols.auto-download background`).
Even when calling dsymForUUID with a reasonable timeout, it can take a
while to complete.

This patch improves the user experience by registering a callback and
printing a message when we're waiting on the thread pool for more than a
second. Both the timeout and the callback is configurable.

Fixing the underlying issue is a much harder problem. Neither C++
threads no pthread support interrupting threads, so it would have to be
a cooperative interruption mechanism, similar to how we deal with this
for HostThreads. If we go that route, we should do it at the thread pool
level so that it applies to all background tasks.

A cooperative interruption mechanism alone solve the dsymForUUID. It's a
blocking call to an external binary, so we'd have to monitor for
interrupts and kill the execution from yet another thread.
---
 lldb/include/lldb/API/SBDebugger.h |  7 +++++++
 lldb/include/lldb/API/SBDefines.h  |  1 +
 lldb/include/lldb/Core/Debugger.h  |  5 +++++
 lldb/include/lldb/lldb-types.h     |  1 +
 lldb/source/API/SBDebugger.cpp     |  8 ++++++++
 lldb/source/Core/Debugger.cpp      | 31 ++++++++++++++++++++++++++++--
 lldb/tools/driver/Driver.cpp       |  6 ++++++
 7 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/lldb/include/lldb/API/SBDebugger.h b/lldb/include/lldb/API/SBDebugger.h
index 62b2f91f5076d5..c2b4e6adfa5d27 100644
--- a/lldb/include/lldb/API/SBDebugger.h
+++ b/lldb/include/lldb/API/SBDebugger.h
@@ -331,6 +331,13 @@ class LLDB_API SBDebugger {
   void SetDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback,
                           void *baton);
 
+  /// Register a callback that is called when destroying the thread pool exceeds
+  /// the given timeout.
+  static void
+  SetThreadPoolTimeoutCallback(uint64_t timeout_milliseconds,
+                               lldb::SBDebuggerTimeoutCallback timeout_callback,
+                               void *baton);
+
 #ifndef SWIG
   LLDB_DEPRECATED_FIXME("Use DispatchInput(const void *, size_t)",
                         "DispatchInput(const void *, size_t)")
diff --git a/lldb/include/lldb/API/SBDefines.h b/lldb/include/lldb/API/SBDefines.h
index 1181920677b46f..5df380a250278c 100644
--- a/lldb/include/lldb/API/SBDefines.h
+++ b/lldb/include/lldb/API/SBDefines.h
@@ -138,6 +138,7 @@ typedef bool (*SBBreakpointHitCallback)(void *baton, SBProcess &process,
 
 typedef void (*SBDebuggerDestroyCallback)(lldb::user_id_t debugger_id,
                                           void *baton);
+typedef void (*SBDebuggerTimeoutCallback)(void *baton);
 
 typedef SBError (*SBPlatformLocateModuleCallback)(
     void *baton, const SBModuleSpec &module_spec, SBFileSpec &module_file_spec,
diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h
index 6ba90eb6ed8fdf..b16334d302a47c 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -250,6 +250,11 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
 
   void SetLoggingCallback(lldb::LogOutputCallback log_callback, void *baton);
 
+  static void
+  SetThreadPoolTimeoutCallback(std::chrono::milliseconds timeout_milliseconds,
+                               lldb::TimeoutCallback timeout_callback,
+                               void *baton);
+
   // Properties Functions
   enum StopDisassemblyType {
     eStopDisassemblyTypeNever = 0,
diff --git a/lldb/include/lldb/lldb-types.h b/lldb/include/lldb/lldb-types.h
index d60686e33142ac..e1d58f02f60820 100644
--- a/lldb/include/lldb/lldb-types.h
+++ b/lldb/include/lldb/lldb-types.h
@@ -69,6 +69,7 @@ typedef int pipe_t;                     // Host pipe type
 #define LLDB_INVALID_HOST_THREAD ((lldb::thread_t)NULL)
 #define LLDB_INVALID_PIPE ((lldb::pipe_t)-1)
 
+typedef void (*TimeoutCallback)(void *baton);
 typedef void (*LogOutputCallback)(const char *, void *baton);
 typedef bool (*CommandOverrideCallback)(void *baton, const char **argv);
 typedef bool (*ExpressionCancelCallback)(ExpressionEvaluationPhase phase,
diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index fbcf30e67fc1cd..5c78b4fa05aaaf 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -1695,6 +1695,14 @@ void SBDebugger::SetDestroyCallback(
   }
 }
 
+void SBDebugger::SetThreadPoolTimeoutCallback(
+    uint64_t timeout_milliseconds,
+    lldb::SBDebuggerTimeoutCallback timeout_callback, void *baton) {
+  LLDB_INSTRUMENT_VA(timeout_milliseconds, timeout_callback, baton);
+  Debugger::SetThreadPoolTimeoutCallback(
+      std::chrono::milliseconds(timeout_milliseconds), timeout_callback, baton);
+}
+
 SBTrace
 SBDebugger::LoadTraceFromFile(SBError &error,
                               const SBFileSpec &trace_description_file) {
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index c3e603dbae8961..c48ca3bfe677b0 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -103,7 +103,14 @@ static std::recursive_mutex *g_debugger_list_mutex_ptr =
     nullptr; // NOTE: intentional leak to avoid issues with C++ destructor chain
 static Debugger::DebuggerList *g_debugger_list_ptr =
     nullptr; // NOTE: intentional leak to avoid issues with C++ destructor chain
+
+/// Thread Pool
+/// @{
 static llvm::ThreadPool *g_thread_pool = nullptr;
+static std::chrono::milliseconds g_timeout_milliseconds;
+static lldb::TimeoutCallback g_timeout_callback = nullptr;
+static void *g_timeout_callback_baton = nullptr;
+/// @}
 
 static constexpr OptionEnumValueElement g_show_disassembly_enum_values[] = {
     {
@@ -623,8 +630,20 @@ void Debugger::Terminate() {
   }
 
   if (g_thread_pool) {
-    // The destructor will wait for all the threads to complete.
-    delete g_thread_pool;
+    // Delete the thread pool in a different thread so we can set a timeout.
+    std::future<void> future = std::async(std::launch::async, []() {
+      // The destructor will wait for all the threads to complete.
+      delete g_thread_pool;
+    });
+
+    if (g_timeout_callback) {
+      if (future.wait_for(g_timeout_milliseconds) ==
+          std::future_status::timeout)
+        g_timeout_callback(g_timeout_callback_baton);
+    }
+
+    // Wait for the asynchronous task to complete.
+    future.wait();
   }
 
   if (g_debugger_list_ptr && g_debugger_list_mutex_ptr) {
@@ -2195,3 +2214,11 @@ llvm::ThreadPool &Debugger::GetThreadPool() {
          "Debugger::GetThreadPool called before Debugger::Initialize");
   return *g_thread_pool;
 }
+
+void Debugger::SetThreadPoolTimeoutCallback(
+    std::chrono::milliseconds timeout_milliseconds,
+    lldb::TimeoutCallback timeout_callback, void *baton) {
+  g_timeout_milliseconds = timeout_milliseconds;
+  g_timeout_callback = timeout_callback;
+  g_timeout_callback_baton = baton;
+}
diff --git a/lldb/tools/driver/Driver.cpp b/lldb/tools/driver/Driver.cpp
index c63ff0ff597e5c..b3ba1962141967 100644
--- a/lldb/tools/driver/Driver.cpp
+++ b/lldb/tools/driver/Driver.cpp
@@ -96,6 +96,10 @@ static void reset_stdin_termios() {
   }
 }
 
+static void thread_pool_timeout_callback(void *) {
+  fprintf(stderr, "Waiting for background tasks to complete...");
+}
+
 Driver::Driver()
     : SBBroadcaster("Driver"), m_debugger(SBDebugger::Create(false)) {
   // We want to be able to handle CTRL+D in the terminal to have it terminate
@@ -816,6 +820,8 @@ int main(int argc, char const *argv[]) {
     }
   }
 
+  SBDebugger::SetThreadPoolTimeoutCallback(
+      /*timeout_milliseconds=*/1000, &thread_pool_timeout_callback, nullptr);
   SBDebugger::Terminate();
   return exit_code;
 }



More information about the lldb-commits mailing list