Skip to content

Commit 6b0ca19

Browse files
committed
Fix second-pass Tier 1 findings
1 parent c899766 commit 6b0ca19

3 files changed

Lines changed: 29 additions & 20 deletions

File tree

plugin_arch/PluginManager.hpp

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,13 @@ class PluginManager {
388388
LoadPolicy policy = LoadPolicy::strict) {
389389
PluginRegistry registry;
390390
(void)registry.scan(directory);
391+
load_all_parallel(registry, config_map, policy);
392+
}
393+
394+
// Parallel load from a pre-populated registry.
395+
void load_all_parallel(PluginRegistry& registry,
396+
const ConfigMap& config_map = {},
397+
LoadPolicy policy = LoadPolicy::strict) {
391398
load_errors_.clear();
392399

393400
auto [infos, levels] = discover_and_sort(registry, policy, &load_errors_);
@@ -464,7 +471,6 @@ class PluginManager {
464471
infos[result.info_idx].deps});
465472
rebuild_name_index();
466473
wire_instance_tracked(plugins_.back(), config_map);
467-
468474
}
469475
}
470476
}
@@ -568,7 +574,7 @@ class PluginManager {
568574
{std::move(instance), std::nullopt, entry, std::move(deps)});
569575
rebuild_name_index();
570576
wire_instance_tracked(plugins_.back(), config_map, /*skip_validation=*/true);
571-
notify_loaded(entry.name, entry.type, entry);
577+
notify_loaded(plugins_.back().entry);
572578

573579
}
574580

@@ -593,12 +599,12 @@ class PluginManager {
593599
// Shutdown reverse dependents
594600
for (std::size_t idx : dep_indices) {
595601
shutdown_plugin(plugins_[idx]);
596-
notify_unloaded(plugins_[idx].entry.name, plugins_[idx].entry.type, plugins_[idx].entry);
602+
notify_unloaded(plugins_[idx].entry);
597603
}
598604

599605
// Shutdown the target
600606
shutdown_plugin(plugins_[it->second]);
601-
notify_unloaded(name, target_type, plugins_[it->second].entry);
607+
notify_unloaded(plugins_[it->second].entry);
602608

603609
// Collect all indices to remove
604610
std::unordered_set<std::size_t> to_remove(dep_indices.begin(),
@@ -688,7 +694,7 @@ class PluginManager {
688694
wire_instance_tracked(plugins_[idx], config_map);
689695
}
690696

691-
notify_reloaded(name, target_type, target.entry);
697+
notify_reloaded(target.entry);
692698

693699
}
694700

@@ -754,7 +760,7 @@ class PluginManager {
754760
lp.enabled = true;
755761
rebuild_locator();
756762
wire_instance_tracked(lp, config_map);
757-
notify_enabled(lp.entry.name, lp.entry.type, lp.entry);
763+
notify_enabled(lp.entry);
758764
}
759765

760766
// Check if a plugin is enabled.
@@ -961,20 +967,20 @@ class PluginManager {
961967

962968
// --- Observer notification ---
963969

964-
void notify_loaded(const std::string& name, const std::string& type, const PluginEntry& entry) {
965-
for (auto* obs : observers_) { try { obs->on_plugin_loaded(name, type, entry); } catch (...) {} }
970+
void notify_loaded(const PluginEntry& entry) {
971+
for (auto* obs : observers_) { try { obs->on_plugin_loaded(entry.name, entry.type, entry); } catch (...) {} }
966972
}
967-
void notify_unloaded(const std::string& name, const std::string& type, const PluginEntry& entry) {
968-
for (auto* obs : observers_) { try { obs->on_plugin_unloaded(name, type, entry); } catch (...) {} }
973+
void notify_unloaded(const PluginEntry& entry) {
974+
for (auto* obs : observers_) { try { obs->on_plugin_unloaded(entry.name, entry.type, entry); } catch (...) {} }
969975
}
970-
void notify_reloaded(const std::string& name, const std::string& type, const PluginEntry& entry) {
971-
for (auto* obs : observers_) { try { obs->on_plugin_reloaded(name, type, entry); } catch (...) {} }
976+
void notify_reloaded(const PluginEntry& entry) {
977+
for (auto* obs : observers_) { try { obs->on_plugin_reloaded(entry.name, entry.type, entry); } catch (...) {} }
972978
}
973-
void notify_enabled(const std::string& name, const std::string& type, const PluginEntry& entry) {
974-
for (auto* obs : observers_) { try { obs->on_plugin_enabled(name, type, entry); } catch (...) {} }
979+
void notify_enabled(const PluginEntry& entry) {
980+
for (auto* obs : observers_) { try { obs->on_plugin_enabled(entry.name, entry.type, entry); } catch (...) {} }
975981
}
976-
void notify_disabled(const std::string& name, const std::string& type, const PluginEntry& entry) {
977-
for (auto* obs : observers_) { try { obs->on_plugin_disabled(name, type, entry); } catch (...) {} }
982+
void notify_disabled(const PluginEntry& entry) {
983+
for (auto* obs : observers_) { try { obs->on_plugin_disabled(entry.name, entry.type, entry); } catch (...) {} }
978984
}
979985

980986
// --- Name index ---
@@ -1008,7 +1014,7 @@ class PluginManager {
10081014
}
10091015
lp.subscription_ids.clear();
10101016
lp.enabled = false;
1011-
notify_disabled(lp.entry.name, lp.entry.type, lp.entry);
1017+
notify_disabled(lp.entry);
10121018
}
10131019

10141020
// --- Reverse dependency graph ---
@@ -1204,6 +1210,8 @@ class PluginManager {
12041210

12051211
protected:
12061212
// --- Extensibility points (override in subclasses) ---
1213+
// Subclasses that override these SHOULD call the base implementation to
1214+
// preserve built-in mixin wiring, lifecycle hooks, and locator registration.
12071215

12081216
// Wire all opt-in mixins on an instance.
12091217
// Order: validate+configure → inject services → inject events → custom wirers → init.
@@ -1271,7 +1279,7 @@ class PluginManager {
12711279
{std::move(instance), std::move(loader), entry, deps});
12721280
rebuild_name_index();
12731281
wire_instance_tracked(plugins_.back(), config_map);
1274-
notify_loaded(entry.name, entry.type, entry);
1282+
notify_loaded(entry);
12751283
}
12761284
};
12771285

plugin_arch/PluginObserver.hpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,9 @@
1010

1111
#include <string>
1212

13-
#include "PluginRegistry.hpp"
14-
1513
namespace plugin_arch {
1614

15+
struct PluginEntry;
1716
class PluginManager;
1817

1918
class PluginObserver {

plugin_arch/ServiceLocator.hpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
#include <cstddef>
1212
#include <memory>
13+
#include <stdexcept>
1314
#include <string>
1415
#include <vector>
1516

@@ -39,6 +40,7 @@ class ServiceLocator {
3940
// per type is expected.
4041
void add_unique(const std::shared_ptr<IPlugin>& service) {
4142
if (!service) return;
43+
cleanup(); // prune expired entries before checking for duplicates
4244
for (const auto& weak_svc : services_) {
4345
auto svc = weak_svc.lock();
4446
if (svc && svc->type() == service->type()) {

0 commit comments

Comments
 (0)