llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Jacob Lalonde (Jlalond) <details> <summary>Changes</summary> 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). 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. --- Full diff: https://github.com/llvm/llvm-project/pull/168425.diff 1 Files Affected: - (modified) lldb/source/Target/TargetList.cpp (+12-2) ``````````diff diff --git a/lldb/source/Target/TargetList.cpp b/lldb/source/Target/TargetList.cpp index 2e03bc1e38ea0..af0d24d7b1d0a 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 and state + // directly and instead calls into methods which + // they themselves are thread-safe. We do this so + // the load 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 and state + // directly and instead calls into methods which + // they themselves are thread-safe. We do this so + // the load 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; `````````` </details> https://github.com/llvm/llvm-project/pull/168425 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
