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

[via lldb-commits]

Author: Jacob Lalonde
Date: 2025-12-03T18:29:18Z
New Revision: 106edbdabef8bcd914ec1720f7fa6adb07aa4e6b

URL: https://github.com/llvm/llvm-project/commit/106edbdabef8bcd914ec1720f7fa6adb07aa4e6b
DIFF: https://github.com/llvm/llvm-project/commit/106edbdabef8bcd914ec1720f7fa6adb07aa4e6b.diff

LOG: [LLDB] Fix deadlock in module callback when running in parallel (#168425)

When the target is being created, the target list acquires the mutex for
the duration of the target creation process. However if a module
callback is enabled and is being called in parallel there exists an
opportunity to deadlock if the callback calls into targetlist. I've
created a minimum repro
[here](https://gist.github.com/Jlalond/2557e06fa09825f338eca08b1d21884f).

```
command script import dead-lock-example (from above gist)
...
target create a.out
[hangs]
```

This looks like a straight forward fix, where `CreateTargetInternal`
doesn't access any state directly, and instead calls methods which they
themselves are thread-safe. So I've moved the lock to when we update the
list with the created target. I'm not sure if this is a comprehensive
fix, but it does fix my above example and in my (albeit limited)
testing, doesn't cause any strange change in behavior.

Added: 
    

Modified: 
    lldb/include/lldb/Target/TargetList.h
    lldb/source/Target/TargetList.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Target/TargetList.h b/lldb/include/lldb/Target/TargetList.h
index 88272512bcc0f..d7ff639d0d2b6 100644
--- a/lldb/include/lldb/Target/TargetList.h
+++ b/lldb/include/lldb/Target/TargetList.h
@@ -216,6 +216,11 @@ class TargetList : public Broadcaster {
       llvm::StringRef triple_str, LoadDependentFiles load_dependent_files,
       const OptionGroupPlatform *platform_options, lldb::TargetSP &target_sp);
 
+  // Create Target Internal does not modify any state directly, and should not
+  // be called under the target list mutex. Instead any state changes should
+  // call into methods which themselves are protected by the target list mutex.
+  // We need to do this so the locate module call back doesn't cause a re-entry
+  // dead lock when creating the target.
   static Status CreateTargetInternal(Debugger &debugger,
                                      llvm::StringRef user_exe_path,
                                      const ArchSpec &arch,

diff  --git a/lldb/source/Target/TargetList.cpp b/lldb/source/Target/TargetList.cpp
index 2e03bc1e38ea0..ce04e9c1209b8 100644
--- a/lldb/source/Target/TargetList.cpp
+++ b/lldb/source/Target/TargetList.cpp
@@ -48,7 +48,7 @@ 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);
+
   auto result = TargetList::CreateTargetInternal(
       debugger, user_exe_path, triple_str, load_dependent_files,
       platform_options, target_sp);
@@ -63,7 +63,7 @@ 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);
+
   auto result = TargetList::CreateTargetInternal(
       debugger, user_exe_path, specified_arch, load_dependent_files,
       platform_sp, target_sp);
@@ -521,6 +521,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);