fix(plugins): prevent callback races

Lifecycle callbacks can be registered while
another thread dispatches them, risking races
and iterator invalidation.

Snapshot shared callback objects under a mutex,
then invoke them unlocked to preserve reentrancy
and mutable callback state.

src/slic3r/CMakeLists.txt

@@ -616,6 +616,7 @@ set(SLIC3R_GUI_SOURCES
     plugin/PluginFsUtils.hpp
     plugin/PluginCatalog.cpp
     plugin/PluginCatalog.hpp
+    plugin/PluginCallbackList.hpp
     plugin/PluginLoader.cpp
     plugin/PluginLoader.hpp
     plugin/PluginDescriptor.hpp

src/slic3r/plugin/PluginCallbackList.hpp

@@ -0,0 +1,44 @@
+#pragma once
+
+#include <functional>
+#include <memory>
+#include <mutex>
+#include <utility>
+#include <vector>
+
+namespace Slic3r {
+
+// Thread-safe append-only callback storage. Each dispatch operates on a snapshot,
+// so callbacks may safely subscribe more callbacks without deadlocking or changing
+// the in-flight dispatch.
+template<typename Signature> class PluginCallbackList
+{
+public:
+    using Callback = std::function<Signature>;
+    using CallbackPtr = std::shared_ptr<Callback>;
+
+    void subscribe(Callback callback)
+    {
+        auto owned_callback = std::make_shared<Callback>(std::move(callback));
+        std::lock_guard<std::mutex> lock(m_mutex);
+        m_callbacks.emplace_back(std::move(owned_callback));
+    }
+
+    template<typename Dispatcher> void dispatch(Dispatcher&& dispatcher) const
+    {
+        std::vector<CallbackPtr> callbacks;
+        {
+            std::lock_guard<std::mutex> lock(m_mutex);
+            callbacks = m_callbacks;
+        }
+
+        for (const CallbackPtr& callback : callbacks)
+            dispatcher(*callback);
+    }
+
+private:
+    mutable std::mutex       m_mutex;
+    std::vector<CallbackPtr> m_callbacks;
+};
+
+} // namespace Slic3r

src/slic3r/plugin/PluginLoader.cpp

@@ -1429,7 +1429,7 @@ void PluginLoader::unload_cloud_plugins()
 
 void PluginLoader::run_on_load_callbacks(const std::string& plugin_key)
 {
-    for (auto& fn : m_callbacks[CallbackType::Load]) {
+    m_load_callbacks.dispatch([&plugin_key](const PluginLifecycleCompleteFn& fn) {
         try {
             fn(plugin_key);
         } catch (const std::exception& ex) {
@@ -1437,12 +1437,12 @@ void PluginLoader::run_on_load_callbacks(const std::string& plugin_key)
         } catch (...) {
             BOOST_LOG_TRIVIAL(error) << "Plugin load completion callback failed for " << plugin_key;
         }
-    }
+    });
 }
 
 void PluginLoader::run_on_unload_callbacks(const std::string& plugin_key)
 {
-    for (auto& fn : m_callbacks[CallbackType::Unload]) {
+    m_unload_callbacks.dispatch([&plugin_key](const PluginLifecycleCompleteFn& fn) {
         try {
             fn(plugin_key);
         } catch (const std::exception& ex) {
@@ -1450,12 +1450,12 @@ void PluginLoader::run_on_unload_callbacks(const std::string& plugin_key)
         } catch (...) {
             BOOST_LOG_TRIVIAL(error) << "Plugin unload completion callback failed for " << plugin_key << ": unknown error";
         }
-    }
+    });
 }
 
 void PluginLoader::run_on_capability_load_callbacks(const PluginCapabilityIdentifier& id)
 {
-    for (auto& fn : m_capability_callbacks[CallbackType::Load]) {
+    m_capability_load_callbacks.dispatch([&id](const CapabilityLifecycleFn& fn) {
         try {
             fn(id);
         } catch (const std::exception& ex) {
@@ -1465,12 +1465,12 @@ void PluginLoader::run_on_capability_load_callbacks(const PluginCapabilityIdenti
             BOOST_LOG_TRIVIAL(error) << "Plugin capability load callback failed for " << id.plugin_key << "/"
                                      << id.name << ": unknown error";
         }
-    }
+    });
 }
 
 void PluginLoader::run_on_capability_unload_callbacks(const PluginCapabilityIdentifier& id)
 {
-    for (auto& fn : m_capability_callbacks[CallbackType::Unload]) {
+    m_capability_unload_callbacks.dispatch([&id](const CapabilityLifecycleFn& fn) {
         try {
             fn(id);
         } catch (const std::exception& ex) {
@@ -1480,27 +1480,27 @@ void PluginLoader::run_on_capability_unload_callbacks(const PluginCapabilityIden
             BOOST_LOG_TRIVIAL(error) << "Plugin capability unload callback failed for " << id.plugin_key << "/"
                                      << id.name << ": unknown error";
         }
-    }
+    });
 }
 
 void PluginLoader::subscribe_on_load_callback(PluginLifecycleCompleteFn fn)
 {
-    m_callbacks[CallbackType::Load].push_back(std::move(fn));
+    m_load_callbacks.subscribe(std::move(fn));
 }
 
 void PluginLoader::subscribe_on_unload_callback(PluginLifecycleCompleteFn fn)
 {
-    m_callbacks[CallbackType::Unload].push_back(std::move(fn));
+    m_unload_callbacks.subscribe(std::move(fn));
 }
 
 void PluginLoader::subscribe_on_capability_load_callback(CapabilityLifecycleFn fn)
 {
-    m_capability_callbacks[CallbackType::Load].push_back(std::move(fn));
+    m_capability_load_callbacks.subscribe(std::move(fn));
 }
 
 void PluginLoader::subscribe_on_capability_unload_callback(CapabilityLifecycleFn fn)
 {
-    m_capability_callbacks[CallbackType::Unload].push_back(std::move(fn));
+    m_capability_unload_callbacks.subscribe(std::move(fn));
 }
 
 } // namespace Slic3r

src/slic3r/plugin/PluginLoader.hpp

@@ -1,5 +1,6 @@
 #pragma once
 
+#include "PluginCallbackList.hpp"
 #include "PluginDescriptor.hpp"
 
 #include <boost/filesystem/path.hpp>
@@ -197,8 +198,10 @@ private:
     mutable std::mutex m_mutex;
     mutable std::condition_variable m_plugin_load_cv;
 
-    std::unordered_map<CallbackType, std::vector<PluginLifecycleCompleteFn>> m_callbacks{};
-    std::unordered_map<CallbackType, std::vector<CapabilityLifecycleFn>> m_capability_callbacks{};
+    PluginCallbackList<void(const std::string&)>                m_load_callbacks;
+    PluginCallbackList<void(const std::string&)>                m_unload_callbacks;
+    PluginCallbackList<void(const PluginCapabilityIdentifier&)> m_capability_load_callbacks;
+    PluginCallbackList<void(const PluginCapabilityIdentifier&)> m_capability_unload_callbacks;
 
     /*
         callbacks:

tests/slic3rutils/CMakeLists.txt

@@ -3,6 +3,7 @@ add_executable(${_TEST_NAME}_tests
     ${_TEST_NAME}_tests_main.cpp
     test_action_source.cpp
     test_plugin_host_api.cpp
+    test_plugin_callback_list.cpp
     test_plugin_capability_identifier.cpp
     test_plugin_install.cpp
     test_plugin_sort.cpp

tests/slic3rutils/test_plugin_callback_list.cpp

@@ -0,0 +1,105 @@
+#include <catch2/catch_all.hpp>
+
+#include <slic3r/plugin/PluginCallbackList.hpp>
+
+#include <atomic>
+#include <chrono>
+#include <condition_variable>
+#include <future>
+#include <mutex>
+#include <string>
+#include <vector>
+
+using Slic3r::PluginCallbackList;
+
+TEST_CASE("PluginCallbackList dispatches subscribers in subscription order", "[plugin][callback-list]")
+{
+    PluginCallbackList<void()> callbacks;
+    std::vector<int>            calls;
+
+    callbacks.subscribe([&calls] { calls.push_back(1); });
+    callbacks.subscribe([&calls] { calls.push_back(2); });
+
+    callbacks.dispatch([](const auto& callback) { callback(); });
+
+    CHECK(calls == std::vector<int>{1, 2});
+}
+
+TEST_CASE("PluginCallbackList defers subscribers added during dispatch", "[plugin][callback-list]")
+{
+    PluginCallbackList<void()> callbacks;
+    std::vector<std::string>   calls;
+
+    callbacks.subscribe([&] {
+        calls.emplace_back("original");
+        callbacks.subscribe([&calls] { calls.emplace_back("added"); });
+    });
+
+    callbacks.dispatch([](const auto& callback) { callback(); });
+    CHECK(calls == std::vector<std::string>{"original"});
+
+    callbacks.dispatch([](const auto& callback) { callback(); });
+    CHECK(calls == std::vector<std::string>{"original", "original", "added"});
+}
+
+TEST_CASE("PluginCallbackList preserves mutable callback state across dispatches", "[plugin][callback-list]")
+{
+    PluginCallbackList<void()> callbacks;
+    std::vector<int>            calls;
+
+    callbacks.subscribe([count = 0, &calls]() mutable { calls.push_back(++count); });
+
+    callbacks.dispatch([](const auto& callback) { callback(); });
+    callbacks.dispatch([](const auto& callback) { callback(); });
+
+    CHECK(calls == std::vector<int>{1, 2});
+}
+
+TEST_CASE("PluginCallbackList does not hold its mutex while invoking callbacks", "[plugin][callback-list]")
+{
+    using namespace std::chrono_literals;
+
+    PluginCallbackList<void()> callbacks;
+    std::mutex                 mutex;
+    std::condition_variable    callback_entered_cv;
+    std::condition_variable    release_callback_cv;
+    bool                       callback_entered = false;
+    bool                       release_callback = false;
+    std::atomic<int>           late_calls{0};
+
+    callbacks.subscribe([&] {
+        std::unique_lock<std::mutex> lock(mutex);
+        callback_entered = true;
+        callback_entered_cv.notify_one();
+        release_callback_cv.wait(lock, [&release_callback] { return release_callback; });
+    });
+
+    auto dispatch = std::async(std::launch::async, [&] {
+        callbacks.dispatch([](const auto& callback) { callback(); });
+    });
+
+    {
+        std::unique_lock<std::mutex> lock(mutex);
+        callback_entered_cv.wait_for(lock, 5s, [&callback_entered] { return callback_entered; });
+    }
+
+    auto subscribe = std::async(std::launch::async, [&] {
+        callbacks.subscribe([&late_calls] { ++late_calls; });
+    });
+    const bool subscribed_while_callback_blocked = subscribe.wait_for(5s) == std::future_status::ready;
+
+    {
+        std::lock_guard<std::mutex> lock(mutex);
+        release_callback = true;
+    }
+    release_callback_cv.notify_one();
+
+    dispatch.get();
+    subscribe.get();
+    CHECK(callback_entered);
+    CHECK(subscribed_while_callback_blocked);
+    CHECK(late_calls.load() == 0);
+
+    callbacks.dispatch([](const auto& callback) { callback(); });
+    CHECK(late_calls.load() == 1);
+}