Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Iox 1640 create polymorphic singleton abstraction #1656

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
2bf8488
iox-#1640 Polymorphic handler implementation
MatthiasKillat Sep 19, 2022
aef2103
iox-#1640 Polymorphic handler test
MatthiasKillat Sep 19, 2022
d88805e
iox-#1640 Test Acivatable concept
MatthiasKillat Sep 20, 2022
2a6e336
iox-#1640 Document PolymorphicHandler
MatthiasKillat Sep 20, 2022
f38f187
iox-#1640 Move PolymorphicHandler impl
MatthiasKillat Sep 20, 2022
235739f
iox-#1640 Update PolymorphicHandler tests
MatthiasKillat Sep 20, 2022
f2baec6
iox-#1640 Add hooks to PolymorphicHandler
MatthiasKillat Sep 20, 2022
c07c569
iox-#1640 Move PolymorphicSingleton impl
MatthiasKillat Sep 21, 2022
ddbf37c
iox-#1640 Add StaticLifetimeGuard
MatthiasKillat Oct 19, 2022
b97eeaf
iox-#1640 Remove mutex
MatthiasKillat Oct 21, 2022
4692854
iox-#1640 Move StaticLifetimeGuard impl
MatthiasKillat Oct 24, 2022
90c756f
iox-#1640 PolymorphicHandler set by guard
MatthiasKillat Oct 24, 2022
0644af9
iox-#1640 Make StaticLifetimeGuard assignable
MatthiasKillat Oct 24, 2022
5a6be27
iox-#1640 Add design document
MatthiasKillat Oct 26, 2022
7722618
iox-#1640 Update documentation
MatthiasKillat Nov 9, 2022
78f9443
iox-#1640 Optimize memory order
MatthiasKillat Nov 15, 2022
cf6dc33
iox-#1640 Correct activation during self exchange
MatthiasKillat Nov 15, 2022
65e2ec6
iox-#1640 Remove Activatable restriction
MatthiasKillat Nov 30, 2022
a26d747
iox-#1640 Correct finalize and reset test
MatthiasKillat Nov 30, 2022
d67f4dc
iox-#1640 Update documentation
MatthiasKillat Dec 1, 2022
b2d2029
iox-#1640 Make setCount protected
MatthiasKillat Jan 18, 2023
0abe2a6
iox-#1640 Weakened counter modification
MatthiasKillat Jan 18, 2023
f5c444b
iox-#1640 Make StaticLifetimeGuard tests independent
MatthiasKillat Jan 18, 2023
614c94a
iox-#1640 Optimize PolymorphicHandler
MatthiasKillat Jan 19, 2023
00486cf
iox-#1640 Let set and reset return bool
MatthiasKillat Jan 19, 2023
3bf204b
iox-#1640 Update iceoryx-unreleased.md
MatthiasKillat Jan 19, 2023
97793c8
iox-#1640 Add concurrent guard test
MatthiasKillat Jan 24, 2023
193b4c6
iox-#1640 Update tests
MatthiasKillat Jan 30, 2023
b35cb02
iox-#1640 Correct concurrent StaticLifetimeGuard destruction
MatthiasKillat Jan 30, 2023
4c259b9
iox-#1640 Update comments and documentation
MatthiasKillat Jan 31, 2023
fcf639d
iox-#1640 Let DefaultHooks call abort
MatthiasKillat Jan 31, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions doc/design/polymorphic_handler.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ that also inherits from `I`
variables
1. Obtaining the current handler must be thread-safe
1. It must be possible to finalize the handler, i.e. prohibit any changes after it is finalized.
1. Any attempt to change the handler after it is finalized, shall call an function that has access
1. Any attempt to change the handler after it is finalized, shall call a function that has access
to the current and new handler (for e.g. logging).

To achieve this, we define another support class `StaticLifeTimeGuard` that solves the singleton lifetime problem.
To achieve this, we define another support class `StaticLifetimeGuard` that solves the singleton lifetime problem.
This class can be used on its own and is based on the nifty counter reference counting.

While obtaining the instance is thread-safe, the instance managed by the handler may not be
Expand Down Expand Up @@ -172,7 +172,15 @@ StaticLifetimeGuard<OtherHandler> guard;

// set the handler to the instance guarded by guard,
// this will create another guard to ensure the lifetime
Handler::set(guard);
bool success = Handler::set(guard);
if(!success)
{
// set may only fail after finalize
//
// do something in this case
// ...
// even if set was not successful, get() will return the previous handler
}

// OtherHandler instance exists now,
// any other thread will eventually use the new handler
Expand Down Expand Up @@ -226,7 +234,7 @@ Otherwise it will obtain the new handler with a stronger memory synchronization

Note that the current handler can change any time but there is no problem as all handlers remain
usbale during the entire program lifetime. Due to this, there are no issues like the ABA problem,
the worst thing that can happen is working with a outdated handler.
the worst thing that can happen is working with an outdated handler.

This does not require blocking and only relies on fairly cheap atomic operations.
Without using a mutex while using the handler, it is impossible that
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
#define IOX_HOOFS_DESIGN_PATTERN_POLYMORPHIC_HANDLER_HPP

#include <atomic>
#include <iostream>
#include <type_traits>

#include "iceoryx_hoofs/design_pattern/static_lifetime_guard.hpp"
Expand All @@ -39,7 +38,8 @@ struct DefaultHooks
/// @brief called if the polymorphic handler is set or reset after finalize
/// @param currentInstance the current instance of the handler singleton
/// @param newInstance the instance of the handler singleton to be set
static void onSetAfterFinalize(Interface& currentInstance, Interface& newInstance) noexcept;
/// @note calls terminate and does not return
[[noreturn]] static void onSetAfterFinalize(Interface& currentInstance, Interface& newInstance) noexcept;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this does not restrict other hooks to define a non-noreturn method?

Copy link
Contributor Author

@MatthiasKillat MatthiasKillat Jan 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No it does not and should not. It is not mandatory for any hook. It is just plugged in as a template argument type ...

However, it is a guarantee the DefaultHooks are able to give and so they should. Can remove it, but would advise not to. This allows optimization and communicates intent. And is portable in C++11.

};

} // namespace detail
Expand All @@ -53,13 +53,13 @@ struct DefaultHooks
/// attempting to set or reset the handler after finalize was called.
///
/// @note In the special case where Default equals Interface, no polymorphism is required.
/// It is then possible to e.g. switch between multiple instances of Default type.
/// @note The lifetime of external non-default instances must exceed the lifetime of the PolymorphicHandler.
/// @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 they are destroyed before the PolymorphicHandler.
/// @note Hooks must implement
/// static void onSetAfterFinalize(Interface& /*currentInstance*/, Interface& /*newInstance*/).
/// @note DefaultHooks call terminate if the handler is set or reset after finalize
template <typename Interface, typename Default, typename Hooks = detail::DefaultHooks<Interface>>
class PolymorphicHandler
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ namespace design_pattern
/// Hence there can be guards without any instance existing.
/// These guards still protect the instance from destruction if it is ever constructed.
/// 4. If and once the instance is constructed, it will be destructed only after main exits (static
/// destruction time).
/// Existing guards used variables must be used to control destruction order
/// of static variables if a specific order is required.
/// destruction).
/// 5. Guards can be used in static variables to control destruction order of static (singleton)
/// instances if a specific order of destruction is required.
/// @tparam T the type of the instance to be guarded
///
/// @note all public functions are thread-safe
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "iceoryx_hoofs/design_pattern/polymorphic_handler.hpp"
#include "iceoryx_hoofs/design_pattern/static_lifetime_guard.hpp"
#include <atomic>
#include <exception>
MatthiasKillat marked this conversation as resolved.
Show resolved Hide resolved
#include <type_traits>

namespace iox
Expand All @@ -31,7 +32,7 @@ namespace detail
{

template <typename Interface>
void DefaultHooks<Interface>::onSetAfterFinalize(Interface&, Interface&) noexcept
[[noreturn]] void DefaultHooks<Interface>::onSetAfterFinalize(Interface&, Interface&) noexcept
{
// we should not use an error handling construct (e.g. some IOX_ASSERT) here for dependency reasons
// we could in principle do nothing by default as well, but the misuse failure should have visible consequences
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,7 @@ template <typename T>
template <typename... Args>
T& StaticLifetimeGuard<T>::instance(Args&&... args) noexcept
{
// primary guard
static StaticLifetimeGuard<T> guard;
static StaticLifetimeGuard<T> primaryGuard;

// we determine wether this call has to initialize the instance
// via CAS (without mutex!)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ TEST_F(PolymorphicHandler_test, settingAfterFinalizeCallsHook)
// we always finalize it to be alternateHandler
Handler::set(alternateGuard);

// reset the handler value to non-zero and check later whether they are set to non-zero as expecteded
// reset the handler value to zero and check later whether they are set to non-zero as expected
defaultHandler.reset();
alternateHandler.reset();

Expand Down
4 changes: 2 additions & 2 deletions iceoryx_hoofs/test/moduletests/test_static_lifetime_guard.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ TEST_F(StaticLifetimeGuard_test, constructionAfterDestructionWorks)
EXPECT_EQ(instance.id, FIRST_INSTANCE_ID);
}

// first instance destroyed (should usually only happen at the the program end
// first instance destroyed (should usually only happen at the end of the program
// during static destruction)

T::Foo::reset();
Expand Down Expand Up @@ -349,7 +349,7 @@ TEST_F(StaticLifetimeGuard_test, instanceCtorIsConcurrentlyCalledExactlyOnce)
// all threads have notfied (but may pass wait in any order ...)

// cannot wait too long otherwise we slow down the tests too much,
// cannot be optimized away, as it has has side effects (counting)
// cannot be optimized away, as it has side effects (counting)
Sut::instance(std::chrono::milliseconds(1));
};

Expand Down