iox-eclipse-iceoryx#1640 Update comments and documentation

Signed-off-by: Matthias Killat <[email protected]>

doc/design/polymorphic_handler.md

@@ -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
@@ -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) 
+{
+    // setting 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
@@ -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

iceoryx_hoofs/include/iceoryx_hoofs/design_pattern/polymorphic_handler.hpp

@@ -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"
@@ -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;
 };
 
 } // namespace detail
@@ -53,7 +53,6 @@ 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.

iceoryx_hoofs/include/iceoryx_hoofs/design_pattern/static_lifetime_guard.hpp

@@ -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

iceoryx_hoofs/include/iceoryx_hoofs/internal/design_pattern/polymorphic_handler.inl

@@ -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>
 #include <type_traits>
 
 namespace iox
@@ -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

iceoryx_hoofs/include/iceoryx_hoofs/internal/design_pattern/static_lifetime_guard.inl

@@ -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!)

iceoryx_hoofs/test/moduletests/test_polymorphic_handler.cpp

@@ -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();
 

iceoryx_hoofs/test/moduletests/test_static_lifetime_guard.cpp

@@ -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();
@@ -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));
     };