Skip to content

Commit 36f6e5a

Browse files
Jlalondkcloudy0717
authored andcommitted
[LLDB] Fix deadlock in module callback when running in parallel (llvm#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.
1 parent a495c17 commit 36f6e5a

File tree

2 files changed

+8
-2
lines changed

2 files changed

+8
-2
lines changed

lldb/include/lldb/Target/TargetList.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,11 @@ class TargetList : public Broadcaster {
216216
llvm::StringRef triple_str, LoadDependentFiles load_dependent_files,
217217
const OptionGroupPlatform *platform_options, lldb::TargetSP &target_sp);
218218

219+
// Create Target Internal does not modify any state directly, and should not
220+
// be called under the target list mutex. Instead any state changes should
221+
// call into methods which themselves are protected by the target list mutex.
222+
// We need to do this so the locate module call back doesn't cause a re-entry
223+
// dead lock when creating the target.
219224
static Status CreateTargetInternal(Debugger &debugger,
220225
llvm::StringRef user_exe_path,
221226
const ArchSpec &arch,

lldb/source/Target/TargetList.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ Status TargetList::CreateTarget(Debugger &debugger,
4848
LoadDependentFiles load_dependent_files,
4949
const OptionGroupPlatform *platform_options,
5050
TargetSP &target_sp) {
51-
std::lock_guard<std::recursive_mutex> guard(m_target_list_mutex);
51+
5252
auto result = TargetList::CreateTargetInternal(
5353
debugger, user_exe_path, triple_str, load_dependent_files,
5454
platform_options, target_sp);
@@ -63,7 +63,7 @@ Status TargetList::CreateTarget(Debugger &debugger,
6363
const ArchSpec &specified_arch,
6464
LoadDependentFiles load_dependent_files,
6565
PlatformSP &platform_sp, TargetSP &target_sp) {
66-
std::lock_guard<std::recursive_mutex> guard(m_target_list_mutex);
66+
6767
auto result = TargetList::CreateTargetInternal(
6868
debugger, user_exe_path, specified_arch, load_dependent_files,
6969
platform_sp, target_sp);
@@ -521,6 +521,7 @@ uint32_t TargetList::GetIndexOfTarget(lldb::TargetSP target_sp) const {
521521
}
522522

523523
void TargetList::AddTargetInternal(TargetSP target_sp, bool do_select) {
524+
std::lock_guard<std::recursive_mutex> guard(m_target_list_mutex);
524525
lldbassert(!llvm::is_contained(m_target_list, target_sp) &&
525526
"target already exists it the list");
526527
UnregisterInProcessTarget(target_sp);

0 commit comments

Comments
 (0)