https://github.com/JDevlieghere updated https://github.com/llvm/llvm-project/pull/184452
>From b7e4e3ab97a2aee8076fa627daf766f1562bcebe Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <[email protected]> Date: Tue, 3 Mar 2026 14:57:56 -0800 Subject: [PATCH 1/2] [lldb] Make the PluginManager thread safe In #184273, John pointed out that the PluginManager is currently not thread safe. While we don't currently provide any guarantees, it seems desirable to be able to interact safely with the PluginManager from different threads. For example, we allow dynamically loading plugins from the command interpreter, which in theory could be coming from different threads. --- lldb/source/Core/PluginManager.cpp | 228 ++++++++++++++++++----------- 1 file changed, 143 insertions(+), 85 deletions(-) diff --git a/lldb/source/Core/PluginManager.cpp b/lldb/source/Core/PluginManager.cpp index e430305999b39..6b84227a6672a 100644 --- a/lldb/source/Core/PluginManager.cpp +++ b/lldb/source/Core/PluginManager.cpp @@ -491,6 +491,7 @@ template <typename Instance> class PluginInstances { if (!callback) return false; assert(!name.empty()); + std::lock_guard<std::mutex> lock(m_mutex); m_instances.emplace_back(name, description, callback, std::forward<Args>(args)...); return true; @@ -499,6 +500,7 @@ template <typename Instance> class PluginInstances { bool UnregisterPlugin(typename Instance::CallbackType callback) { if (!callback) return false; + std::lock_guard<std::mutex> lock(m_mutex); auto pos = m_instances.begin(); auto end = m_instances.end(); for (; pos != end; ++pos) { @@ -511,31 +513,61 @@ template <typename Instance> class PluginInstances { } typename Instance::CallbackType GetCallbackAtIndex(uint32_t idx) { - if (const Instance *instance = GetInstanceAtIndex(idx)) - return instance->create_callback; + std::lock_guard<std::mutex> lock(m_mutex); + uint32_t count = 0; + for (const auto &instance : m_instances) { + if (!instance.enabled) + continue; + if (count++ == idx) + return instance.create_callback; + } return nullptr; } llvm::StringRef GetDescriptionAtIndex(uint32_t idx) { - if (const Instance *instance = GetInstanceAtIndex(idx)) - return instance->description; + std::lock_guard<std::mutex> lock(m_mutex); + uint32_t count = 0; + for (const auto &instance : m_instances) { + if (!instance.enabled) + continue; + if (count++ == idx) + return instance.description; + } return ""; } llvm::StringRef GetNameAtIndex(uint32_t idx) { - if (const Instance *instance = GetInstanceAtIndex(idx)) - return instance->name; + std::lock_guard<std::mutex> lock(m_mutex); + uint32_t count = 0; + for (const auto &instance : m_instances) { + if (!instance.enabled) + continue; + if (count++ == idx) + return instance.name; + } return ""; } typename Instance::CallbackType GetCallbackForName(llvm::StringRef name) { - if (const Instance *instance = GetInstanceForName(name)) - return instance->create_callback; + if (name.empty()) + return nullptr; + std::lock_guard<std::mutex> lock(m_mutex); + for (const auto &instance : m_instances) { + if (!instance.enabled) + continue; + if (instance.name == name) + return instance.create_callback; + } return nullptr; } void PerformDebuggerCallback(Debugger &debugger) { - for (const auto &instance : m_instances) { + std::vector<Instance> snapshot; + { + std::lock_guard<std::mutex> lock(m_mutex); + snapshot = m_instances; + } + for (const auto &instance : snapshot) { if (!instance.enabled) continue; if (instance.debugger_init_callback) @@ -548,38 +580,43 @@ template <typename Instance> class PluginInstances { // to the returned instances will not be reflected back to instances // stored by the PluginInstances object. std::vector<Instance> GetSnapshot() { + std::lock_guard<std::mutex> lock(m_mutex); std::vector<Instance> enabled_instances; - for (const auto &instance : m_instances) { + for (const Instance &instance : m_instances) { if (instance.enabled) enabled_instances.push_back(instance); } return enabled_instances; } - const Instance *GetInstanceAtIndex(uint32_t idx) { + // Return a field from the idx-th enabled instance, or default_val if none. + template <typename Ret, typename Callback> + Ret GetFieldAtIndex(uint32_t idx, Ret default_val, Callback callback) { + std::lock_guard<std::mutex> lock(m_mutex); uint32_t count = 0; - - return FindEnabledInstance( - [&](const Instance &instance) { return count++ == idx; }); + for (const Instance &instance : m_instances) { + if (!instance.enabled) + continue; + if (count++ == idx) + return callback(instance); + } + return default_val; } - const Instance *GetInstanceForName(llvm::StringRef name) { + // Return a field from the enabled instance with the given name, or + // default_val if none. + template <typename Ret, typename Callback> + Ret GetFieldForName(llvm::StringRef name, Ret default_val, Callback callback) { if (name.empty()) - return nullptr; - - return FindEnabledInstance( - [&](const Instance &instance) { return instance.name == name; }); - } - - const Instance * - FindEnabledInstance(std::function<bool(const Instance &)> predicate) const { - for (const auto &instance : m_instances) { + return default_val; + std::lock_guard<std::mutex> lock(m_mutex); + for (const Instance &instance : m_instances) { if (!instance.enabled) continue; - if (predicate(instance)) - return &instance; + if (instance.name == name) + return callback(instance); } - return nullptr; + return default_val; } // Return a list of all the registered plugin instances. This includes both @@ -587,6 +624,7 @@ template <typename Instance> class PluginInstances { // were registered which is the order they would be queried if they were all // enabled. std::vector<RegisteredPluginInfo> GetPluginInfoForAllInstances() { + std::lock_guard<std::mutex> lock(m_mutex); // Lookup the plugin info for each instance in the sorted order. std::vector<RegisteredPluginInfo> plugin_infos; plugin_infos.reserve(m_instances.size()); @@ -598,6 +636,7 @@ template <typename Instance> class PluginInstances { } bool SetInstanceEnabled(llvm::StringRef name, bool enable) { + std::lock_guard<std::mutex> lock(m_mutex); auto it = llvm::find_if(m_instances, [&](const Instance &instance) { return instance.name == name; }); @@ -610,7 +649,32 @@ template <typename Instance> class PluginInstances { } private: + const Instance *GetInstanceAtIndexLocked(uint32_t idx) { + uint32_t count = 0; + return FindEnabledInstanceLocked( + [&](const Instance &instance) { return count++ == idx; }); + } + + const Instance *GetInstanceForNameLocked(llvm::StringRef name) { + if (name.empty()) + return nullptr; + return FindEnabledInstanceLocked( + [&](const Instance &instance) { return instance.name == name; }); + } + + const Instance *FindEnabledInstanceLocked( + std::function<bool(const Instance &)> predicate) const { + for (const auto &instance : m_instances) { + if (!instance.enabled) + continue; + if (predicate(instance)) + return &instance; + } + return nullptr; + } + std::vector<Instance> m_instances; + mutable std::mutex m_mutex; }; #pragma mark ABI @@ -905,16 +969,16 @@ PluginManager::GetLanguageRuntimeCreateCallbackAtIndex(uint32_t idx) { LanguageRuntimeGetCommandObject PluginManager::GetLanguageRuntimeGetCommandObjectAtIndex(uint32_t idx) { - if (auto instance = GetLanguageRuntimeInstances().GetInstanceAtIndex(idx)) - return instance->command_callback; - return nullptr; + return GetLanguageRuntimeInstances().GetFieldAtIndex( + idx, (LanguageRuntimeGetCommandObject) nullptr, + [](const auto &i) { return i.command_callback; }); } LanguageRuntimeGetExceptionPrecondition PluginManager::GetLanguageRuntimeGetExceptionPreconditionAtIndex(uint32_t idx) { - if (auto instance = GetLanguageRuntimeInstances().GetInstanceAtIndex(idx)) - return instance->precondition_callback; - return nullptr; + return GetLanguageRuntimeInstances().GetFieldAtIndex( + idx, (LanguageRuntimeGetExceptionPrecondition) nullptr, + [](const auto &i) { return i.precondition_callback; }); } #pragma mark SystemRuntime @@ -975,7 +1039,8 @@ bool PluginManager::IsRegisteredObjectFilePluginName(llvm::StringRef name) { if (name.empty()) return false; - return GetObjectFileInstances().GetInstanceForName(name) != nullptr; + return GetObjectFileInstances().GetFieldForName( + name, false, [](const auto &) { return true; }); } bool PluginManager::RegisterPlugin( @@ -1001,25 +1066,25 @@ PluginManager::GetObjectFileCreateCallbackAtIndex(uint32_t idx) { ObjectFileCreateMemoryInstance PluginManager::GetObjectFileCreateMemoryCallbackAtIndex(uint32_t idx) { - if (auto instance = GetObjectFileInstances().GetInstanceAtIndex(idx)) - return instance->create_memory_callback; - return nullptr; + return GetObjectFileInstances().GetFieldAtIndex( + idx, (ObjectFileCreateMemoryInstance) nullptr, + [](const auto &i) { return i.create_memory_callback; }); } ObjectFileGetModuleSpecifications PluginManager::GetObjectFileGetModuleSpecificationsCallbackAtIndex( uint32_t idx) { - if (auto instance = GetObjectFileInstances().GetInstanceAtIndex(idx)) - return instance->get_module_specifications; - return nullptr; + return GetObjectFileInstances().GetFieldAtIndex( + idx, (ObjectFileGetModuleSpecifications) nullptr, + [](const auto &i) { return i.get_module_specifications; }); } ObjectFileCreateMemoryInstance PluginManager::GetObjectFileCreateMemoryCallbackForPluginName( llvm::StringRef name) { - if (auto instance = GetObjectFileInstances().GetInstanceForName(name)) - return instance->create_memory_callback; - return nullptr; + return GetObjectFileInstances().GetFieldForName( + name, (ObjectFileCreateMemoryInstance) nullptr, + [](const auto &i) { return i.create_memory_callback; }); } Status PluginManager::SaveCore(lldb_private::SaveCoreOptions &options) { @@ -1132,17 +1197,17 @@ PluginManager::GetObjectContainerCreateCallbackAtIndex(uint32_t idx) { ObjectContainerCreateMemoryInstance PluginManager::GetObjectContainerCreateMemoryCallbackAtIndex(uint32_t idx) { - if (auto instance = GetObjectContainerInstances().GetInstanceAtIndex(idx)) - return instance->create_memory_callback; - return nullptr; + return GetObjectContainerInstances().GetFieldAtIndex( + idx, (ObjectContainerCreateMemoryInstance) nullptr, + [](const auto &i) { return i.create_memory_callback; }); } ObjectFileGetModuleSpecifications PluginManager::GetObjectContainerGetModuleSpecificationsCallbackAtIndex( uint32_t idx) { - if (auto instance = GetObjectContainerInstances().GetInstanceAtIndex(idx)) - return instance->get_module_specifications; - return nullptr; + return GetObjectContainerInstances().GetFieldAtIndex( + idx, (ObjectFileGetModuleSpecifications) nullptr, + [](const auto &i) { return i.get_module_specifications; }); } #pragma mark Platform @@ -1309,9 +1374,9 @@ lldb::RegisterTypeBuilderSP PluginManager::GetRegisterTypeBuilder(Target &target) { // We assume that RegisterTypeBuilderClang is the only instance of this plugin // type and is always present. - auto instance = GetRegisterTypeBuilderInstances().GetInstanceAtIndex(0); - assert(instance); - return instance->create_callback(target); + auto cb = GetRegisterTypeBuilderInstances().GetCallbackAtIndex(0); + assert(cb); + return cb(target); } #pragma mark ScriptInterpreter @@ -1486,13 +1551,13 @@ PluginManager::GetStructuredDataPluginCreateCallbackAtIndex(uint32_t idx) { StructuredDataFilterLaunchInfo PluginManager::GetStructuredDataFilterCallbackAtIndex( uint32_t idx, bool &iteration_complete) { - if (auto instance = - GetStructuredDataPluginInstances().GetInstanceAtIndex(idx)) { - iteration_complete = false; - return instance->filter_callback; - } iteration_complete = true; - return nullptr; + return GetStructuredDataPluginInstances() + .GetFieldAtIndex<StructuredDataFilterLaunchInfo>( + idx, nullptr, [&iteration_complete](const auto &i) { + iteration_complete = false; + return i.filter_callback; + }); } #pragma mark SymbolFile @@ -1724,22 +1789,19 @@ PluginManager::GetTraceCreateCallback(llvm::StringRef plugin_name) { TraceCreateInstanceForLiveProcess PluginManager::GetTraceCreateCallbackForLiveProcess( llvm::StringRef plugin_name) { - if (auto instance = GetTracePluginInstances().GetInstanceForName(plugin_name)) - return instance->create_callback_for_live_process; - - return nullptr; + return GetTracePluginInstances().GetFieldForName( + plugin_name, (TraceCreateInstanceForLiveProcess) nullptr, + [](const auto &i) { return i.create_callback_for_live_process; }); } llvm::StringRef PluginManager::GetTraceSchema(llvm::StringRef plugin_name) { - if (auto instance = GetTracePluginInstances().GetInstanceForName(plugin_name)) - return instance->schema; - return llvm::StringRef(); + return GetTracePluginInstances().GetFieldForName( + plugin_name, llvm::StringRef(), [](const auto &i) { return i.schema; }); } llvm::StringRef PluginManager::GetTraceSchema(size_t index) { - if (const TraceInstance *instance = - GetTracePluginInstances().GetInstanceAtIndex(index)) - return instance->schema; + return GetTracePluginInstances().GetFieldAtIndex( + index, llvm::StringRef(), [](const auto &i) { return i.schema; }); return llvm::StringRef(); } @@ -1786,10 +1848,9 @@ bool PluginManager::UnregisterPlugin( ThreadTraceExportCommandCreator PluginManager::GetThreadTraceExportCommandCreatorAtIndex(uint32_t index) { - if (const TraceExporterInstance *instance = - GetTraceExporterInstances().GetInstanceAtIndex(index)) - return instance->create_thread_trace_export_command; - return nullptr; + return GetTraceExporterInstances().GetFieldAtIndex( + index, (ThreadTraceExportCommandCreator) nullptr, + [](const auto &i) { return i.create_thread_trace_export_command; }); } llvm::StringRef @@ -1889,10 +1950,9 @@ bool PluginManager::UnregisterPlugin( InstrumentationRuntimeGetType PluginManager::GetInstrumentationRuntimeGetTypeCallbackAtIndex(uint32_t idx) { - if (auto instance = - GetInstrumentationRuntimeInstances().GetInstanceAtIndex(idx)) - return instance->get_type_callback; - return nullptr; + return GetInstrumentationRuntimeInstances().GetFieldAtIndex( + idx, (InstrumentationRuntimeGetType) nullptr, + [](const auto &i) { return i.get_type_callback; }); } InstrumentationRuntimeCreateInstance @@ -2010,16 +2070,15 @@ PluginManager::GetScriptedInterfaceDescriptionAtIndex(uint32_t index) { lldb::ScriptLanguage PluginManager::GetScriptedInterfaceLanguageAtIndex(uint32_t idx) { - if (auto instance = GetScriptedInterfaceInstances().GetInstanceAtIndex(idx)) - return instance->language; - return ScriptLanguage::eScriptLanguageNone; + return GetScriptedInterfaceInstances().GetFieldAtIndex( + idx, ScriptLanguage::eScriptLanguageNone, + [](const auto &i) { return i.language; }); } ScriptedInterfaceUsages PluginManager::GetScriptedInterfaceUsagesAtIndex(uint32_t idx) { - if (auto instance = GetScriptedInterfaceInstances().GetInstanceAtIndex(idx)) - return instance->usages; - return {}; + return GetScriptedInterfaceInstances().GetFieldAtIndex( + idx, ScriptedInterfaceUsages{}, [](const auto &i) { return i.usages; }); } #pragma mark REPL @@ -2057,9 +2116,8 @@ REPLCreateInstance PluginManager::GetREPLCreateCallbackAtIndex(uint32_t idx) { } LanguageSet PluginManager::GetREPLSupportedLanguagesAtIndex(uint32_t idx) { - if (auto instance = GetREPLInstances().GetInstanceAtIndex(idx)) - return instance->supported_languages; - return LanguageSet(); + return GetREPLInstances().GetFieldAtIndex( + idx, LanguageSet{}, [](const auto &i) { return i.supported_languages; }); } LanguageSet PluginManager::GetREPLAllTypeSystemSupportedLanguages() { >From d17f5a7c3d62437bc53a75d212d15b38f54f5d22 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <[email protected]> Date: Tue, 3 Mar 2026 15:09:50 -0800 Subject: [PATCH 2/2] Appease the formatter --- lldb/source/Core/PluginManager.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lldb/source/Core/PluginManager.cpp b/lldb/source/Core/PluginManager.cpp index 6b84227a6672a..abbdd62b95cca 100644 --- a/lldb/source/Core/PluginManager.cpp +++ b/lldb/source/Core/PluginManager.cpp @@ -606,7 +606,8 @@ template <typename Instance> class PluginInstances { // Return a field from the enabled instance with the given name, or // default_val if none. template <typename Ret, typename Callback> - Ret GetFieldForName(llvm::StringRef name, Ret default_val, Callback callback) { + Ret GetFieldForName(llvm::StringRef name, Ret default_val, + Callback callback) { if (name.empty()) return default_val; std::lock_guard<std::mutex> lock(m_mutex); _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
