[Lldb-commits] [lldb] [lldb] Fix deadlock in parallel module loading with separate symbol thread pool (PR #160225)

[via lldb-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: None (GeorgeHuyubo)

<details>
<summary>Changes</summary>

Fixes a deadlock that occurs during parallel module loading when symbol indexing
operations conflict with module list access. The deadlock happens when:

- Thread A (symbol loading): holds Module.m_mutex → tries ModuleList.m_modules_mutex  
- Thread B (parallel module loading): holds ModuleList.m_modules_mutex → tries Module.m_mutex

Solution: Create a dedicated symbol thread pool separate from the main thread pool
used for parallel module loading. This isolates DWARF indexing operations from
module loading operations, preventing the mutex ordering conflict.

Changes:
- Add Debugger::GetSymbolThreadPool() alongside existing GetThreadPool()
- Update ManualDWARFIndex to use symbol thread pool for indexing operations
- Initialize/cleanup symbol thread pool in Debugger::Initialize/Terminate

This maintains performance while eliminating the deadlock without major
architectural changes to the threading model.

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


3 Files Affected:

- (modified) lldb/include/lldb/Core/Debugger.h (+5) 
- (modified) lldb/source/Core/Debugger.cpp (+16) 
- (modified) lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp (+4-2) 


``````````diff
diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h
index 250ad64b76d9a..c3019ccb3077d 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -506,6 +506,11 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
   /// Shared thread pool. Use only with ThreadPoolTaskGroup.
   static llvm::ThreadPoolInterface &GetThreadPool();
 
+  /// Dedicated symbol thread pool to prevent deadlock with module loading.
+  /// Use this for symbol indexing operations that might need to access
+  /// the shared module list while holding module mutexes.
+  static llvm::ThreadPoolInterface &GetSymbolThreadPool();
+
   /// Report warning events.
   ///
   /// Warning events will be delivered to any debuggers that have listeners
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index ed674ee1275c7..9e662d5b6a024 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -110,6 +110,7 @@ static std::recursive_mutex *g_debugger_list_mutex_ptr =
 static Debugger::DebuggerList *g_debugger_list_ptr =
     nullptr; // NOTE: intentional leak to avoid issues with C++ destructor chain
 static llvm::DefaultThreadPool *g_thread_pool = nullptr;
+static llvm::DefaultThreadPool *g_symbol_thread_pool = nullptr;
 
 static constexpr OptionEnumValueElement g_show_disassembly_enum_values[] = {
     {
@@ -715,6 +716,8 @@ void Debugger::Initialize(LoadPluginCallbackType load_plugin_callback) {
   g_debugger_list_mutex_ptr = new std::recursive_mutex();
   g_debugger_list_ptr = new DebuggerList();
   g_thread_pool = new llvm::DefaultThreadPool(llvm::optimal_concurrency());
+  g_symbol_thread_pool =
+      new llvm::DefaultThreadPool(llvm::optimal_concurrency());
   g_load_plugin_callback = load_plugin_callback;
 }
 
@@ -731,6 +734,13 @@ void Debugger::Terminate() {
   if (g_thread_pool) {
     // The destructor will wait for all the threads to complete.
     delete g_thread_pool;
+    g_thread_pool = nullptr;
+  }
+
+  if (g_symbol_thread_pool) {
+    // The destructor will wait for all the threads to complete.
+    delete g_symbol_thread_pool;
+    g_symbol_thread_pool = nullptr;
   }
 
   if (g_debugger_list_ptr && g_debugger_list_mutex_ptr) {
@@ -2383,3 +2393,9 @@ llvm::ThreadPoolInterface &Debugger::GetThreadPool() {
          "Debugger::GetThreadPool called before Debugger::Initialize");
   return *g_thread_pool;
 }
+
+llvm::ThreadPoolInterface &Debugger::GetSymbolThreadPool() {
+  assert(g_symbol_thread_pool &&
+         "Debugger::GetSymbolThreadPool called before Debugger::Initialize");
+  return *g_symbol_thread_pool;
+}
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
index d90108f687f84..27298be688261 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -86,10 +86,12 @@ void ManualDWARFIndex::Index() {
                     total_progress, /*debugger=*/nullptr,
                     Progress::kDefaultHighFrequencyReportTime);
 
+  // Use separate symbol thread pool to avoid deadlock with module loading
   // Share one thread pool across operations to avoid the overhead of
   // recreating the threads.
-  llvm::ThreadPoolTaskGroup task_group(Debugger::GetThreadPool());
-  const size_t num_threads = Debugger::GetThreadPool().getMaxConcurrency();
+  llvm::ThreadPoolTaskGroup task_group(Debugger::GetSymbolThreadPool());
+  const size_t num_threads =
+      Debugger::GetSymbolThreadPool().getMaxConcurrency();
 
   // Run a function for each compile unit in parallel using as many threads as
   // are available. This is significantly faster than submiting a new task for

``````````

</details>


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