[Lldb-commits] [lldb] [LLDB] Fix deadlock in module callback when running in parallel (PR #168425)

[Jacob Lalonde via lldb-commits]

https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/168425

>From f2d3ad3c0ee73682b9877d919f5bbc176de8f0b5 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Mon, 17 Nov 2025 10:51:50 -0800
Subject: [PATCH 1/2] Move lock guard before AddTargetInternal instead of
 holding the mutex for the entire target creation and causing a deadlock with
 the module callback when running in parallel.

---
 lldb/source/Target/TargetList.cpp | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/lldb/source/Target/TargetList.cpp b/lldb/source/Target/TargetList.cpp
index 2e03bc1e38ea0..32d0be9a5cb06 100644
--- a/lldb/source/Target/TargetList.cpp
+++ b/lldb/source/Target/TargetList.cpp
@@ -48,11 +48,16 @@ Status TargetList::CreateTarget(Debugger &debugger,
                                 LoadDependentFiles load_dependent_files,
                                 const OptionGroupPlatform *platform_options,
                                 TargetSP &target_sp) {
-  std::lock_guard<std::recursive_mutex> guard(m_target_list_mutex);
+  // Create Target Internal does not modify any state
+  // directly and instead calls into methods which
+  // themselves are thread-safe. We need to do this so
+  // the locate module call back doesn't cause a re-entry
+  // dead lock when creating the target.
   auto result = TargetList::CreateTargetInternal(
       debugger, user_exe_path, triple_str, load_dependent_files,
       platform_options, target_sp);
 
+  std::lock_guard<std::recursive_mutex> guard(m_target_list_mutex);
   if (target_sp && result.Success())
     AddTargetInternal(target_sp, /*do_select*/ true);
   return result;
@@ -63,11 +68,16 @@ Status TargetList::CreateTarget(Debugger &debugger,
                                 const ArchSpec &specified_arch,
                                 LoadDependentFiles load_dependent_files,
                                 PlatformSP &platform_sp, TargetSP &target_sp) {
-  std::lock_guard<std::recursive_mutex> guard(m_target_list_mutex);
+  // Create Target Internal does not modify any state
+  // directly and instead calls into methods which
+  // themselves are thread-safe. We need to do this so
+  // the locate module call back doesn't cause a re-entry
+  // dead lock when creating the target.
   auto result = TargetList::CreateTargetInternal(
       debugger, user_exe_path, specified_arch, load_dependent_files,
       platform_sp, target_sp);
 
+  std::lock_guard<std::recursive_mutex> guard(m_target_list_mutex);
   if (target_sp && result.Success())
     AddTargetInternal(target_sp, /*do_select*/ true);
   return result;

>From e672344707103d279e58836e81486bae498ad608 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Mon, 17 Nov 2025 12:53:44 -0800
Subject: [PATCH 2/2] Move critical section into AddTargetInternal

---
 lldb/source/Target/TargetList.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lldb/source/Target/TargetList.cpp b/lldb/source/Target/TargetList.cpp
index 32d0be9a5cb06..907963011acfe 100644
--- a/lldb/source/Target/TargetList.cpp
+++ b/lldb/source/Target/TargetList.cpp
@@ -57,7 +57,6 @@ Status TargetList::CreateTarget(Debugger &debugger,
       debugger, user_exe_path, triple_str, load_dependent_files,
       platform_options, target_sp);
 
-  std::lock_guard<std::recursive_mutex> guard(m_target_list_mutex);
   if (target_sp && result.Success())
     AddTargetInternal(target_sp, /*do_select*/ true);
   return result;
@@ -77,7 +76,6 @@ Status TargetList::CreateTarget(Debugger &debugger,
       debugger, user_exe_path, specified_arch, load_dependent_files,
       platform_sp, target_sp);
 
-  std::lock_guard<std::recursive_mutex> guard(m_target_list_mutex);
   if (target_sp && result.Success())
     AddTargetInternal(target_sp, /*do_select*/ true);
   return result;
@@ -531,6 +529,7 @@ uint32_t TargetList::GetIndexOfTarget(lldb::TargetSP target_sp) const {
 }
 
 void TargetList::AddTargetInternal(TargetSP target_sp, bool do_select) {
+  std::lock_guard<std::recursive_mutex> guard(m_target_list_mutex);
   lldbassert(!llvm::is_contained(m_target_list, target_sp) &&
              "target already exists it the list");
   UnregisterInProcessTarget(target_sp);