iox-eclipse-iceoryx#751 Remove manual signal handling from roudi app …

…and use signal_watcher instead, use UnnamedSemaphore instead of Semaphore in NamedPipe, PeriodicTask, WatchDog, ConditionVariableData Signed-off-by: Christian Eltzschig <me@elchris.org>
ApexAI · Jun 23, 2022 · 208586b · 208586b
1 parent 6589841
commit 208586b
Show file tree

Hide file tree

Showing 18 changed files with 50 additions and 147 deletions.
diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/cxx/functional_interface.hpp b/iceoryx_hoofs/include/iceoryx_hoofs/cxx/functional_interface.hpp
@@ -16,6 +16,7 @@
 #ifndef IOX_HOOFS_CXX_FUNCTIONAL_POLICY_HPP
 #define IOX_HOOFS_CXX_FUNCTIONAL_POLICY_HPP
 
+#include "iceoryx_hoofs/cxx/attributes.hpp"
 #include "iceoryx_hoofs/cxx/function_ref.hpp"
 #include "iceoryx_hoofs/cxx/type_traits.hpp"
 #include "iceoryx_hoofs/platform/unistd.hpp"

diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/internal/concurrent/periodic_task.hpp b/iceoryx_hoofs/include/iceoryx_hoofs/internal/concurrent/periodic_task.hpp
@@ -1,4 +1,4 @@
-// Copyright (c) 2020 - 2021 by Apex.AI Inc. All rights reserved.
+// Copyright (c) 2020 - 2022 by Apex.AI Inc. All rights reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -19,8 +19,8 @@
 
 #include "iceoryx_hoofs/cxx/string.hpp"
 #include "iceoryx_hoofs/internal/units/duration.hpp"
-#include "iceoryx_hoofs/posix_wrapper/semaphore.hpp"
 #include "iceoryx_hoofs/posix_wrapper/thread.hpp"
+#include "iceoryx_hoofs/posix_wrapper/unnamed_semaphore.hpp"
 
 #include <thread>
 
@@ -121,8 +121,7 @@ class PeriodicTask
     T m_callable;
     posix::ThreadName_t m_taskName;
     units::Duration m_interval{units::Duration::fromMilliseconds(0U)};
-    /// @todo use a refactored posix::Timer object once available
-    posix::Semaphore m_stop{posix::Semaphore::create(posix::CreateUnnamedSingleProcessSemaphore, 0U).value()};
+    cxx::optional<posix::UnnamedSemaphore> m_stop;
     std::thread m_taskExecutor;
 };
 

diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/internal/concurrent/periodic_task.inl b/iceoryx_hoofs/include/iceoryx_hoofs/internal/concurrent/periodic_task.inl
@@ -1,4 +1,4 @@
-// Copyright (c) 2020 - 2021 by Apex.AI Inc. All rights reserved.
+// Copyright (c) 2020 - 2022 by Apex.AI Inc. All rights reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -17,6 +17,8 @@
 #ifndef IOX_HOOFS_CONCURRENT_PERIODIC_TASK_INL
 #define IOX_HOOFS_CONCURRENT_PERIODIC_TASK_INL
 
+#include "iceoryx_hoofs/internal/concurrent/periodic_task.hpp"
+
 namespace iox
 {
 namespace concurrent
@@ -29,6 +31,8 @@ inline PeriodicTask<T>::PeriodicTask(const PeriodicTaskManualStart_t,
     : m_callable(std::forward<Args>(args)...)
     , m_taskName(taskName)
 {
+    posix::UnnamedSemaphoreBuilder().initialValue(0U).isInterProcessCapable(false).create(m_stop).expect(
+        "Unable to create semaphore for periodic task");
 }
 
 template <typename T>
@@ -62,7 +66,7 @@ inline void PeriodicTask<T>::stop() noexcept
 {
     if (m_taskExecutor.joinable())
     {
-        cxx::Expects(!m_stop.post().has_error());
+        cxx::Expects(!m_stop->post().has_error());
         m_taskExecutor.join();
     }
 }
@@ -82,7 +86,7 @@ inline void PeriodicTask<T>::run() noexcept
         IOX_DISCARD_RESULT(m_callable());
 
         /// @todo use a refactored posix::Timer::wait method returning TIMER_TICK and TIMER_STOPPED once available
-        auto waitResult = m_stop.timedWait(m_interval);
+        auto waitResult = m_stop->timedWait(m_interval);
         cxx::Expects(!waitResult.has_error());
 
         waitState = waitResult.value();

diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/named_pipe.hpp b/iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/named_pipe.hpp
@@ -1,4 +1,4 @@
-// Copyright (c) 2021 by Apex.AI Inc. All rights reserved.
+// Copyright (c) 2021 - 2022 by Apex.AI Inc. All rights reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.

diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/signal_watcher.hpp b/iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/signal_watcher.hpp
@@ -1,4 +1,4 @@
-// Copyright (c) 2021 by Apex.AI Inc. All rights reserved.
+// Copyright (c) 2021 - 2022 by Apex.AI Inc. All rights reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.

diff --git a/iceoryx_hoofs/source/posix_wrapper/signal_watcher.cpp b/iceoryx_hoofs/source/posix_wrapper/signal_watcher.cpp
@@ -1,4 +1,4 @@
-// Copyright (c) 2021 by Apex.AI Inc. All rights reserved.
+// Copyright (c) 2021 - 2022 by Apex.AI Inc. All rights reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.

diff --git a/iceoryx_hoofs/test/moduletests/test_concurrent_smart_lock.cpp b/iceoryx_hoofs/test/moduletests/test_concurrent_smart_lock.cpp
@@ -17,7 +17,6 @@
 #include "iceoryx_hoofs/cxx/optional.hpp"
 #include "iceoryx_hoofs/cxx/vector.hpp"
 #include "iceoryx_hoofs/internal/concurrent/smart_lock.hpp"
-#include "iceoryx_hoofs/posix_wrapper/semaphore.hpp"
 #include "iceoryx_hoofs/testing/watch_dog.hpp"
 #include "test.hpp"
 

diff --git a/iceoryx_hoofs/test/moduletests/test_ipc_channel.cpp b/iceoryx_hoofs/test/moduletests/test_ipc_channel.cpp
@@ -65,7 +65,7 @@ class IpcChannel_test : public Test
         auto serverResult = IpcChannelType::create(goodName, IpcChannelSide::SERVER, MaxMsgSize, MaxMsgNumber);
         ASSERT_THAT(serverResult.has_error(), Eq(false));
         server = std::move(serverResult.value());
-        internal::CaptureStderr();
+        ::testing::internal::CaptureStderr();
 
         auto clientResult = IpcChannelType::create(goodName, IpcChannelSide::CLIENT, MaxMsgSize, MaxMsgNumber);
         ASSERT_THAT(clientResult.has_error(), Eq(false));
@@ -74,7 +74,7 @@ class IpcChannel_test : public Test
 
     void TearDown()
     {
-        std::string output = internal::GetCapturedStderr();
+        std::string output = ::testing::internal::GetCapturedStderr();
         if (Test::HasFailure())
         {
             std::cout << output << std::endl;

diff --git a/iceoryx_hoofs/testing/include/iceoryx_hoofs/testing/watch_dog.hpp b/iceoryx_hoofs/testing/include/iceoryx_hoofs/testing/watch_dog.hpp
@@ -18,7 +18,7 @@
 #define IOX_HOOFS_TESTUTILS_WATCH_DOG_HPP
 
 #include "iceoryx_hoofs/internal/units/duration.hpp"
-#include "iceoryx_hoofs/posix_wrapper/semaphore.hpp"
+#include "iceoryx_hoofs/posix_wrapper/unnamed_semaphore.hpp"
 
 #include <functional>
 #include <gtest/gtest.h>
@@ -33,6 +33,11 @@ class Watchdog
     explicit Watchdog(const iox::units::Duration& timeToWait) noexcept
         : m_timeToWait(timeToWait)
     {
+        iox::posix::UnnamedSemaphoreBuilder()
+            .initialValue(0U)
+            .isInterProcessCapable(false)
+            .create(m_watchdogSemaphore)
+            .expect("unable to create semaphore for Watchdog");
     }
 
     Watchdog(const Watchdog&) = delete;
@@ -49,7 +54,7 @@ class Watchdog
     {
         if (m_watchdog.joinable())
         {
-            IOX_DISCARD_RESULT(m_watchdogSemaphore.post());
+            IOX_DISCARD_RESULT(m_watchdogSemaphore->post());
             m_watchdog.join();
         }
     }
@@ -59,7 +64,7 @@ class Watchdog
         reset();
 
         m_watchdog = std::thread([=] {
-            m_watchdogSemaphore.timedWait(m_timeToWait)
+            m_watchdogSemaphore->timedWait(m_timeToWait)
                 .and_then([&](auto& result) {
                     if (result == iox::posix::SemaphoreWaitState::TIMEOUT)
                     {
@@ -82,8 +87,7 @@ class Watchdog
 
   private:
     iox::units::Duration m_timeToWait{0_s};
-    iox::posix::Semaphore m_watchdogSemaphore{
-        iox::posix::Semaphore::create(iox::posix::CreateUnnamedSingleProcessSemaphore, 0U).value()};
+    iox::cxx::optional<iox::posix::UnnamedSemaphore> m_watchdogSemaphore;
     std::thread m_watchdog;
 };
 

diff --git a/iceoryx_posh/include/iceoryx_posh/internal/popo/building_blocks/chunk_queue_data.hpp b/iceoryx_posh/include/iceoryx_posh/internal/popo/building_blocks/chunk_queue_data.hpp
@@ -20,7 +20,6 @@
 #include "iceoryx_hoofs/cxx/variant_queue.hpp"
 #include "iceoryx_hoofs/internal/cxx/unique_id.hpp"
 #include "iceoryx_hoofs/internal/relocatable_pointer/relative_pointer.hpp"
-#include "iceoryx_hoofs/posix_wrapper/semaphore.hpp"
 #include "iceoryx_posh/iceoryx_posh_types.hpp"
 #include "iceoryx_posh/internal/mepoo/shm_safe_unmanaged_chunk.hpp"
 #include "iceoryx_posh/internal/popo/building_blocks/condition_notifier.hpp"

diff --git a/iceoryx_posh/include/iceoryx_posh/internal/popo/building_blocks/condition_variable_data.hpp b/iceoryx_posh/include/iceoryx_posh/internal/popo/building_blocks/condition_variable_data.hpp
@@ -17,7 +17,7 @@
 #ifndef IOX_POSH_POPO_BUILDING_BLOCKS_CONDITION_VARIABLE_DATA_HPP
 #define IOX_POSH_POPO_BUILDING_BLOCKS_CONDITION_VARIABLE_DATA_HPP
 
-#include "iceoryx_hoofs/posix_wrapper/semaphore.hpp"
+#include "iceoryx_hoofs/posix_wrapper/unnamed_semaphore.hpp"
 #include "iceoryx_posh/error_handling/error_handling.hpp"
 #include "iceoryx_posh/iceoryx_posh_types.hpp"
 
@@ -38,16 +38,11 @@ struct ConditionVariableData
     ConditionVariableData& operator=(ConditionVariableData&& rhs) = delete;
     ~ConditionVariableData() noexcept = default;
 
-    posix::Semaphore m_semaphore = std::move(
-        posix::Semaphore::create(posix::CreateUnnamedSharedMemorySemaphore, 0U)
-            .or_else([](posix::SemaphoreError&) {
-                errorHandler(PoshError::POPO__CONDITION_VARIABLE_DATA_FAILED_TO_CREATE_SEMAPHORE, ErrorLevel::FATAL);
-            })
-            .value());
-
+    cxx::optional<posix::UnnamedSemaphore> semaphore;
     RuntimeName_t m_runtimeName;
     std::atomic_bool m_toBeDestroyed{false};
     std::atomic_bool m_activeNotifications[MAX_NUMBER_OF_NOTIFIERS];
+    std::atomic_bool wasNotified{false};
 };
 
 } // namespace popo

diff --git a/iceoryx_posh/include/iceoryx_posh/roudi/roudi_app.hpp b/iceoryx_posh/include/iceoryx_posh/roudi/roudi_app.hpp
@@ -18,7 +18,6 @@
 #define IOX_POSH_ROUDI_ROUDI_APP_HPP
 
 #include "iceoryx_hoofs/log/logcommon.hpp"
-#include "iceoryx_hoofs/posix_wrapper/semaphore.hpp"
 #include "iceoryx_posh/error_handling/error_handling.hpp"
 #include "iceoryx_posh/iceoryx_posh_config.hpp"
 #include "iceoryx_posh/mepoo/mepoo_config.hpp"
@@ -35,9 +34,6 @@ namespace roudi
 class RouDiApp
 {
   public:
-    /// @brief Method passed to the OS signal handler
-    static void roudiSigHandler(int32_t signal) noexcept;
-
     /// @brief C'tor with command line parser, which has already parsed the command line parameters
     /// @param[in] cmdLineParser reference to a command line parser object
     /// @param[in] config the configuration to use
@@ -50,23 +46,16 @@ class RouDiApp
     virtual uint8_t run() noexcept = 0;
 
   protected:
-    /// @brief Tells the OS which signals shall be hooked
-    void registerSigHandler() noexcept;
-
     /// @brief waits for the next signal to RouDi daemon
-    bool waitForSignal() noexcept;
+    [[deprecated(
+        "use iox::posix::waitForTerminationRequest() from 'iceoryx_hoofs/posix_wrapper/signal_watcher.hpp'")]] bool
+    waitForSignal() noexcept;
 
     iox::log::LogLevel m_logLevel{iox::log::LogLevel::kWarn};
     roudi::MonitoringMode m_monitoringMode{roudi::MonitoringMode::ON};
     bool m_run{true};
     RouDiConfig_t m_config;
 
-    posix::Semaphore m_semaphore =
-        std::move(posix::Semaphore::create(posix::CreateUnnamedSingleProcessSemaphore, 0u)
-                      .or_else([](posix::SemaphoreError&) {
-                          errorHandler(PoshError::ROUDI_APP__FAILED_TO_CREATE_SEMAPHORE, ErrorLevel::FATAL);
-                      })
-                      .value());
     version::CompatibilityCheckLevel m_compatibilityCheckLevel{version::CompatibilityCheckLevel::PATCH};
     units::Duration m_processKillDelay{roudi::PROCESS_DEFAULT_KILL_DELAY};
 

diff --git a/iceoryx_posh/source/popo/building_blocks/condition_listener.cpp b/iceoryx_posh/source/popo/building_blocks/condition_listener.cpp
@@ -33,7 +33,7 @@ void ConditionListener::resetSemaphore() noexcept
     bool hasFatalError = false;
     while (!hasFatalError
            && getMembers()
-                  ->m_semaphore.tryWait()
+                  ->semaphore->tryWait()
                   .or_else([&](posix::SemaphoreError) {
                       errorHandler(PoshError::POPO__CONDITION_LISTENER_SEMAPHORE_CORRUPTED_IN_RESET, ErrorLevel::FATAL);
                       hasFatalError = true;
@@ -46,27 +46,20 @@ void ConditionListener::resetSemaphore() noexcept
 void ConditionListener::destroy() noexcept
 {
     m_toBeDestroyed.store(true, std::memory_order_relaxed);
-    getMembers()->m_semaphore.post().or_else([](auto) {
+    getMembers()->semaphore->post().or_else([](auto) {
         errorHandler(PoshError::POPO__CONDITION_LISTENER_SEMAPHORE_CORRUPTED_IN_DESTROY, ErrorLevel::FATAL);
     });
 }
 
 bool ConditionListener::wasNotified() const noexcept
 {
-    auto result = getMembers()->m_semaphore.getValue();
-    if (result.has_error())
-    {
-        errorHandler(PoshError::POPO__CONDITION_LISTENER_SEMAPHORE_CORRUPTED_IN_WAS_TRIGGERED, ErrorLevel::FATAL);
-        return false;
-    }
-
-    return *result != 0;
+    return getMembers()->wasNotified.load(std::memory_order_relaxed);
 }
 
 ConditionListener::NotificationVector_t ConditionListener::wait() noexcept
 {
     return waitImpl([this]() -> bool {
-        if (this->getMembers()->m_semaphore.wait().has_error())
+        if (this->getMembers()->semaphore->wait().has_error())
         {
             errorHandler(PoshError::POPO__CONDITION_LISTENER_SEMAPHORE_CORRUPTED_IN_WAIT, ErrorLevel::FATAL);
             return false;
@@ -78,7 +71,7 @@ ConditionListener::NotificationVector_t ConditionListener::wait() noexcept
 ConditionListener::NotificationVector_t ConditionListener::timedWait(const units::Duration& timeToWait) noexcept
 {
     return waitImpl([this, timeToWait]() -> bool {
-        if (this->getMembers()->m_semaphore.timedWait(timeToWait).has_error())
+        if (this->getMembers()->semaphore->timedWait(timeToWait).has_error())
         {
             errorHandler(PoshError::POPO__CONDITION_LISTENER_SEMAPHORE_CORRUPTED_IN_TIMED_WAIT, ErrorLevel::FATAL);
         }
@@ -116,10 +109,8 @@ ConditionListener::NotificationVector_t ConditionListener::waitImpl(const cxx::f
 
 void ConditionListener::reset(const uint64_t index) noexcept
 {
-    if (index < MAX_NUMBER_OF_NOTIFIERS)
-    {
-        getMembers()->m_activeNotifications[index].store(false, std::memory_order_relaxed);
-    }
+    getMembers()->m_activeNotifications[index].store(false, std::memory_order_relaxed);
+    getMembers()->wasNotified.store(false, std::memory_order_relaxed);
 }
 
 const ConditionVariableData* ConditionListener::getMembers() const noexcept

diff --git a/iceoryx_posh/source/popo/building_blocks/condition_notifier.cpp b/iceoryx_posh/source/popo/building_blocks/condition_notifier.cpp
@@ -36,11 +36,9 @@ ConditionNotifier::ConditionNotifier(ConditionVariableData& condVarDataRef, cons
 
 void ConditionNotifier::notify() noexcept
 {
-    if (m_notificationIndex < MAX_NUMBER_OF_NOTIFIERS)
-    {
-        getMembers()->m_activeNotifications[m_notificationIndex].store(true, std::memory_order_release);
-    }
-    getMembers()->m_semaphore.post().or_else(
+    getMembers()->m_activeNotifications[m_notificationIndex].store(true, std::memory_order_release);
+    getMembers()->wasNotified.store(true, std::memory_order_relaxed);
+    getMembers()->semaphore->post().or_else(
         [](auto) { errorHandler(PoshError::POPO__CONDITION_NOTIFIER_SEMAPHORE_CORRUPT_IN_NOTIFY, ErrorLevel::FATAL); });
 }
 

diff --git a/iceoryx_posh/source/popo/building_blocks/condition_variable_data.cpp b/iceoryx_posh/source/popo/building_blocks/condition_variable_data.cpp
@@ -29,6 +29,10 @@ ConditionVariableData::ConditionVariableData() noexcept
 ConditionVariableData::ConditionVariableData(const RuntimeName_t& runtimeName) noexcept
     : m_runtimeName(runtimeName)
 {
+    posix::UnnamedSemaphoreBuilder().initialValue(0U).isInterProcessCapable(true).create(semaphore).or_else([](auto) {
+        errorHandler(PoshError::POPO__CONDITION_VARIABLE_DATA_FAILED_TO_CREATE_SEMAPHORE, ErrorLevel::FATAL);
+    });
+
     for (auto& id : m_activeNotifications)
     {
         id.store(false, std::memory_order_relaxed);

diff --git a/iceoryx_posh/source/roudi/application/iceoryx_roudi_app.cpp b/iceoryx_posh/source/roudi/application/iceoryx_roudi_app.cpp
@@ -19,6 +19,7 @@
 
 #include "iceoryx_hoofs/cxx/optional.hpp"
 #include "iceoryx_hoofs/cxx/scoped_static.hpp"
+#include "iceoryx_hoofs/posix_wrapper/signal_watcher.hpp"
 #include "iceoryx_posh/internal/roudi/roudi.hpp"
 #include "iceoryx_posh/roudi/iceoryx_roudi_components.hpp"
 
@@ -48,7 +49,7 @@ uint8_t IceOryxRouDiApp::run() noexcept
                                                                 RouDi::RuntimeMessagesThreadStart::IMMEDIATE,
                                                                 m_compatibilityCheckLevel,
                                                                 m_processKillDelay});
-        waitForSignal();
+        iox::posix::waitForTerminationRequest();
     }
     return EXIT_SUCCESS;
 }