https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/168425
>From 52f40b355d27273b46ace44a357a7ebdb2b837a3 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <[email protected]> Date: Mon, 17 Nov 2025 10:51:50 -0800 Subject: [PATCH 1/3] 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 905ce5ccb10f90f074519087a084a9459d1847fb Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <[email protected]> Date: Mon, 17 Nov 2025 12:53:44 -0800 Subject: [PATCH 2/3] 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); >From 89a63003ea978c4dbd6c1d49054005dd721dade3 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <[email protected]> Date: Tue, 2 Dec 2025 13:06:55 -0800 Subject: [PATCH 3/3] Move comments from the call-site to the method definition --- lldb/include/lldb/Target/TargetList.h | 5 +++++ lldb/source/Target/TargetList.cpp | 12 ++---------- 2 files changed, 7 insertions(+), 10 deletions(-) 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 907963011acfe..ce04e9c1209b8 100644 --- a/lldb/source/Target/TargetList.cpp +++ b/lldb/source/Target/TargetList.cpp @@ -48,11 +48,7 @@ Status TargetList::CreateTarget(Debugger &debugger, LoadDependentFiles load_dependent_files, const OptionGroupPlatform *platform_options, TargetSP &target_sp) { - // 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); @@ -67,11 +63,7 @@ Status TargetList::CreateTarget(Debugger &debugger, const ArchSpec &specified_arch, LoadDependentFiles load_dependent_files, PlatformSP &platform_sp, TargetSP &target_sp) { - // 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); _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
