Skip to content

Commit

Permalink
iox-eclipse-iceoryx#1640 Add hooks to PolymorphicHandler
Browse files Browse the repository at this point in the history
Signed-off-by: Matthias Killat <[email protected]>
  • Loading branch information
MatthiasKillat committed Sep 20, 2022
1 parent 3ca98f6 commit 16a202d
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ namespace iox
{
namespace design
{

// TODO: namespace of this concept? should not depend on templated class (e.g. PolymorphicHandler)
// it is not internal as it is supposed to be used to design new Handlers

/// @brief Implements the Activatable concept to be used in the PolymorphicHandler
/// The concept implements a binary switch. By default is switched on (active).
Expand Down Expand Up @@ -42,6 +44,24 @@ class Activatable
bool m_active{true};
};

namespace detail
{

///@brief default hooks for the PolymorphicHandler
///@note template hooks to avoid forced virtual inheritance (STL approach),
template <typename Interface>
struct DefaultHooks
{
/// @brief called after if the polymorphic handler is set or reset after finalize
static void onSetAfterFinalize(Interface& /*currentInstance*/, Interface& /*newInstance*/)
{
std::cerr << "setting the polymorphic handler after finalize is not allowed" << std::endl;
std::terminate();
}
};

} // namespace detail

/// @brief Implements a singleton handler that has a default instance and can be changed
/// to another instance at runtime. All instances have to derive from the same interface.
/// The singleton handler owns the default instance but all other instances are created externally.
Expand All @@ -54,7 +74,7 @@ class Activatable
/// @note The PolymorphicHandler is guaranteed to provide a valid handler during the whole program lifetime (static).
/// It is hence not advisable to have other static variables depend on the PolymorphicHandler.
/// It must be ensured that the are destroyed before the PolymorphicHandler.
template <typename Interface, typename Default>
template <typename Interface, typename Default, typename Hooks = detail::DefaultHooks<Interface>>
class PolymorphicHandler
{
static_assert(std::is_base_of<Interface, Default>::value, "Default must inherit from Interface");
Expand Down Expand Up @@ -117,8 +137,8 @@ class PolymorphicHandler
// under lock, we update the local handler to the new one (note that it cannot change
// while this happens as we hold the lock)

template <typename Interface, typename Default>
Interface& PolymorphicHandler<Interface, Default>::get()
template <typename Interface, typename Default, typename Hooks>
Interface& PolymorphicHandler<Interface, Default, Hooks>::get()
{
// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) false positive, thread_local
thread_local Interface* localHandler = getCurrent(); // initialized once per thread on first call
Expand All @@ -131,17 +151,16 @@ Interface& PolymorphicHandler<Interface, Default>::get()
return *localHandler;
}

template <typename Interface, typename Default>
Interface* PolymorphicHandler<Interface, Default>::set(Interface& handler)
template <typename Interface, typename Default, typename Hooks>
Interface* PolymorphicHandler<Interface, Default, Hooks>::set(Interface& handler)
{
auto& ins = instance();

// setting is rare and we lock to synchronize and update the active flags properly
std::lock_guard<std::mutex> lock(ins.m_mutex);
if (ins.m_isFinal)
{
std::cerr << "setting the polymorphic handler after finalize is not allowed" << std::endl;
std::terminate(); // TODO, what to do by default?
Hooks::onSetAfterFinalize(*getCurrent(), handler);
return nullptr;
}

Expand All @@ -154,44 +173,44 @@ Interface* PolymorphicHandler<Interface, Default>::set(Interface& handler)
return prev;
}

template <typename Interface, typename Default>
Interface* PolymorphicHandler<Interface, Default>::reset()
template <typename Interface, typename Default, typename Hooks>
Interface* PolymorphicHandler<Interface, Default, Hooks>::reset()
{
return set(getDefault());
}

template <typename Interface, typename Default>
void PolymorphicHandler<Interface, Default>::finalize()
template <typename Interface, typename Default, typename Hooks>
void PolymorphicHandler<Interface, Default, Hooks>::finalize()
{
auto& ins = instance();
std::lock_guard<std::mutex> lock(ins.m_mutex);
ins.m_isFinal.store(true);
}

template <typename Interface, typename Default>
PolymorphicHandler<Interface, Default>::PolymorphicHandler()
template <typename Interface, typename Default, typename Hooks>
PolymorphicHandler<Interface, Default, Hooks>::PolymorphicHandler()
{
std::lock_guard<std::mutex> lock(m_mutex);
// this is set the first time we call instance()
m_current.store(&getDefault(), std::memory_order_release);
}

template <typename Interface, typename Default>
PolymorphicHandler<Interface, Default>& PolymorphicHandler<Interface, Default>::instance()
template <typename Interface, typename Default, typename Hooks>
PolymorphicHandler<Interface, Default, Hooks>& PolymorphicHandler<Interface, Default, Hooks>::instance()
{
static PolymorphicHandler handler;
return handler;
}

template <typename Interface, typename Default>
Default& PolymorphicHandler<Interface, Default>::getDefault()
template <typename Interface, typename Default, typename Hooks>
Default& PolymorphicHandler<Interface, Default, Hooks>::getDefault()
{
static Default defaultHandler;
return defaultHandler;
}

template <typename Interface, typename Default>
Interface* PolymorphicHandler<Interface, Default>::getCurrent()
template <typename Interface, typename Default, typename Hooks>
Interface* PolymorphicHandler<Interface, Default, Hooks>::getCurrent()
{
auto& ins = instance();
return ins.m_current.load(
Expand Down
52 changes: 39 additions & 13 deletions iceoryx_hoofs/test/moduletests/test_polymorphic_handler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@ struct Interface : public iox::design::Activatable
virtual ~Interface() = default;

virtual uint32_t id() = 0;

void reset()
{
value = 0;
}

uint32_t value{0};
};

constexpr uint32_t DEFAULT_ID = 73;
Expand Down Expand Up @@ -61,7 +68,17 @@ Alternate& alternateHandler()
return h;
};

using Handler = iox::design::PolymorphicHandler<Interface, Default>;
struct Hooks
{
// to check whether the arguments are used correctly
static void onSetAfterFinalize(Interface& currentInstance, Interface& newInstance)
{
currentInstance.value = currentInstance.id();
newInstance.value = newInstance.id();
}
};

using Handler = iox::design::PolymorphicHandler<Interface, Default, Hooks>;

class PolymorphicHandler_test : public Test
{
Expand Down Expand Up @@ -126,28 +143,37 @@ TEST_F(PolymorphicHandler_test, resetToDefaultWorks)
EXPECT_EQ(handler.id(), DEFAULT_ID);
}

TEST_F(PolymorphicHandler_test, settingAfterFinalizeTerminates)
TEST_F(PolymorphicHandler_test, settingAfterFinalizeCallsHook)
{
::testing::Test::RecordProperty("TEST_ID", "171ac802-01b9-4e08-80a6-6f2defecaf6d");

auto f = [&]() {
Handler::finalize();
Handler::set(alternateHandler());
};
auto& handler = Handler::get();
handler.reset();
alternateHandler().reset();

// note that all following tests will also call the after finalize
// hook but we only check if we care whether it was called
Handler::finalize();
Handler::set(alternateHandler());

EXPECT_DEATH(f(), "setting the polymorphic handler after finalize is not allowed");
// does the hook set the values to the corresponding arguments?
EXPECT_EQ(handler.value, DEFAULT_ID);
EXPECT_EQ(alternateHandler().value, ALTERNATE_ID);
}

TEST_F(PolymorphicHandler_test, resetAfterFinalizeTerminates)
TEST_F(PolymorphicHandler_test, resetAfterFinalizeCallsHook)
{
::testing::Test::RecordProperty("TEST_ID", "996220e3-7985-4d57-bd3f-844987cf99dc");

auto f = [&]() {
Handler::finalize();
Handler::reset();
};
auto& handler = Handler::get();
handler.reset();
alternateHandler().reset();

Handler::finalize();
Handler::reset();

EXPECT_DEATH(f(), "setting the polymorphic handler after finalize is not allowed");
EXPECT_EQ(handler.value, DEFAULT_ID);
EXPECT_EQ(alternateHandler().value, 0);
}

class Activatable_test : public Test
Expand Down

0 comments on commit 16a202d

Please sign in to comment.