From 9042024f3bfc810197584b458f8b3ed5568705af Mon Sep 17 00:00:00 2001 From: Matthias Killat Date: Tue, 20 Sep 2022 15:48:38 +0200 Subject: [PATCH] iox-#1640 Move PolymorphicHandler impl Signed-off-by: Matthias Killat --- .../design_pattern/polymorphic_handler.hpp | 194 ++++++++++-------- .../moduletests/test_polymorphic_handler.cpp | 10 - 2 files changed, 108 insertions(+), 96 deletions(-) diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/design_pattern/polymorphic_handler.hpp b/iceoryx_hoofs/include/iceoryx_hoofs/design_pattern/polymorphic_handler.hpp index 2fe23de1916..a72b08d8b9a 100644 --- a/iceoryx_hoofs/include/iceoryx_hoofs/design_pattern/polymorphic_handler.hpp +++ b/iceoryx_hoofs/include/iceoryx_hoofs/design_pattern/polymorphic_handler.hpp @@ -64,117 +64,139 @@ class PolymorphicHandler static_assert(std::is_base_of::value, "Interface must inherit from Activatable"); public: - // on first call (in a thread): - // 1. localHandler is initialized - // - getCurrent is called - // - instantiates singleton instance() - // - instantiates default - // - sets m_current of instance to default instance (release) - // - default is active - - // if any thread changes the active handler with set (or reset) - // under mutex protection, it will: - // - set the new handler to active - // - set current handler to the new handler - // - deactivate the old handler (can still be used as it still needs to exist) - // - - // on any call after the handler was changes in another thread - // 1. we check whether the handler is active - // (can be outdated information but will eventually be false once the atomic value is updated) - // 2. if it was changed it is now inactive and we wait to obtain the mutex lock - // under lock, we update the local handler to the new one (note that it cannot change - // while this happens as we hold the lock) - /// @brief get the current singleton instance /// @return the current instance - static Interface& get() - { - // NOLINTNEXTLINE - thread_local Interface* localHandler = getCurrent(); // initialized once per thread on first call - - if (!localHandler->isActive()) - { - std::lock_guard lock(instance().m_mutex); - localHandler = getCurrent(); - } - return *localHandler; - } + static Interface& get(); /// @brief set the current singleton instance /// @param handler the handler instance to be set /// @return pointer to the previous instance - static Interface* set(Interface& handler) - { - auto& ins = instance(); - - // setting is rare and we lock to synchronize and update the active flags properly - std::lock_guard lock(ins.m_mutex); - if (ins.m_isFinal) - { - std::cerr << "setting the polymorphic handler after finalize is not allowed" << std::endl; - std::terminate(); - return nullptr; - } - - handler.activate(); // it may have been deactivated before, so always reactivate it - auto prev = ins.m_current.exchange(&handler, std::memory_order_relaxed); - - // anyone still using it will eventually see that it is inactive - // and switch to the new handler - prev->deactivate(); - return prev; - } + static Interface* set(Interface& handler); /// @brief reset the current singleton instance to the default instance /// @return pointer to the previous instance - static Interface* reset() - { - return set(getDefault()); - } + static Interface* reset(); /// @brief finalizes the instance, afterwards no further instance can be set /// during program lifetime - static void finalize() - { - auto& ins = instance(); - std::lock_guard lock(ins.m_mutex); - ins.m_isFinal.store(true); - } + static void finalize(); private: std::atomic_bool m_isFinal{false}; std::mutex m_mutex; std::atomic m_current; - PolymorphicHandler() - { - std::lock_guard lock(m_mutex); - // this is set the first time we call instance() - m_current.store(&getDefault(), std::memory_order_release); - } + PolymorphicHandler(); - static PolymorphicHandler& instance() - { - static PolymorphicHandler handler; - return handler; - } + static PolymorphicHandler& instance(); + + static Default& getDefault(); + + static Interface* getCurrent(); +}; + +// on first call (in a thread): +// 1. localHandler is initialized +// - getCurrent is called +// - instantiates singleton instance() +// - instantiates default +// - sets m_current of instance to default instance (release) +// - default is active + +// if any thread changes the active handler with set (or reset) +// under mutex protection, it will: +// - set the new handler to active +// - set current handler to the new handler +// - deactivate the old handler (can still be used as it still needs to exist) +// + +// on any call after the handler was changes in another thread +// 1. we check whether the handler is active +// (can be outdated information but will eventually be false once the atomic value is updated) +// 2. if it was changed it is now inactive and we wait to obtain the mutex lock +// under lock, we update the local handler to the new one (note that it cannot change +// while this happens as we hold the lock) - // the assumption is that this class manages the default but not any - // other that could be set (which must be created statically before being set) - static Default& getDefault() +template +Interface& PolymorphicHandler::get() +{ + // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) false positive, thread_local + thread_local Interface* localHandler = getCurrent(); // initialized once per thread on first call + + if (!localHandler->isActive()) { - static Default defaultHandler; - return defaultHandler; + std::lock_guard lock(instance().m_mutex); + localHandler = getCurrent(); } + return *localHandler; +} + +template +Interface* PolymorphicHandler::set(Interface& handler) +{ + auto& ins = instance(); - static Interface* getCurrent() + // setting is rare and we lock to synchronize and update the active flags properly + std::lock_guard lock(ins.m_mutex); + if (ins.m_isFinal) { - auto& ins = instance(); - return ins.m_current.load( - std::memory_order_acquire); // must be strong enough to sync memory of the object pointed to + std::cerr << "setting the polymorphic handler after finalize is not allowed" << std::endl; + std::terminate(); // TODO, what to do by default? + return nullptr; } -}; + + handler.activate(); // it may have been deactivated before, so always reactivate it + auto prev = ins.m_current.exchange(&handler, std::memory_order_relaxed); + + // anyone still using it will eventually see that it is inactive + // and switch to the new handler + prev->deactivate(); + return prev; +} + +template +Interface* PolymorphicHandler::reset() +{ + return set(getDefault()); +} + +template +void PolymorphicHandler::finalize() +{ + auto& ins = instance(); + std::lock_guard lock(ins.m_mutex); + ins.m_isFinal.store(true); +} + +template +PolymorphicHandler::PolymorphicHandler() +{ + std::lock_guard lock(m_mutex); + // this is set the first time we call instance() + m_current.store(&getDefault(), std::memory_order_release); +} + +template +PolymorphicHandler& PolymorphicHandler::instance() +{ + static PolymorphicHandler handler; + return handler; +} + +template +Default& PolymorphicHandler::getDefault() +{ + static Default defaultHandler; + return defaultHandler; +} + +template +Interface* PolymorphicHandler::getCurrent() +{ + auto& ins = instance(); + return ins.m_current.load( + std::memory_order_acquire); // must be strong enough to sync memory of the object pointed to +} } // namespace design } // namespace iox \ No newline at end of file diff --git a/iceoryx_hoofs/test/moduletests/test_polymorphic_handler.cpp b/iceoryx_hoofs/test/moduletests/test_polymorphic_handler.cpp index fbb508b30f9..0366472a5bb 100644 --- a/iceoryx_hoofs/test/moduletests/test_polymorphic_handler.cpp +++ b/iceoryx_hoofs/test/moduletests/test_polymorphic_handler.cpp @@ -68,17 +68,11 @@ class PolymorphicHandler_test : public Test public: void SetUp() override { - internal::CaptureStderr(); Handler::reset(); } void TearDown() override { - std::string output = internal::GetCapturedStderr(); - if (Test::HasFailure()) - { - std::cout << output << std::endl; - } } }; @@ -132,9 +126,6 @@ TEST_F(PolymorphicHandler_test, resetToDefaultWorks) EXPECT_EQ(handler.id(), DEFAULT_ID); } -// TODO: death tests abort (regardless of finalize), why? -// how does the fork affect statics? -#if 0 TEST_F(PolymorphicHandler_test, settingAfterFinalizeTerminates) { ::testing::Test::RecordProperty("TEST_ID", "123"); @@ -158,7 +149,6 @@ TEST_F(PolymorphicHandler_test, resetAfterFinalizeTerminates) EXPECT_DEATH(f(), "setting the polymorphic handler after finalize is not allowed"); } -#endif class Activatable_test : public Test {