diff --git a/src/libslic3r/LifecycleEvents.hpp b/src/libslic3r/LifecycleEvents.hpp index 9954836b3e..9ceaa99f30 100644 --- a/src/libslic3r/LifecycleEvents.hpp +++ b/src/libslic3r/LifecycleEvents.hpp @@ -8,6 +8,9 @@ // the slicing engine itself; see fire_lifecycle_event() below. #include +#include +#include +#include #include #include @@ -165,17 +168,79 @@ namespace Slic3r // require callers to hold a Print& just to report an event. using LifecycleHookFn = std::function; - inline LifecycleHookFn& lifecycle_hook_fn() + namespace detail { + + struct LifecycleHookState { - static LifecycleHookFn fn; - return fn; + std::mutex mutex; + std::condition_variable cv; + LifecycleHookFn fn; + std::size_t active_dispatches = 0; + bool accepting = false; + }; + + inline LifecycleHookState& lifecycle_hook_state() + { + static LifecycleHookState state; + return state; } - inline void set_lifecycle_hook_fn(LifecycleHookFn fn) { lifecycle_hook_fn() = std::move(fn); } + class LifecycleDispatchGuard + { + public: + explicit LifecycleDispatchGuard(LifecycleHookState& state) : m_state(state) {} + + ~LifecycleDispatchGuard() + { + std::lock_guard lock(m_state.mutex); + --m_state.active_dispatches; + if (m_state.active_dispatches == 0) + m_state.cv.notify_all(); + } + + LifecycleDispatchGuard(const LifecycleDispatchGuard&) = delete; + LifecycleDispatchGuard& operator=(const LifecycleDispatchGuard&) = delete; + + private: + LifecycleHookState& m_state; + }; + + } // namespace detail + + // Installing a hook starts accepting dispatches. Passing an empty function stops accepting + // new dispatches, detaches the hook, and waits for callbacks already in progress to finish. + // This is used during plugin shutdown so plugin code cannot be unloaded while a lifecycle + // callback is still executing. The empty-function path must not be called from inside the + // lifecycle callback itself. + inline void set_lifecycle_hook_fn(LifecycleHookFn fn) + { + detail::LifecycleHookState& state = detail::lifecycle_hook_state(); + if (fn) { + std::lock_guard lock(state.mutex); + state.fn = std::move(fn); + state.accepting = true; + return; + } + + std::unique_lock lock(state.mutex); + state.accepting = false; + state.fn = nullptr; + state.cv.wait(lock, [&state] { return state.active_dispatches == 0; }); + } inline void fire_lifecycle_event(LifecycleEvent event, const LifecycleEventContext& ctx) { - if (const LifecycleHookFn& fn = lifecycle_hook_fn(); fn) - fn(event, ctx); + detail::LifecycleHookState& state = detail::lifecycle_hook_state(); + LifecycleHookFn fn; + { + std::lock_guard lock(state.mutex); + if (!state.accepting || !state.fn) + return; + fn = state.fn; + ++state.active_dispatches; + } + + detail::LifecycleDispatchGuard guard(state); + fn(event, ctx); } } diff --git a/src/slic3r/plugin/PluginHooks.hpp b/src/slic3r/plugin/PluginHooks.hpp index bff276c5df..5eeacfa449 100644 --- a/src/slic3r/plugin/PluginHooks.hpp +++ b/src/slic3r/plugin/PluginHooks.hpp @@ -14,8 +14,9 @@ namespace Slic3r::plugin_hooks { void install(); // Reset every hook to null so none can enter Python after the interpreter -// finalizes. Called from PluginManager::shutdown(); callers must have stopped -// background slicing first (resetting a hook while process() runs is a race). +// finalizes. The lifecycle-event hook drains callbacks already in progress +// before returning. Other hooks retain their existing caller-side shutdown +// requirements. void uninstall(); } // namespace Slic3r::plugin_hooks diff --git a/src/slic3r/plugin/PluginManager.cpp b/src/slic3r/plugin/PluginManager.cpp index dad77aa7a0..2b30841172 100644 --- a/src/slic3r/plugin/PluginManager.cpp +++ b/src/slic3r/plugin/PluginManager.cpp @@ -135,7 +135,8 @@ void PluginManager::shutdown() BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << ": PluginManager shutdown enter"; // Detach the libslic3r hooks first so nothing dispatches into Python while (or after) plugins - // unload. Callers stop background slicing before this. + // unload. The lifecycle-event hook also drains callbacks already in progress before returning; + // the remaining hook seams retain their existing shutdown requirements. plugin_hooks::uninstall(); // Reject new plugin loads before we drain. diff --git a/tests/slic3rutils/test_plugin_lifecycle.cpp b/tests/slic3rutils/test_plugin_lifecycle.cpp index 63a4e6827b..bac3a19ea9 100644 --- a/tests/slic3rutils/test_plugin_lifecycle.cpp +++ b/tests/slic3rutils/test_plugin_lifecycle.cpp @@ -1,5 +1,6 @@ #include +#include #include #include #include @@ -12,8 +13,11 @@ #include #include +#include #include +#include #include +#include #include using namespace Slic3r; @@ -114,6 +118,72 @@ PluginDescriptor descriptor_of(PluginManager& manager, const std::string& plugin } // namespace +TEST_CASE("Lifecycle hook shutdown drains concurrent dispatch", "[PluginLifecycle][LifecycleEvents]") +{ + std::mutex mutex; + std::condition_variable cv; + bool callback_entered = false; + bool release_callback = false; + bool shutdown_started = false; + bool shutdown_finished = false; + + set_lifecycle_hook_fn([&](LifecycleEvent, const LifecycleEventContext&) { + std::unique_lock lock(mutex); + callback_entered = true; + cv.notify_all(); + cv.wait(lock, [&] { return release_callback; }); + }); + + std::thread dispatch_thread([] { + LifecycleEventContext ctx; + fire_lifecycle_event(LifecycleEvent::ProjectOpened, ctx); + }); + + bool callback_was_entered = false; + { + std::unique_lock lock(mutex); + callback_was_entered = cv.wait_for(lock, std::chrono::seconds(5), [&] { return callback_entered; }); + } + if (!callback_was_entered) { + { + std::lock_guard lock(mutex); + release_callback = true; + cv.notify_all(); + } + dispatch_thread.join(); + set_lifecycle_hook_fn(nullptr); + FAIL("Lifecycle callback did not start"); + } + + std::thread shutdown_thread([&] { + { + std::lock_guard lock(mutex); + shutdown_started = true; + cv.notify_all(); + } + set_lifecycle_hook_fn(nullptr); + std::lock_guard lock(mutex); + shutdown_finished = true; + cv.notify_all(); + }); + + bool shutdown_was_started = false; + { + std::unique_lock lock(mutex); + shutdown_was_started = cv.wait_for(lock, std::chrono::seconds(5), [&] { return shutdown_started; }); + if (shutdown_was_started) + CHECK_FALSE(cv.wait_for(lock, std::chrono::milliseconds(100), [&] { return shutdown_finished; })); + release_callback = true; + cv.notify_all(); + } + + dispatch_thread.join(); + shutdown_thread.join(); + if (!shutdown_was_started) + FAIL("Lifecycle hook shutdown did not start"); + CHECK(shutdown_finished); +} + TEST_CASE("A discovered script plugin loads and materializes its capability", "[PluginLifecycle][Python]") { ScopedPluginManager plugin_system;