From 9a4fb4c1f78be1c058625eecbf78138f1a4c1ecd Mon Sep 17 00:00:00 2001 From: Christian Eltzschig Date: Thu, 16 Jun 2022 17:23:05 +0200 Subject: [PATCH 01/17] iox-#751 expect uses 'write' instead of 'std::cout' so that it can be used from within a signal handler Signed-off-by: Christian Eltzschig --- .../include/iceoryx_hoofs/cxx/functional_interface.hpp | 3 ++- .../iceoryx_hoofs/internal/cxx/functional_interface.inl | 8 ++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/cxx/functional_interface.hpp b/iceoryx_hoofs/include/iceoryx_hoofs/cxx/functional_interface.hpp index e67065b943..e7569626f6 100644 --- a/iceoryx_hoofs/include/iceoryx_hoofs/cxx/functional_interface.hpp +++ b/iceoryx_hoofs/include/iceoryx_hoofs/cxx/functional_interface.hpp @@ -18,8 +18,9 @@ #include "iceoryx_hoofs/cxx/function_ref.hpp" #include "iceoryx_hoofs/cxx/type_traits.hpp" +#include "iceoryx_hoofs/platform/unistd.hpp" -#include +#include #include namespace iox diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/functional_interface.inl b/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/functional_interface.inl index cd0ce76e29..3e5967cc68 100644 --- a/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/functional_interface.inl +++ b/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/functional_interface.inl @@ -37,7 +37,9 @@ inline void Expect::expect(const StringType& msg) const noexcept if (!(*static_cast(this))) { - std::cout << msg << std::endl; + // it is possible that expect is called inside an error handler therefore we + // use write + IOX_DISCARD_RESULT(write(STDERR_FILENO, &msg[0], strlen(&msg[0]))); Ensures(false); } } @@ -53,7 +55,9 @@ inline ValueType& ExpectWithValue::expect(const StringType& if (!(*derivedThis)) { - std::cout << msg << std::endl; + // it is possible that expect is called inside an error handler therefore we + // use write + IOX_DISCARD_RESULT(write(STDERR_FILENO, &msg[0], strlen(&msg[0]))); Ensures(false); } From 6589841b5827b737ea40336b60b140668543fa00 Mon Sep 17 00:00:00 2001 From: Christian Eltzschig Date: Thu, 16 Jun 2022 17:23:30 +0200 Subject: [PATCH 02/17] iox-#751 Remove old semaphore and use the new unnamed semaphore in the signal watcher and the named pipe Signed-off-by: Christian Eltzschig --- iceoryx_hoofs/CMakeLists.txt | 1 - .../posix_wrapper/semaphore_interface.hpp | 24 +- .../posix_wrapper/named_pipe.hpp | 18 +- .../iceoryx_hoofs/posix_wrapper/semaphore.hpp | 333 ---------------- .../posix_wrapper/signal_watcher.hpp | 4 +- .../source/posix_wrapper/named_pipe.cpp | 32 +- .../source/posix_wrapper/semaphore.cpp | 298 -------------- .../source/posix_wrapper/signal_watcher.cpp | 26 +- .../moduletests/test_semaphore_module.cpp | 374 ------------------ 9 files changed, 56 insertions(+), 1054 deletions(-) delete mode 100644 iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/semaphore.hpp delete mode 100644 iceoryx_hoofs/source/posix_wrapper/semaphore.cpp delete mode 100644 iceoryx_hoofs/test/moduletests/test_semaphore_module.cpp diff --git a/iceoryx_hoofs/CMakeLists.txt b/iceoryx_hoofs/CMakeLists.txt index 7f75907c0c..0de0e6aa33 100644 --- a/iceoryx_hoofs/CMakeLists.txt +++ b/iceoryx_hoofs/CMakeLists.txt @@ -71,7 +71,6 @@ iox_add_library( source/posix_wrapper/named_pipe.cpp source/posix_wrapper/named_semaphore.cpp source/posix_wrapper/posix_access_rights.cpp - source/posix_wrapper/semaphore.cpp source/posix_wrapper/semaphore_interface.cpp source/posix_wrapper/shared_memory_object.cpp source/posix_wrapper/shared_memory_object/allocator.cpp diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/semaphore_interface.hpp b/iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/semaphore_interface.hpp index 102db14242..95a3cbc946 100644 --- a/iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/semaphore_interface.hpp +++ b/iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/semaphore_interface.hpp @@ -19,12 +19,34 @@ #include "iceoryx_hoofs/cxx/expected.hpp" #include "iceoryx_hoofs/internal/units/duration.hpp" #include "iceoryx_hoofs/platform/semaphore.hpp" -#include "iceoryx_hoofs/posix_wrapper/semaphore.hpp" namespace iox { namespace posix { +enum class SemaphoreError +{ + CREATION_FAILED, + NAME_TOO_LONG, + UNABLE_TO_OPEN_HANDLE, + INVALID_SEMAPHORE_HANDLE, + SEMAPHORE_OVERFLOW, + INTERRUPTED_BY_SIGNAL_HANDLER, + INVALID_NAME, + PERMISSION_DENIED, + ALREADY_EXIST, + FILE_DESCRIPTOR_LIMIT_REACHED, + NO_SEMAPHORE_WITH_THAT_NAME_EXISTS, + OUT_OF_MEMORY, + UNDEFINED +}; + +enum class SemaphoreWaitState +{ + TIMEOUT, + NO_TIMEOUT, +}; + namespace internal { /// @brief Defines the interface of a named and unnamed semaphore. diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/named_pipe.hpp b/iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/named_pipe.hpp index c414a244f7..17448d5caa 100644 --- a/iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/named_pipe.hpp +++ b/iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/named_pipe.hpp @@ -22,7 +22,8 @@ #include "iceoryx_hoofs/internal/posix_wrapper/ipc_channel.hpp" #include "iceoryx_hoofs/internal/posix_wrapper/shared_memory_object.hpp" #include "iceoryx_hoofs/internal/units/duration.hpp" -#include "iceoryx_hoofs/posix_wrapper/semaphore.hpp" +#include "iceoryx_hoofs/platform/semaphore.hpp" +#include "iceoryx_hoofs/posix_wrapper/unnamed_semaphore.hpp" #include @@ -36,7 +37,10 @@ class NamedPipe : public DesignPattern::Creation // no system restrictions at all, except available memory. MAX_MESSAGE_SIZE and MAX_NUMBER_OF_MESSAGES can be // increased as long as there is enough memory available static constexpr uint64_t MAX_MESSAGE_SIZE = 4U * 1024U; - static constexpr uint64_t MAX_NUMBER_OF_MESSAGES = 10U; + static constexpr uint32_t MAX_NUMBER_OF_MESSAGES = 10U; + static_assert( + MAX_NUMBER_OF_MESSAGES < 51, + "The maximum number of supported messages must be less or equal to the maximum allowed semaphore value"); static constexpr uint64_t NULL_TERMINATOR_SIZE = 0U; static constexpr units::Duration CYCLE_TIME = units::Duration::fromMilliseconds(10); @@ -122,16 +126,15 @@ class NamedPipe : public DesignPattern::Creation class NamedPipeData { public: - NamedPipeData(bool& isInitialized, IpcChannelError& error, const uint64_t maxMsgNumber) noexcept; + NamedPipeData(bool& isInitialized, IpcChannelError& error, const uint32_t maxMsgNumber) noexcept; NamedPipeData(const NamedPipeData&) = delete; NamedPipeData(NamedPipeData&& rhs) = delete; - ~NamedPipeData() noexcept; NamedPipeData& operator=(const NamedPipeData&) = delete; NamedPipeData& operator=(NamedPipeData&& rhs) = delete; - Semaphore& sendSemaphore() noexcept; - Semaphore& receiveSemaphore() noexcept; + UnnamedSemaphore& sendSemaphore() noexcept; + UnnamedSemaphore& receiveSemaphore() noexcept; bool waitForInitialization() const noexcept; bool hasValidState() const noexcept; @@ -148,8 +151,7 @@ class NamedPipe : public DesignPattern::Creation static constexpr units::Duration WAIT_FOR_INIT_SLEEP_TIME = units::Duration::fromMilliseconds(1); std::atomic initializationGuard{INVALID_DATA}; - using semaphoreMemory_t = uint8_t[sizeof(Semaphore)]; - alignas(Semaphore) semaphoreMemory_t semaphores[2U]; + cxx::optional semaphores[2U]; }; diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/semaphore.hpp b/iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/semaphore.hpp deleted file mode 100644 index 3e545b1fc5..0000000000 --- a/iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/semaphore.hpp +++ /dev/null @@ -1,333 +0,0 @@ -// Copyright (c) 2019 by Robert Bosch GmbH. All rights reserved. -// Copyright (c) 2021 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. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// -// SPDX-License-Identifier: Apache-2.0 -#ifndef IOX_HOOFS_POSIX_WRAPPER_SEMAPHORE_HPP -#define IOX_HOOFS_POSIX_WRAPPER_SEMAPHORE_HPP - -#include "iceoryx_hoofs/cxx/expected.hpp" -#include "iceoryx_hoofs/cxx/helplets.hpp" -#include "iceoryx_hoofs/cxx/string.hpp" -#include "iceoryx_hoofs/design_pattern/creation.hpp" -#include "iceoryx_hoofs/internal/relocatable_pointer/relative_pointer.hpp" -#include "iceoryx_hoofs/internal/units/duration.hpp" -#include "iceoryx_hoofs/platform/fcntl.hpp" -#include "iceoryx_hoofs/platform/semaphore.hpp" -#include "iceoryx_hoofs/platform/stat.hpp" - -#include - -namespace iox -{ -namespace posix -{ -enum class SemaphoreError -{ - CREATION_FAILED, - NAME_TOO_LONG, - UNABLE_TO_OPEN_HANDLE, - INVALID_SEMAPHORE_HANDLE, - SEMAPHORE_OVERFLOW, - INTERRUPTED_BY_SIGNAL_HANDLER, - INVALID_NAME, - PERMISSION_DENIED, - ALREADY_EXIST, - FILE_DESCRIPTOR_LIMIT_REACHED, - NO_SEMAPHORE_WITH_THAT_NAME_EXISTS, - OUT_OF_MEMORY, - UNDEFINED -}; - -enum class SemaphoreWaitState -{ - TIMEOUT, - NO_TIMEOUT, -}; - -struct CreateUnnamedSingleProcessSemaphore_t -{ -}; -struct CreateUnnamedSharedMemorySemaphore_t -{ -}; -struct CreateNamedSemaphore_t -{ -}; -struct OpenNamedSemaphore_t -{ -}; -static constexpr CreateUnnamedSingleProcessSemaphore_t CreateUnnamedSingleProcessSemaphore = - CreateUnnamedSingleProcessSemaphore_t(); -static constexpr CreateUnnamedSharedMemorySemaphore_t CreateUnnamedSharedMemorySemaphore = - CreateUnnamedSharedMemorySemaphore_t(); -static constexpr CreateNamedSemaphore_t CreateNamedSemaphore = CreateNamedSemaphore_t(); -static constexpr OpenNamedSemaphore_t OpenNamedSemaphore = OpenNamedSemaphore_t(); - -/// @brief Posix semaphore C++ Wrapping class -/// @code -/// auto semaphore = posix::Semaphore::CreateUnnamed(false, 5); -/// int value; -/// if ( semaphore.getValue(value) ) // no error has occurred -/// { -/// std::cout << value << std::endl; -/// } -/// @endcode -class Semaphore : public DesignPattern::Creation -{ - public: - /// @brief Default constructor which creates an uninitialized semaphore. - /// This semaphore object is unusable you need to reassign it with - /// an object created by the semaphore factory methods - Semaphore() noexcept; - - /// @brief Move constructor. - Semaphore(Semaphore&& rhs) noexcept; - - /// @brief Move assignment operator. - Semaphore& operator=(Semaphore&& rhs) noexcept; - - /// @brief We are denying Semaphore copy since it manages the semaphore resource - /// and the underlying concept did not include copying - Semaphore(const Semaphore&) = delete; - - /// @brief We are denying Semaphore copy since it manages the semaphore resource - /// and the underlying concept did not include copying - Semaphore& operator=(const Semaphore&) = delete; - - /// @brief Destructor - ~Semaphore() noexcept; - - /// @brief calls sem_getvalue which gets the value of a semaphore - /// From the sem_getvalue manpage: sem_getvalue() places the current value - /// of the semaphore pointed to sem into the integer pointed to by sval. - /// - /// If one or more processes or threads are blocked waiting to lock the - /// semaphore with sem_wait(3), POSIX.1 permits two possibilities for the - /// value returned in sval: either 0 is returned; or a negative number whose - /// absolute value is the count of the number of processes and threads - /// currently blocked in sem_wait(3). Linux adopts the former behavior. - /// - /// @param[in] value reference in which the value of the semaphore is - /// written to - /// - /// @return expected which contains either the value of the semaphore or - /// the cause why the value could not be retrieved - cxx::expected getValue() const noexcept; - - /// @brief calls sem_post which unlocks a semaphore - /// From the sem_post manpage: sem_post() increments (unlocks) the - /// semaphore pointed to by sem. If the semaphore's value consequently - /// becomes greater than zero, then another process or thread blocked in a - /// sem_wait(3) call will be woken up and proceed to lock the semaphore. - /// - /// @return if post fails the expected contains the error which occurred - cxx::expected post() noexcept; - - /// @brief see wait() - /// @param[in] abs_timeout timeout of the wait - /// @return when successful the SemaphoreWaitState states if a timeout happened - /// or not otherwise the SemaphoreError contains the error - cxx::expected timedWait(const units::Duration abs_timeout) noexcept; - - /// @brief see wait() - /// @return if the semaphore was decremented the expected contains the value true - /// otherwise false. if an error occurred it is stored inside the expected - cxx::expected tryWait() noexcept; - - /// @brief calls sem_wait which locks a semaphore - /// From the sem_wait manpage: sem_wait() decrements (locks) the semaphore - /// pointed to by sem. If the semaphore's value is greater than zero, then - /// the decrement proceeds, and the function returns, immediately. If the - /// semaphore - /// currently has the value zero, then the call blocks until either it - /// becomes possible to perform the decrement (i.e., the semaphore value - /// rises above zero), or a signal handler interrupts the call. - /// - /// iox_sem_trywait() is the same as sem_wait(), except that if the decrement - /// cannot be immediately performed, then call returns an error (errno set - /// to EAGAIN) instead of blocking. - /// - /// iox_sem_timedwait() is the same as sem_wait(), except that abs_timeout - /// specifies a limit on the amount of time that the call should block if - /// the decrement cannot be immediately performed. - /// - /// If the timeout has already expired by the time of the call, and the - /// semaphore could not be locked immediately, then iox_sem_timedwait() fails - /// with a timeout error (errno set to ETIMEDOUT). - /// - /// If the operation can be performed immediately, then iox_sem_timedwait() - /// never fails with a timeout error, regardless of the value of - /// abs_timeout. Furthermore, the validity of abs_timeout is not checked in - /// this case. - /// - /// @return if an error during the call occurs the error value is set - cxx::expected wait() noexcept; - - private: - cxx::string<128> m_name; - bool m_isCreated = true; - bool m_isNamedSemaphore = true; - bool m_isShared = false; - - mutable iox_sem_t m_handle{}; - mutable iox_sem_t* m_handlePtr = nullptr; - - private: - friend class DesignPattern::Creation; - - /// @brief Creates a local unnamed semaphore. - /// The Semaphore should be initialized but that has to be verified - /// via IsInitialized() - /// For details see man sem_init. - /// @param[in] value initial value of the semaphore - Semaphore(CreateUnnamedSingleProcessSemaphore_t, const unsigned int value) noexcept; - - /// @brief Creates unnamed semaphore in the shared memory. - /// The Semaphore should be initialized but that has to be verified - /// via IsInitialized() - /// For details see man sem_init. - /// @param[in] value initial value of the semaphore - Semaphore(CreateUnnamedSharedMemorySemaphore_t, const unsigned int value) noexcept; - - /// @brief Opens an already existing named semaphore. If a semaphore with - /// name does not exist an uninitialized Semaphore is returned - /// otherwise the Semaphore can be initialized but that has to be - /// verified via IsInitialized(). - /// For details see man sem_open. - /// @param[in] name name of the semaphore - /// @param[in] oflag specifies flags that control the operation of the call - /// O_CREAT flag is not allowed here - Semaphore(OpenNamedSemaphore_t, const char* name, const int oflag) noexcept; - - /// @brief Creates an exclusive named semaphore. If a semaphore with name - /// already exists then the Semaphore returned is not initialized - /// and not usable! - /// You always have to verify if the semaphore returned by this - /// factory is initialized via the IsInitialized() method. - /// For details see man sem_open. - /// @param[in] name name of the semaphore - /// @param[in] mode specifies the permissions to be placed on the new - /// semaphore, see man 2 open to get a detailed description - /// on mode_t - /// @param[in] value the initial value of the semaphore - /// @return Semaphore object which can be initialized, if a semaphore - /// named name exists it is definitly an uninitialized semaphore. - Semaphore(CreateNamedSemaphore_t, const char* name, const mode_t mode, const unsigned int value) noexcept; - - /// @brief calls sem_close which closes a named semaphore - /// From the sem_close manpage: sem_close() closes the named semaphore - /// referred to by sem, allowing any resources that the system has allocated - /// to the calling process for this semaphore to be freed. - /// - /// @return returns false when sem_close fails otherwise true - bool close() noexcept; - - /// @brief calls sem_destroy which destroys a unnamed semaphore - /// From the sem_destroy manpage: sem_destroy() destroys the unnamed - /// semaphore at the address pointed to by sem. - /// - /// Only a semaphore that has been initialized by sem_init(3) should be - /// destroyed using sem_destroy(). - /// - /// Destroying a semaphore that other processes or threads are currently - /// blocked on (in sem_wait(3)) produces undefined behavior. - /// - /// Using a semaphore that has been destroyed produces undefined results, - /// until the semaphore has been reinitialized using sem_init(3). - /// - /// @return returns false when sem_destroy fails otherwise true - bool destroy() noexcept; - - /// @brief calls sem_init which initializes an unnamed semaphore - /// From the sem_init manpage: sem_init() initializes the unnamed semaphore - /// at the address pointed to by sem. The value argument specifies the - /// initial value for the semaphore. - /// - /// The pshared argument indicates whether this semaphore is to be shared - /// between the threads of a process, or between processes. - /// - /// If pshared has the value 0, then the semaphore is shared between the - /// threads of a process, and should be located at some address that is - /// visible to all threads (e.g., a global variable, or a vari‐ able - /// allocated dynamically on the heap). - /// - /// If pshared is nonzero, then the semaphore is shared between processes, - /// and should be located in a region of shared memory (see shm_open(3), - /// mmap(2), and shmget(2)). (Since a child created by fork(2) - /// inherits its parent's memory mappings, it can also access the - /// semaphore.) Any process that can access the shared memory region can - /// operate on the semaphore using sem_post(3), sem_wait(3), and so on. - /// - /// Initializing a semaphore that has already been initialized results in - /// undefined behavior. - /// - /// @return returns false when sem_init fails otherwise true - static bool init(iox_sem_t* handle, const int pshared, const unsigned int value) noexcept; - - /// @brief calls sem_open which initializes and opens a named semaphore - /// From the sem_open manpage: sem_open() creates a new POSIX semaphore or - /// opens an existing semaphore. The semaphore is identified by name. For - /// details of the construction of name, see sem_overview(7). - /// - /// The oflag argument specifies flags that control the operation of the - /// call. (Definitions of the flags values can be obtained by including - /// .) If O_CREAT is specified in oflag, then the sem‐ aphore is - /// created if it does not already exist. The owner (user ID) of the - /// semaphore is set to the effective user ID of the calling process. The - /// group ownership (group ID) is set to the effective group ID of the - /// calling process. If both O_CREAT and O_EXCL are specified in oflag, - /// then an error is returned if a semaphore with the given name already - /// exists. - /// - /// If O_CREAT is specified in oflag, then two additional arguments must - /// be supplied. The mode argument specifies the permissions to be placed - /// on the new semaphore, as for open(2). (Symbolic defini‐ tions for the - /// permissions bits can be obtained by including .) The - /// permissions settings are masked against the process umask. Both read and - /// write permission should be granted to each class of user that will - /// access the semaphore. The value argument specifies the initial value for - /// the new semaphore. If O_CREAT is specified, and a semaphore with the - /// given name already exists, then mode and value are ignored. - /// - /// @return returns false when sem_open fails otherwise true - bool open(const int oflag) noexcept; - - /// @brief returns the pointer to the managed semaphore. You can use this - /// pointer with all the sem_** functions. - iox_sem_t* getHandle() const noexcept; - - bool open(const int oflag, const mode_t mode, const unsigned int value) noexcept; - - /// @brief calls sem_unlink which removes a named semaphore - /// From the sem_unlink manpage: sem_unlink() removes the named semaphore - /// referred to by name. The semaphore name is removed immediately. The - /// semaphore is destroyed once all other processes that have the semaphore - /// open close it. - /// - /// @return returns false when sem_unlink fails otherwise true - static bool unlink(const char* name) noexcept; - - /// @brief Returns true if the semaphore was created with CreateNamed or - /// OpenNamed otherwise it returns false. - bool isNamedSemaphore() const noexcept; - - void closeHandle() noexcept; - - static SemaphoreError errnoToEnum(const int errnoValue) noexcept; -}; -} // namespace posix -} // namespace iox - -#endif // IOX_HOOFS_POSIX_WRAPPER_SEMAPHORE_HPP diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/signal_watcher.hpp b/iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/signal_watcher.hpp index ca99ef8427..180acaa852 100644 --- a/iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/signal_watcher.hpp +++ b/iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/signal_watcher.hpp @@ -16,8 +16,8 @@ #ifndef IOX_HOOFS_POSIX_WRAPPER_SIGNAL_WATCHER_HPP #define IOX_HOOFS_POSIX_WRAPPER_SIGNAL_WATCHER_HPP -#include "iceoryx_hoofs/posix_wrapper/semaphore.hpp" #include "iceoryx_hoofs/posix_wrapper/signal_handler.hpp" +#include "iceoryx_hoofs/posix_wrapper/unnamed_semaphore.hpp" #include @@ -69,7 +69,7 @@ class SignalWatcher private: friend void internalSignalHandler(int) noexcept; mutable std::atomic m_numberOfWaiters{0U}; - mutable Semaphore m_semaphore; + mutable cxx::optional m_semaphore; std::atomic_bool m_hasSignalOccurred{false}; SignalGuard m_sigTermGuard; diff --git a/iceoryx_hoofs/source/posix_wrapper/named_pipe.cpp b/iceoryx_hoofs/source/posix_wrapper/named_pipe.cpp index c25f1ef457..0e154e7c74 100644 --- a/iceoryx_hoofs/source/posix_wrapper/named_pipe.cpp +++ b/iceoryx_hoofs/source/posix_wrapper/named_pipe.cpp @@ -111,7 +111,7 @@ NamedPipe::NamedPipe(const IpcChannelName_t& name, m_isInitialized = true; if (m_sharedMemory->hasOwnership()) { - new (m_data) NamedPipeData(m_isInitialized, m_errorValue, maxMsgNumber); + new (m_data) NamedPipeData(m_isInitialized, m_errorValue, static_cast(maxMsgNumber)); } else { @@ -324,7 +324,7 @@ cxx::expected NamedPipe::timedReceive(const units: // NOLINTNEXTLINE(cppcoreguidelines-pro-type-member-init) semaphores are initalized via placementCreate call NamedPipe::NamedPipeData::NamedPipeData(bool& isInitialized, IpcChannelError& error, - const uint64_t maxMsgNumber) noexcept + const uint32_t maxMsgNumber) noexcept { auto signalError = [&](const char* name) { std::cerr << "Unable to create " << name << " semaphore for named pipe \"" << 'x' << "\""; @@ -332,8 +332,10 @@ NamedPipe::NamedPipeData::NamedPipeData(bool& isInitialized, error = IpcChannelError::INTERNAL_LOGIC_ERROR; }; - Semaphore::placementCreate( - &semaphores[SEND_SEMAPHORE], CreateUnnamedSharedMemorySemaphore, static_cast(maxMsgNumber)) + UnnamedSemaphoreBuilder() + .initialValue(maxMsgNumber) + .isInterProcessCapable(true) + .create(semaphores[SEND_SEMAPHORE]) .or_else([&](auto) { signalError("send"); }); if (!isInitialized) @@ -341,7 +343,10 @@ NamedPipe::NamedPipeData::NamedPipeData(bool& isInitialized, return; } - Semaphore::placementCreate(&semaphores[RECEIVE_SEMAPHORE], CreateUnnamedSharedMemorySemaphore, 0U) + UnnamedSemaphoreBuilder() + .initialValue(0U) + .isInterProcessCapable(true) + .create(semaphores[RECEIVE_SEMAPHORE]) .or_else([&](auto) { signalError("receive"); }); if (!isInitialized) @@ -352,23 +357,14 @@ NamedPipe::NamedPipeData::NamedPipeData(bool& isInitialized, initializationGuard.store(VALID_DATA); } -NamedPipe::NamedPipeData::~NamedPipeData() noexcept +UnnamedSemaphore& NamedPipe::NamedPipeData::sendSemaphore() noexcept { - if (hasValidState()) - { - sendSemaphore().~Semaphore(); - receiveSemaphore().~Semaphore(); - } -} - -Semaphore& NamedPipe::NamedPipeData::sendSemaphore() noexcept -{ - return reinterpret_cast(semaphores[SEND_SEMAPHORE]); + return *semaphores[SEND_SEMAPHORE]; } -Semaphore& NamedPipe::NamedPipeData::receiveSemaphore() noexcept +UnnamedSemaphore& NamedPipe::NamedPipeData::receiveSemaphore() noexcept { - return reinterpret_cast(semaphores[RECEIVE_SEMAPHORE]); + return *semaphores[RECEIVE_SEMAPHORE]; } bool NamedPipe::NamedPipeData::waitForInitialization() const noexcept diff --git a/iceoryx_hoofs/source/posix_wrapper/semaphore.cpp b/iceoryx_hoofs/source/posix_wrapper/semaphore.cpp deleted file mode 100644 index 304fff36b4..0000000000 --- a/iceoryx_hoofs/source/posix_wrapper/semaphore.cpp +++ /dev/null @@ -1,298 +0,0 @@ -// Copyright (c) 2019 by Robert Bosch GmbH. All rights reserved. -// Copyright (c) 2021 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. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// -// SPDX-License-Identifier: Apache-2.0 - -#include "iceoryx_hoofs/posix_wrapper/semaphore.hpp" -#include "iceoryx_hoofs/cxx/helplets.hpp" -#include "iceoryx_hoofs/platform/platform_correction.hpp" -#include "iceoryx_hoofs/posix_wrapper/posix_call.hpp" - -namespace iox -{ -namespace posix -{ -Semaphore::Semaphore() noexcept = default; - -Semaphore::Semaphore(Semaphore&& rhs) noexcept -{ - *this = std::move(rhs); -} - -Semaphore& Semaphore::operator=(Semaphore&& rhs) noexcept -{ - if (this != &rhs) - { - closeHandle(); - - CreationPattern_t::operator=(std::move(rhs)); - - m_name = std::move(rhs.m_name); - m_isCreated = std::move(rhs.m_isCreated); - m_isNamedSemaphore = std::move(rhs.m_isNamedSemaphore); - m_handle = std::move(rhs.m_handle); - m_isShared = std::move(rhs.m_isShared); - if (m_isNamedSemaphore || m_isShared) - { - m_handlePtr = std::move(rhs.m_handlePtr); - } - else - { - m_handlePtr = &m_handle; - } - - rhs.m_handlePtr = nullptr; - rhs.m_isInitialized = false; - } - - return *this; -} - -Semaphore::~Semaphore() noexcept -{ - closeHandle(); -} - -void Semaphore::closeHandle() noexcept -{ - if (m_isInitialized) - { - if (isNamedSemaphore()) - { - close(); - if (m_isCreated) - { - unlink(m_name.c_str()); - } - } - else - { - destroy(); - } - m_isInitialized = false; - } -} - -cxx::expected Semaphore::getValue() const noexcept -{ - int value{0}; - auto call = posixCall(iox_sem_getvalue)(getHandle(), &value).failureReturnValue(-1).evaluate(); - if (call.has_error()) - { - return cxx::error(errnoToEnum(call.get_error().errnum)); - } - - return cxx::success(value); -} - -cxx::expected Semaphore::post() noexcept -{ - auto call = posixCall(iox_sem_post)(getHandle()).failureReturnValue(-1).evaluate(); - if (call.has_error()) - { - return cxx::error(errnoToEnum(call.get_error().errnum)); - } - - return cxx::success<>(); -} - -cxx::expected Semaphore::timedWait(const units::Duration abs_timeout) noexcept -{ - const struct timespec timeout = abs_timeout.timespec(units::TimeSpecReference::Epoch); - auto call = - posixCall(iox_sem_timedwait)(getHandle(), &timeout).failureReturnValue(-1).ignoreErrnos(ETIMEDOUT).evaluate(); - - if (call.has_error()) - { - return cxx::error(errnoToEnum(call.get_error().errnum)); - } - else if (call->errnum == ETIMEDOUT) - { - return cxx::success(SemaphoreWaitState::TIMEOUT); - } - else - { - return cxx::success(SemaphoreWaitState::NO_TIMEOUT); - } -} - -cxx::expected Semaphore::tryWait() noexcept -{ - auto call = posixCall(iox_sem_trywait)(getHandle()).failureReturnValue(-1).ignoreErrnos(EAGAIN).evaluate(); - - if (call.has_error()) - { - return cxx::error(errnoToEnum(call.get_error().errnum)); - } - - return cxx::success(call->errnum != EAGAIN); -} - -cxx::expected Semaphore::wait() noexcept -{ - auto call = posixCall(iox_sem_wait)(getHandle()).failureReturnValue(-1).evaluate(); - - if (call.has_error()) - { - return cxx::error(errnoToEnum(call.get_error().errnum)); - } - - return cxx::success<>(); -} - -iox_sem_t* Semaphore::getHandle() const noexcept -{ - return (isNamedSemaphore()) ? m_handlePtr : &m_handle; -} - -Semaphore::Semaphore(CreateUnnamedSingleProcessSemaphore_t, const unsigned int value) noexcept - : m_isNamedSemaphore(false) -{ - if (init(&m_handle, 0, value)) - { - m_isInitialized = true; - } - else - { - m_isInitialized = false; - m_errorValue = SemaphoreError::CREATION_FAILED; - } -} - -Semaphore::Semaphore(CreateUnnamedSharedMemorySemaphore_t, const unsigned int value) noexcept - : m_isNamedSemaphore(false) -{ - if (init(&m_handle, 1, value)) - { - m_isInitialized = true; - } - else - { - m_isInitialized = false; - m_errorValue = SemaphoreError::CREATION_FAILED; - } -} - -Semaphore::Semaphore(OpenNamedSemaphore_t, const char* name, const int oflag) noexcept - : m_isCreated(false) -{ - if (!m_name.unsafe_assign(name)) - { - m_isInitialized = false; - m_errorValue = SemaphoreError::NAME_TOO_LONG; - return; - } - - if (open(oflag)) - { - m_isInitialized = true; - } - else - { - m_errorValue = SemaphoreError::UNABLE_TO_OPEN_HANDLE; - m_isInitialized = false; - } -} - -// NOLINTNEXTLINE(readability-function-size) todo(iox-#832): make a struct out of arguments -Semaphore::Semaphore(CreateNamedSemaphore_t, const char* name, const mode_t mode, const unsigned int value) noexcept - : m_isCreated(true) -{ - if (!m_name.unsafe_assign(name)) - { - m_isInitialized = false; - m_errorValue = SemaphoreError::NAME_TOO_LONG; - return; - } - - if (open(O_CREAT | O_EXCL, mode, value)) - { - m_isInitialized = true; - } - else - { - m_errorValue = SemaphoreError::CREATION_FAILED; - m_isInitialized = false; - } -} - -bool Semaphore::close() noexcept -{ - return !posixCall(iox_sem_close)(getHandle()).failureReturnValue(-1).evaluate().has_error(); -} - -bool Semaphore::destroy() noexcept -{ - return !posixCall(iox_sem_destroy)(getHandle()).failureReturnValue(-1).evaluate().has_error(); -} - -bool Semaphore::init(iox_sem_t* handle, const int pshared, const unsigned int value) noexcept -{ - return !posixCall(iox_sem_init)(handle, pshared, value).failureReturnValue(-1).evaluate().has_error(); -} - -bool Semaphore::open(const int oflag) noexcept -{ - return !posixCall(iox_sem_open)(m_name.c_str(), oflag) - .failureReturnValue(IOX_SEM_FAILED) - .evaluate() - .and_then([this](auto& r) { this->m_handlePtr = r.value; }) - .or_else([this](auto&) { this->m_errorValue = SemaphoreError::CREATION_FAILED; }) - .has_error(); -} - -bool Semaphore::open(const int oflag, const mode_t mode, const unsigned int value) noexcept -{ - return !posixCall(iox_sem_open_ext)(m_name.c_str(), oflag, mode, value) - .failureReturnValue(IOX_SEM_FAILED) - .evaluate() - .and_then([this](auto& r) { this->m_handlePtr = r.value; }) - .or_else([this](auto&) { this->m_errorValue = SemaphoreError::CREATION_FAILED; }) - .has_error(); -} - -bool Semaphore::unlink(const char* name) noexcept -{ - return !posixCall(iox_sem_unlink)(name).failureReturnValue(-1).evaluate().has_error(); -} - -bool Semaphore::isNamedSemaphore() const noexcept -{ - return m_isNamedSemaphore; -} - -SemaphoreError Semaphore::errnoToEnum(const int errnoValue) noexcept -{ - switch (errnoValue) - { - case EINVAL: - std::cerr << "semaphore object is in an inconsistent state" << std::endl; - return SemaphoreError::INVALID_SEMAPHORE_HANDLE; - case EOVERFLOW: - std::cerr << "semaphore is overflowing" << std::endl; - return SemaphoreError::SEMAPHORE_OVERFLOW; - case EINTR: - std::cerr << "call was interrupted by signal handler" << std::endl; - return SemaphoreError::INTERRUPTED_BY_SIGNAL_HANDLER; - default: - std::cerr << "an unexpected error occurred in semaphore - this should never happen! errno: " - // NOLINTNEXTLINE(concurrency-mt-unsafe) impossible case - << strerror(errnoValue) << std::endl; - return SemaphoreError::UNDEFINED; - } -} - -} // namespace posix -} // namespace iox diff --git a/iceoryx_hoofs/source/posix_wrapper/signal_watcher.cpp b/iceoryx_hoofs/source/posix_wrapper/signal_watcher.cpp index 88e6a46e32..a8c5317b83 100644 --- a/iceoryx_hoofs/source/posix_wrapper/signal_watcher.cpp +++ b/iceoryx_hoofs/source/posix_wrapper/signal_watcher.cpp @@ -29,26 +29,18 @@ void internalSignalHandler(int) noexcept for (uint64_t remainingNumberOfWaiters = instance.m_numberOfWaiters.load(); remainingNumberOfWaiters > 0; --remainingNumberOfWaiters) { - instance.m_semaphore.post().or_else([](auto) { - constexpr const char MSG[] = "Unable to increment semaphore in signal handler"; - auto result = write(STDERR_FILENO, MSG, sizeof(MSG)); - IOX_DISCARD_RESULT(result); - std::abort(); - }); + instance.m_semaphore->post().expect("Unable to increment semaphore in signal handler"); } } SignalWatcher::SignalWatcher() noexcept - : m_semaphore{std::move(Semaphore::create(CreateUnnamedSingleProcessSemaphore, 0U) - .or_else([](auto) { - std::cerr << "Unable to create semaphore for signal watcher" << std::endl; - constexpr bool UNABLE_TO_CREATE_SEMAPHORE_FOR_SIGNAL_WATCHER = false; - cxx::Ensures(UNABLE_TO_CREATE_SEMAPHORE_FOR_SIGNAL_WATCHER); - }) - .value())} - , m_sigTermGuard(registerSignalHandler(Signal::TERM, internalSignalHandler)) + : m_sigTermGuard(registerSignalHandler(Signal::TERM, internalSignalHandler)) , m_sigIntGuard(registerSignalHandler(Signal::INT, internalSignalHandler)) { + UnnamedSemaphoreBuilder() + .isInterProcessCapable(false) + .create(m_semaphore) + .expect("Unable to create semaphore for signal watcher"); } SignalWatcher& SignalWatcher::getInstance() noexcept @@ -65,11 +57,7 @@ void SignalWatcher::waitForSignal() const noexcept return; } - m_semaphore.wait().or_else([](auto) { - std::cerr << "Unable to wait on semaphore in signal watcher" << std::endl; - constexpr bool UNABLE_TO_WAIT_ON_SEMAPHORE_IN_SIGNAL_WATCHER = false; - cxx::Ensures(UNABLE_TO_WAIT_ON_SEMAPHORE_IN_SIGNAL_WATCHER); - }); + m_semaphore->wait().expect("Unable to wait on semaphore in signal watcher"); } bool SignalWatcher::wasSignalTriggered() const noexcept diff --git a/iceoryx_hoofs/test/moduletests/test_semaphore_module.cpp b/iceoryx_hoofs/test/moduletests/test_semaphore_module.cpp deleted file mode 100644 index ba63ce08b0..0000000000 --- a/iceoryx_hoofs/test/moduletests/test_semaphore_module.cpp +++ /dev/null @@ -1,374 +0,0 @@ -// Copyright (c) 2019, 2021 by Robert Bosch GmbH. All rights reserved. -// Copyright (c) 2021 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. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// -// SPDX-License-Identifier: Apache-2.0 - -#include "iceoryx_hoofs/cxx/convert.hpp" -#include "iceoryx_hoofs/internal/units/duration.hpp" -#include "iceoryx_hoofs/platform/time.hpp" -#include "iceoryx_hoofs/posix_wrapper/semaphore.hpp" -#include "iceoryx_hoofs/testing/test.hpp" -#include "iceoryx_hoofs/testing/timing_test.hpp" -#include "test_posix_semaphore_common.hpp" - -#include -#include -#include - -namespace -{ -using namespace ::testing; -using namespace iox::units::duration_literals; - -typedef iox::posix::Semaphore* CreateSemaphore(); - -iox::posix::Semaphore* createNamedSemaphore() -{ - static int i = 10; - auto semaphore = iox::posix::Semaphore::create(iox::posix::CreateNamedSemaphore, - std::string("/fuuSem" + iox::cxx::convert::toString(i++)).c_str(), - S_IRUSR | S_IWUSR, - 0); - return (semaphore.has_error()) ? nullptr : new iox::posix::Semaphore(std::move(*semaphore)); -} - -iox::posix::Semaphore* createUnnamedSemaphore() -{ - auto semaphore = iox::posix::Semaphore::create(iox::posix::CreateUnnamedSingleProcessSemaphore, 0); - return (semaphore.has_error()) ? nullptr : new iox::posix::Semaphore(std::move(*semaphore)); -} - -class Semaphore_test : public TestWithParam -{ - public: - Semaphore_test() - : sut((*GetParam())()) - { - } - - ~Semaphore_test() - { - delete sut; - delete syncSemaphore; - } - - void SetUp() - { - internal::CaptureStderr(); - ASSERT_THAT(sut, Ne(nullptr)); - ASSERT_THAT(syncSemaphore, Ne(nullptr)); - } - - void TearDown() - { - std::string output = internal::GetCapturedStderr(); - if (Test::HasFailure()) - { - std::cout << output << std::endl; - } - } - - static constexpr unsigned long long TIMING_TEST_TIMEOUT{(100_ms).toNanoseconds()}; - - iox::posix::Semaphore* sut{nullptr}; - iox::posix::Semaphore* syncSemaphore = [] { - auto semaphore = iox::posix::Semaphore::create(iox::posix::CreateUnnamedSingleProcessSemaphore, 0); - return (semaphore.has_error()) ? nullptr : new iox::posix::Semaphore(std::move(*semaphore)); - }(); -}; - -class SemaphoreCreate_test : public Test -{ - public: - void SetUp() - { - internal::CaptureStderr(); - } - - void TearDown() - { - std::string output = internal::GetCapturedStderr(); - if (Test::HasFailure()) - { - std::cout << output << std::endl; - } - } -}; - -INSTANTIATE_TEST_SUITE_P(Semaphore_test, Semaphore_test, Values(&createNamedSemaphore, &createUnnamedSemaphore)); - -TEST_F(SemaphoreCreate_test, CreateNamedSemaphore) -{ - ::testing::Test::RecordProperty("TEST_ID", "80f5fba8-c6db-4948-86e1-9e23d413d1ac"); - auto semaphore = iox::posix::Semaphore::create(iox::posix::CreateNamedSemaphore, "/fuuSem", S_IRUSR | S_IWUSR, 10); - EXPECT_THAT(semaphore.has_error(), Eq(false)); -} - -TEST_F(SemaphoreCreate_test, CreateExistingNamedSemaphore) -{ - ::testing::Test::RecordProperty("TEST_ID", "bee46586-bcf2-42e6-9dda-ab2611fc973f"); - auto semaphore = iox::posix::Semaphore::create(iox::posix::CreateNamedSemaphore, "/fuuSem1", S_IRUSR | S_IWUSR, 10); - auto semaphore2 = - iox::posix::Semaphore::create(iox::posix::CreateNamedSemaphore, "/fuuSem1", S_IRUSR | S_IWUSR, 10); - ASSERT_EQ(semaphore.has_error(), false); - ASSERT_EQ(semaphore2.has_error(), true); -} - -TEST_F(SemaphoreCreate_test, CreateLocalUnnamedSemaphore) -{ - ::testing::Test::RecordProperty("TEST_ID", "42099e77-9ac9-425f-8e53-056dc3b73d71"); - auto semaphore = iox::posix::Semaphore::create(iox::posix::CreateUnnamedSingleProcessSemaphore, 10); - EXPECT_THAT(semaphore.has_error(), Eq(false)); -} - -TEST_F(SemaphoreCreate_test, OpenNamedSemaphore) -{ - ::testing::Test::RecordProperty("TEST_ID", "349cdf0d-987e-4e2f-aa35-98a40fdf979b"); - auto semaphore = iox::posix::Semaphore::create(iox::posix::CreateNamedSemaphore, "/fuuSem", S_IRUSR | S_IWUSR, 10); - auto semaphore2 = iox::posix::Semaphore::create(iox::posix::OpenNamedSemaphore, "/fuuSem", 0); - EXPECT_THAT(semaphore.has_error(), Eq(false)); - EXPECT_THAT(semaphore2.has_error(), Eq(false)); -} - -TEST_F(SemaphoreCreate_test, OpenNamedSemaphoreWithEmptyNameFails) -{ - ::testing::Test::RecordProperty("TEST_ID", "5dab9f61-8b27-4684-8e9d-bbd50430b9fa"); - auto semaphore = iox::posix::Semaphore::create(iox::posix::CreateNamedSemaphore, "", S_IRUSR | S_IWUSR, 10); - EXPECT_THAT(semaphore.has_error(), Eq(true)); -} - -TEST_F(SemaphoreCreate_test, OpenNonExistingNamedSemaphore) -{ - ::testing::Test::RecordProperty("TEST_ID", "0034b274-5a3b-49dc-a5d2-1715d068809f"); - auto semaphore2 = iox::posix::Semaphore::create(iox::posix::OpenNamedSemaphore, "/fuuSem", S_IRUSR | S_IWUSR); - EXPECT_THAT(semaphore2.has_error(), Eq(true)); -} - -TEST_P(Semaphore_test, PostIncreasesSemaphoreValue) -{ - ::testing::Test::RecordProperty("TEST_ID", "af3013ef-683e-4ad4-874f-5c7e1a3f41fd"); - for (int i = 0; i < 12; ++i) - { - ASSERT_FALSE(sut->post().has_error()); - } - - EXPECT_TRUE(setSemaphoreToZeroAndVerifyValue(*sut, 12)); -} - -TEST_P(Semaphore_test, WaitDecreasesSemaphoreValue) -{ - ::testing::Test::RecordProperty("TEST_ID", "29a63157-caee-4d28-a477-c4fa7048911c"); - for (int i = 0; i < 18; ++i) - { - ASSERT_FALSE(sut->post().has_error()); - } - for (int i = 0; i < 7; ++i) - { - ASSERT_FALSE(sut->wait().has_error()); - } - - EXPECT_TRUE(setSemaphoreToZeroAndVerifyValue(*sut, 11)); -} - -TEST_P(Semaphore_test, SuccessfulTryWaitDecreasesSemaphoreValue) -{ - ::testing::Test::RecordProperty("TEST_ID", "a98d1c45-d538-4abc-9633-f4868163785d"); - for (int i = 0; i < 15; ++i) - { - ASSERT_FALSE(sut->post().has_error()); - } - for (int i = 0; i < 9; ++i) - { - auto call = sut->tryWait(); - ASSERT_THAT(call.has_error(), Eq(false)); - ASSERT_THAT(*call, Eq(true)); - } - - EXPECT_TRUE(setSemaphoreToZeroAndVerifyValue(*sut, 6)); -} - -TEST_P(Semaphore_test, FailingTryWaitDoesNotChangeSemaphoreValue) -{ - ::testing::Test::RecordProperty("TEST_ID", "8bad3784-888b-44cd-ad5d-1c4662b26b17"); - for (int i = 0; i < 4; ++i) - { - auto call = sut->tryWait(); - ASSERT_THAT(call.has_error(), Eq(false)); - ASSERT_THAT(*call, Eq(false)); - } - - auto result = sut->getValue(); - ASSERT_THAT(result.has_error(), Eq(false)); - EXPECT_THAT(*result, Eq(0)); -} - -TEST_P(Semaphore_test, SuccessfulTimedWaitDecreasesSemaphoreValue) -{ - ::testing::Test::RecordProperty("TEST_ID", "e3bd3f5d-967c-4a5b-9f22-a8f92f73b3c3"); - const iox::units::Duration timeToWait = 2_ms; - for (int i = 0; i < 19; ++i) - { - ASSERT_FALSE(sut->post().has_error()); - } - - for (int i = 0; i < 12; ++i) - { - auto call = sut->timedWait(timeToWait); - ASSERT_FALSE(call.has_error()); - ASSERT_TRUE(call.value() == iox::posix::SemaphoreWaitState::NO_TIMEOUT); - } - - EXPECT_TRUE(setSemaphoreToZeroAndVerifyValue(*sut, 7)); -} - -TEST_P(Semaphore_test, FailingTimedWaitDoesNotChangeSemaphoreValue) -{ - ::testing::Test::RecordProperty("TEST_ID", "5a630be5-aef6-493e-a8ee-4dfabe642258"); - const iox::units::Duration timeToWait = 2_us; - for (int i = 0; i < 4; ++i) - { - auto call = sut->timedWait(timeToWait); - ASSERT_FALSE(call.has_error()); - ASSERT_TRUE(call.value() == iox::posix::SemaphoreWaitState::TIMEOUT); - } - - auto result = sut->getValue(); - ASSERT_THAT(result.has_error(), Eq(false)); - EXPECT_THAT(*result, Eq(0)); -} - - -TEST_P(Semaphore_test, TryWaitAfterPostIsSuccessful) -{ - ::testing::Test::RecordProperty("TEST_ID", "22354447-c44d-443c-97a5-f5fdffb09748"); - ASSERT_FALSE(sut->post().has_error()); - auto call = sut->tryWait(); - ASSERT_THAT(call.has_error(), Eq(false)); - ASSERT_THAT(*call, Eq(true)); -} - -TEST_P(Semaphore_test, TryWaitWithNoPostIsNotSuccessful) -{ - ::testing::Test::RecordProperty("TEST_ID", "0e5d6817-88a9-4fca-889e-4dbfe2c30e48"); - ASSERT_FALSE(sut->post().has_error()); - auto call = sut->tryWait(); - ASSERT_THAT(call.has_error(), Eq(false)); - ASSERT_THAT(*call, Eq(true)); -} - -TEST_P(Semaphore_test, WaitValidAfterPostIsNonBlocking) -{ - ::testing::Test::RecordProperty("TEST_ID", "d4b1de28-89c4-4dfa-a465-8c7cfc652d67"); - ASSERT_FALSE(sut->post().has_error()); - // this call should not block and should be successful - EXPECT_THAT(sut->wait().has_error(), Eq(false)); -} - -TEST_P(Semaphore_test, WaitIsBlocking) -{ - ::testing::Test::RecordProperty("TEST_ID", "5869a475-6be3-4b55-aa58-2ebf11d46081"); - std::atomic counter{0}; - std::thread t1([&] { - ASSERT_FALSE(syncSemaphore->wait().has_error()); - ASSERT_FALSE(sut->post().has_error()); - ASSERT_FALSE(syncSemaphore->wait().has_error()); - counter++; - ASSERT_FALSE(sut->post().has_error()); - }); - - EXPECT_THAT(counter.load(), Eq(0)); - - ASSERT_FALSE(syncSemaphore->post().has_error()); - ASSERT_FALSE(sut->wait().has_error()); - EXPECT_THAT(counter.load(), Eq(0)); - - ASSERT_FALSE(syncSemaphore->post().has_error()); - ASSERT_FALSE(sut->wait().has_error()); - EXPECT_THAT(counter.load(), Eq(1)); - - t1.join(); -} - -TEST_P(Semaphore_test, MoveAssignment) -{ - ::testing::Test::RecordProperty("TEST_ID", "bf7277fd-4e5c-49dd-b48b-f5d1ed6e4c01"); - iox::posix::Semaphore b; - { - b = std::move(*sut); - } - - EXPECT_THAT(b.post().has_error(), Eq(false)); -} - -TEST_P(Semaphore_test, MoveCTor) -{ - ::testing::Test::RecordProperty("TEST_ID", "e8083f97-c3c0-4e79-9948-395a837bde84"); - iox::posix::Semaphore b(std::move(*sut)); - - EXPECT_THAT(b.post().has_error(), Eq(false)); -} - -TIMING_TEST_P(Semaphore_test, TimedWaitWithTimeout, Repeat(3), [&] { - using namespace iox::units; - std::atomic_bool timedWaitFinish{false}; - - std::thread t([&] { - auto timeout = Duration::fromNanoseconds(TIMING_TEST_TIMEOUT); - ASSERT_FALSE(syncSemaphore->post().has_error()); - ASSERT_FALSE(sut->wait().has_error()); - auto call = sut->timedWait(timeout); - TIMING_TEST_ASSERT_FALSE(call.has_error()); - TIMING_TEST_EXPECT_TRUE(call.value() == iox::posix::SemaphoreWaitState::TIMEOUT); - timedWaitFinish.store(true); - }); - - ASSERT_FALSE(syncSemaphore->wait().has_error()); - ASSERT_FALSE(sut->post().has_error()); - std::this_thread::sleep_for(std::chrono::nanoseconds(TIMING_TEST_TIMEOUT / 3 * 2)); - TIMING_TEST_EXPECT_FALSE(timedWaitFinish.load()); - - std::this_thread::sleep_for(std::chrono::nanoseconds(TIMING_TEST_TIMEOUT / 3 * 2)); - TIMING_TEST_EXPECT_TRUE(timedWaitFinish.load()); - - t.join(); -}); - - -TIMING_TEST_P(Semaphore_test, TimedWaitWithoutTimeout, Repeat(3), [&] { - using namespace iox::units; - std::atomic_bool timedWaitFinish{false}; - - std::thread t([&] { - auto timeout = Duration::fromNanoseconds(TIMING_TEST_TIMEOUT); - ASSERT_FALSE(syncSemaphore->post().has_error()); - ASSERT_FALSE(sut->wait().has_error()); - auto call = sut->timedWait(timeout); - TIMING_TEST_ASSERT_FALSE(call.has_error()); - TIMING_TEST_EXPECT_TRUE(call.value() == iox::posix::SemaphoreWaitState::NO_TIMEOUT); - timedWaitFinish.store(true); - }); - - ASSERT_FALSE(syncSemaphore->wait().has_error()); - ASSERT_FALSE(sut->post().has_error()); - std::this_thread::sleep_for(std::chrono::nanoseconds(TIMING_TEST_TIMEOUT / 3 * 2)); - TIMING_TEST_EXPECT_FALSE(timedWaitFinish.load()); - - ASSERT_FALSE(sut->post().has_error()); - std::this_thread::sleep_for(std::chrono::nanoseconds(TIMING_TEST_TIMEOUT / 3 * 2)); - TIMING_TEST_EXPECT_TRUE(timedWaitFinish.load()); - - t.join(); -}); -} // namespace From 208586bda1c83de81c50a0db710fd842cc503963 Mon Sep 17 00:00:00 2001 From: Christian Eltzschig Date: Thu, 16 Jun 2022 17:58:49 +0200 Subject: [PATCH 03/17] iox-#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 --- .../cxx/functional_interface.hpp | 1 + .../internal/concurrent/periodic_task.hpp | 7 ++- .../internal/concurrent/periodic_task.inl | 10 +++-- .../posix_wrapper/named_pipe.hpp | 2 +- .../posix_wrapper/signal_watcher.hpp | 2 +- .../source/posix_wrapper/signal_watcher.cpp | 2 +- .../test_concurrent_smart_lock.cpp | 1 - .../test/moduletests/test_ipc_channel.cpp | 4 +- .../iceoryx_hoofs/testing/watch_dog.hpp | 14 +++--- .../popo/building_blocks/chunk_queue_data.hpp | 1 - .../condition_variable_data.hpp | 11 ++--- .../include/iceoryx_posh/roudi/roudi_app.hpp | 17 ++----- .../building_blocks/condition_listener.cpp | 23 +++------- .../building_blocks/condition_notifier.cpp | 8 ++-- .../condition_variable_data.cpp | 4 ++ .../roudi/application/iceoryx_roudi_app.cpp | 3 +- .../source/roudi/application/roudi_app.cpp | 44 ++----------------- .../moduletests/test_mepoo_typed_mempool.cpp | 43 ------------------ 18 files changed, 50 insertions(+), 147 deletions(-) diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/cxx/functional_interface.hpp b/iceoryx_hoofs/include/iceoryx_hoofs/cxx/functional_interface.hpp index e7569626f6..1bd97246d6 100644 --- 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 index e63c137295..0b676dc61c 100644 --- 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 @@ -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 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 index ac885b55a4..0e27833f5d 100644 --- 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::PeriodicTask(const PeriodicTaskManualStart_t, : m_callable(std::forward(args)...) , m_taskName(taskName) { + posix::UnnamedSemaphoreBuilder().initialValue(0U).isInterProcessCapable(false).create(m_stop).expect( + "Unable to create semaphore for periodic task"); } template @@ -62,7 +66,7 @@ inline void PeriodicTask::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::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 index 17448d5caa..9f4e189a51 100644 --- 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 index 180acaa852..eec3ca8cdc 100644 --- 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 index a8c5317b83..6003370e77 100644 --- 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 index 4710d04454..c52c90902e 100644 --- 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 index 23f3bffd3e..1b1f0190a9 100644 --- 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 index dc02d2f9e3..c722f1d707 100644 --- 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 #include @@ -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 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 index 21f8fcb6c0..a284d8bf63 100644 --- 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 index 203368dc58..8dba56533c 100644 --- 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 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 index ce1bfc89c7..358a7c9c4b 100644 --- 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 index 24e5d268a1..c697958e6e 100644 --- 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 index 1ca89b25d8..00953bd3df 100644 --- 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 index e31742a90d..dabbc2c813 100644 --- 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 index 61d819bb95..00eeea5267 100644 --- 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; } diff --git a/iceoryx_posh/source/roudi/application/roudi_app.cpp b/iceoryx_posh/source/roudi/application/roudi_app.cpp index 865907d2dc..2b04b12685 100644 --- a/iceoryx_posh/source/roudi/application/roudi_app.cpp +++ b/iceoryx_posh/source/roudi/application/roudi_app.cpp @@ -27,7 +27,7 @@ #include "iceoryx_hoofs/platform/semaphore.hpp" #include "iceoryx_hoofs/platform/signal.hpp" #include "iceoryx_hoofs/posix_wrapper/posix_access_rights.hpp" -#include "iceoryx_hoofs/posix_wrapper/signal_handler.hpp" +#include "iceoryx_hoofs/posix_wrapper/signal_watcher.hpp" #include "iceoryx_hoofs/posix_wrapper/thread.hpp" #include "iceoryx_posh/iceoryx_posh_types.hpp" #include "iceoryx_posh/internal/log/posh_logging.hpp" @@ -41,43 +41,6 @@ namespace iox { namespace roudi { -// using unnamed namespace to keep the functions in this translation unit -namespace -{ -iox::roudi::RouDiApp* g_RouDiApp; -cxx::optional g_signalGuardInt; -cxx::optional g_signalGuardTerm; -cxx::optional g_signalGuardHup; - -} // unnamed namespace - -void RouDiApp::roudiSigHandler(int32_t signal) noexcept -{ - if (g_RouDiApp) - { - if (signal == SIGHUP) - { - LogWarn() << "SIGHUP not supported by RouDi"; - } - // Post semaphore to exit - g_RouDiApp->m_semaphore.post().or_else([](auto) { - LogFatal() << "RouDi app semaphore seems corrupted. Unable to send termination signal."; - errorHandler(PoshError::ROUDI_APP__FAILED_TO_UNLOCK_SEMAPHORE_IN_SIG_HANDLER, ErrorLevel::FATAL); - }); - } -} - -void RouDiApp::registerSigHandler() noexcept -{ - // Save the pointer to self - g_RouDiApp = this; - - // register sigHandler for SIGINT, SIGTERM and SIGHUP - g_signalGuardInt.emplace(posix::registerSignalHandler(posix::Signal::INT, roudiSigHandler)); - g_signalGuardTerm.emplace(posix::registerSignalHandler(posix::Signal::TERM, roudiSigHandler)); - g_signalGuardHup.emplace(posix::registerSignalHandler(posix::Signal::HUP, roudiSigHandler)); -} - RouDiApp::RouDiApp(const config::CmdLineArgs_t& cmdLineArgs, const RouDiConfig_t& config) noexcept : m_logLevel(cmdLineArgs.logLevel) , m_monitoringMode(cmdLineArgs.monitoringMode) @@ -98,8 +61,6 @@ RouDiApp::RouDiApp(const config::CmdLineArgs_t& cmdLineArgs, const RouDiConfig_t { iox::log::LogManager::GetLogManager().SetDefaultLogLevel(m_logLevel); - registerSigHandler(); - LogVerbose() << "Command line parameters are:\n" << cmdLineArgs; } } @@ -126,7 +87,8 @@ bool RouDiApp::checkAndOptimizeConfig(const RouDiConfig_t& config) noexcept bool RouDiApp::waitForSignal() noexcept { - return !m_semaphore.wait().has_error(); + iox::posix::waitForTerminationRequest(); + return true; } } // namespace roudi diff --git a/iceoryx_posh/test/moduletests/test_mepoo_typed_mempool.cpp b/iceoryx_posh/test/moduletests/test_mepoo_typed_mempool.cpp index cae28a59a6..c6a18dca94 100644 --- a/iceoryx_posh/test/moduletests/test_mepoo_typed_mempool.cpp +++ b/iceoryx_posh/test/moduletests/test_mepoo_typed_mempool.cpp @@ -18,7 +18,6 @@ #include "iceoryx_posh/internal/mepoo/typed_mem_pool.hpp" #include "iceoryx_hoofs/internal/posix_wrapper/shared_memory_object/allocator.hpp" -#include "iceoryx_hoofs/posix_wrapper/semaphore.hpp" #include "test.hpp" @@ -98,46 +97,4 @@ TEST_F(TypedMemPool_test, OutOfChunksErrorWhenFull) EXPECT_THAT(object4.has_error(), Eq(true)); EXPECT_THAT(object4.get_error(), Eq(TypedMemPoolError::OutOfChunks)); } - -class TypedMemPool_Semaphore_test : public Test -{ - public: - static constexpr uint32_t NumberOfChunks{3}; - static constexpr uint32_t ChunkSize{sizeof(iox::posix::Semaphore)}; - - using FreeListIndex_t = MemPool::freeList_t::Index_t; - static constexpr FreeListIndex_t LoFFLiMemoryRequirement{ - MemPool::freeList_t::requiredIndexMemorySize(NumberOfChunks) + 100000}; - - TypedMemPool_Semaphore_test() - : allocator(m_rawMemory, NumberOfChunks * ChunkSize + LoFFLiMemoryRequirement) - , sut(NumberOfChunks, allocator, allocator) - { - } - - void SetUp(){}; - void TearDown(){}; - - alignas(MemPool::CHUNK_MEMORY_ALIGNMENT) uint8_t m_rawMemory[NumberOfChunks * ChunkSize + LoFFLiMemoryRequirement]; - iox::posix::Allocator allocator; - - TypedMemPool sut; -}; - -TEST_F(TypedMemPool_Semaphore_test, CreateValidSemaphore) -{ - ::testing::Test::RecordProperty("TEST_ID", "753fbb32-beec-4277-936f-2d6359a557ce"); - auto semaphorePtr = sut.createObjectWithCreationPattern( - iox::posix::CreateNamedSemaphore, "/fuuSem", S_IRUSR | S_IWUSR, 10); - EXPECT_THAT(semaphorePtr.has_error(), Eq(false)); -} - -TEST_F(TypedMemPool_Semaphore_test, CreateInvalidSemaphore) -{ - ::testing::Test::RecordProperty("TEST_ID", "410ff52d-f14d-41f3-a1e5-f164c5accbd7"); - auto semaphorePtr = sut.createObjectWithCreationPattern( - iox::posix::CreateNamedSemaphore, "", S_IRUSR | S_IWUSR, 10); - EXPECT_THAT(semaphorePtr.has_error(), Eq(true)); -} - } // namespace From 803f501c68102788251ca6c9c17b910db1862d3d Mon Sep 17 00:00:00 2001 From: Christian Eltzschig Date: Thu, 16 Jun 2022 18:30:36 +0200 Subject: [PATCH 04/17] iox-#751 Add wait_loop to adaptive_wait and use it in the tests for a more efficient busy loop Signed-off-by: Christian Eltzschig --- .../internal/cxx/adaptive_wait.hpp | 6 ++++ iceoryx_hoofs/source/cxx/adaptive_wait.cpp | 13 +++++++ .../moduletests/test_cxx_adaptive_wait.cpp | 35 +++++++++++++++++++ .../moduletests/test_posix_signal_watcher.cpp | 6 ++-- .../test_popo_chunk_distributor.cpp | 21 ++++++----- .../test_popo_condition_variable.cpp | 29 +++++++-------- 6 files changed, 81 insertions(+), 29 deletions(-) diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/adaptive_wait.hpp b/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/adaptive_wait.hpp index 33dcb83a12..694e173efc 100644 --- a/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/adaptive_wait.hpp +++ b/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/adaptive_wait.hpp @@ -17,8 +17,10 @@ #ifndef IOX_HOOFS_CXX_ADAPTIVE_WAIT_HPP #define IOX_HOOFS_CXX_ADAPTIVE_WAIT_HPP +#include "iceoryx_hoofs/cxx/function_ref.hpp" #include "iceoryx_hoofs/internal/units/duration.hpp" +#include #include namespace iox @@ -53,6 +55,10 @@ class adaptive_wait /// with exponential waiting times is pursued. void wait() noexcept; + /// @brief Waits in a loop in a smart wait until continueToWait returns false. + /// @param[in] continueToWait callable which returns if the wait should continue + void wait_loop(const function_ref& continueToWait) noexcept; + protected: /// @note All numbers are not accurate and are just rough estimates /// acquired by the experiments described below. diff --git a/iceoryx_hoofs/source/cxx/adaptive_wait.cpp b/iceoryx_hoofs/source/cxx/adaptive_wait.cpp index 94078d2f4b..dbda41cd4a 100644 --- a/iceoryx_hoofs/source/cxx/adaptive_wait.cpp +++ b/iceoryx_hoofs/source/cxx/adaptive_wait.cpp @@ -46,6 +46,19 @@ void adaptive_wait::wait() noexcept std::this_thread::sleep_for(FINAL_WAITING_TIME); } } + +void adaptive_wait::wait_loop(const function_ref& continueToWait) noexcept +{ + if (!continueToWait) + { + return; + } + + while (continueToWait()) + { + wait(); + } +} } // namespace internal } // namespace cxx } // namespace iox diff --git a/iceoryx_hoofs/test/moduletests/test_cxx_adaptive_wait.cpp b/iceoryx_hoofs/test/moduletests/test_cxx_adaptive_wait.cpp index da6ee14247..bb67c6b751 100644 --- a/iceoryx_hoofs/test/moduletests/test_cxx_adaptive_wait.cpp +++ b/iceoryx_hoofs/test/moduletests/test_cxx_adaptive_wait.cpp @@ -19,6 +19,8 @@ using namespace ::testing; #include "iceoryx_hoofs/internal/cxx/adaptive_wait.hpp" +#include + using namespace iox::cxx::internal; namespace @@ -74,4 +76,37 @@ TEST(AdaptiveWaitTest, waitWaitsAtLeastFINAL_WAITING_TIMEafterINITIAL_REPETITION std::chrono::nanoseconds(end - start).count(), Ge(iox::units::Duration::fromMilliseconds(AdaptiveWaitSut::FINAL_WAITING_TIME.count()).toNanoseconds())); } + +TEST(AdaptiveWaitTest, wait_loopWaitsAtLeastAsLongAsTheConditionsReturnsTrue) +{ + ::testing::Test::RecordProperty("TEST_ID", "c44e9315-fed4-4681-ba0c-2d25bce4459b"); + class AdaptiveWaitSut : public adaptive_wait + { + public: + using adaptive_wait::FINAL_WAITING_TIME; + using adaptive_wait::INITIAL_REPETITIONS; + }; + + std::atomic_bool continueToWait{true}; + std::atomic_bool threadIsStarted{false}; + std::thread waitThread{[&] { + threadIsStarted = true; + AdaptiveWaitSut().wait_loop([&] { return continueToWait.load(); }); + }}; + + while (!threadIsStarted.load()) + { + std::this_thread::yield(); + } + + auto start = std::chrono::steady_clock::now(); + const std::chrono::milliseconds waitTime(100); + std::this_thread::sleep_for(waitTime); + auto end = std::chrono::steady_clock::now(); + + continueToWait.store(false); + + EXPECT_THAT(std::chrono::nanoseconds(end - start).count(), + Ge(iox::units::Duration::fromMilliseconds(waitTime.count()).toNanoseconds())); +} } // namespace diff --git a/iceoryx_hoofs/test/moduletests/test_posix_signal_watcher.cpp b/iceoryx_hoofs/test/moduletests/test_posix_signal_watcher.cpp index ae0cbd51ed..eed8ae5311 100644 --- a/iceoryx_hoofs/test/moduletests/test_posix_signal_watcher.cpp +++ b/iceoryx_hoofs/test/moduletests/test_posix_signal_watcher.cpp @@ -14,6 +14,7 @@ // // SPDX-License-Identifier: Apache-2.0 +#include "iceoryx_hoofs/internal/cxx/adaptive_wait.hpp" #include "iceoryx_hoofs/posix_wrapper/signal_watcher.hpp" #include "iceoryx_hoofs/testing/watch_dog.hpp" #include "test.hpp" @@ -98,10 +99,7 @@ void unblocksWhenSignalWasRaisedForWaiters(SignalWatcher_test& test, }); } - while (isThreadStarted.load() != numberOfWaiters) - { - std::this_thread::yield(); - } + iox::cxx::internal::adaptive_wait().wait_loop([&] { return !isThreadStarted; }); std::this_thread::sleep_for(test.waitingTime); diff --git a/iceoryx_posh/test/moduletests/test_popo_chunk_distributor.cpp b/iceoryx_posh/test/moduletests/test_popo_chunk_distributor.cpp index 73f3ee7af1..164eb82e6c 100644 --- a/iceoryx_posh/test/moduletests/test_popo_chunk_distributor.cpp +++ b/iceoryx_posh/test/moduletests/test_popo_chunk_distributor.cpp @@ -620,16 +620,17 @@ TYPED_TEST(ChunkDistributor_test, DeliverToQueueWithBlockingOptionBlocksDelivery ASSERT_FALSE(sut.deliverToQueue(queueData->m_uniqueId, EXPECTED_QUEUE_INDEX, chunk).has_error()); } - auto threadSyncSemaphore = iox::posix::Semaphore::create(iox::posix::CreateUnnamedSingleProcessSemaphore, 0U); + std::atomic_bool isThreadStarted{false}; auto chunk = this->allocateChunk(7373); std::atomic_bool wasChunkDelivered{false}; std::thread t1([&] { - ASSERT_FALSE(threadSyncSemaphore->post().has_error()); + isThreadStarted = true; ASSERT_FALSE(sut.deliverToQueue(queueData->m_uniqueId, EXPECTED_QUEUE_INDEX, chunk).has_error()); wasChunkDelivered = true; }); - ASSERT_FALSE(threadSyncSemaphore->wait().has_error()); + iox::cxx::internal::adaptive_wait().wait_loop([&] { return !isThreadStarted; }); + std::this_thread::sleep_for(this->BLOCKING_DURATION); EXPECT_THAT(wasChunkDelivered.load(), Eq(false)); @@ -737,15 +738,16 @@ TYPED_TEST(ChunkDistributor_test, DeliverToSingleQueueBlocksWhenOptionsAreSetToB ASSERT_FALSE(sut.tryAddQueue(queueData.get(), 0U).has_error()); sut.deliverToAllStoredQueues(this->allocateChunk(155U)); - auto threadSyncSemaphore = iox::posix::Semaphore::create(iox::posix::CreateUnnamedSingleProcessSemaphore, 0U); + std::atomic_bool isThreadStarted{false}; std::atomic_bool wasChunkDelivered{false}; std::thread t1([&] { - ASSERT_FALSE(threadSyncSemaphore->post().has_error()); + isThreadStarted = true; sut.deliverToAllStoredQueues(this->allocateChunk(152U)); wasChunkDelivered = true; }); - ASSERT_FALSE(threadSyncSemaphore->wait().has_error()); + iox::cxx::internal::adaptive_wait().wait_loop([&] { return !isThreadStarted; }); + std::this_thread::sleep_for(this->BLOCKING_DURATION); EXPECT_THAT(wasChunkDelivered.load(), Eq(false)); @@ -783,15 +785,16 @@ TYPED_TEST(ChunkDistributor_test, MultipleBlockingQueuesWillBeFilledWhenThereBec sut.deliverToAllStoredQueues(this->allocateChunk(425U)); - auto threadSyncSemaphore = iox::posix::Semaphore::create(iox::posix::CreateUnnamedSingleProcessSemaphore, 0U); + std::atomic_bool isThreadStarted{false}; std::atomic_bool wasChunkDelivered{false}; std::thread t1([&] { - ASSERT_FALSE(threadSyncSemaphore->post().has_error()); + isThreadStarted.store(true); sut.deliverToAllStoredQueues(this->allocateChunk(1152U)); wasChunkDelivered = true; }); - ASSERT_FALSE(threadSyncSemaphore->wait().has_error()); + iox::cxx::internal::adaptive_wait().wait_loop([&] { return !isThreadStarted; }); + std::this_thread::sleep_for(this->BLOCKING_DURATION); EXPECT_THAT(wasChunkDelivered.load(), Eq(false)); diff --git a/iceoryx_posh/test/moduletests/test_popo_condition_variable.cpp b/iceoryx_posh/test/moduletests/test_popo_condition_variable.cpp index 257b4de325..5ebdd0ff6a 100644 --- a/iceoryx_posh/test/moduletests/test_popo_condition_variable.cpp +++ b/iceoryx_posh/test/moduletests/test_popo_condition_variable.cpp @@ -15,6 +15,7 @@ // // SPDX-License-Identifier: Apache-2.0 +#include "iceoryx_hoofs/internal/cxx/adaptive_wait.hpp" #include "iceoryx_hoofs/testing/timing_test.hpp" #include "iceoryx_hoofs/testing/watch_dog.hpp" #include "iceoryx_posh/internal/popo/building_blocks/condition_listener.hpp" @@ -58,8 +59,6 @@ class ConditionVariable_test : public Test } Watchdog m_watchdog{m_timeToWait}; - iox::posix::Semaphore m_syncSemaphore = - iox::posix::Semaphore::create(iox::posix::CreateUnnamedSingleProcessSemaphore, 0U).value(); }; TEST_F(ConditionVariable_test, ConditionListenerIsNeitherCopyNorMovable) @@ -118,13 +117,15 @@ TEST_F(ConditionVariable_test, WaitAndNotifyResultsInImmediateTriggerMultiThread { ::testing::Test::RecordProperty("TEST_ID", "39b40c73-3dcc-4af6-9682-b62816c69854"); std::atomic counter{0}; + std::atomic_bool isThreadStarted{false}; std::thread waiter([&] { EXPECT_THAT(counter, Eq(0)); - IOX_DISCARD_RESULT(m_syncSemaphore.post()); + isThreadStarted = true; m_waiter.wait(); EXPECT_THAT(counter, Eq(1)); }); - IOX_DISCARD_RESULT(m_syncSemaphore.wait()); + iox::cxx::internal::adaptive_wait().wait_loop([&] { return !isThreadStarted; }); + counter++; m_signaler.notify(); waiter.join(); @@ -235,8 +236,6 @@ TEST_F(ConditionVariable_test, TimedWaitReturnsAllNotifiedIndices) TIMING_TEST_F(ConditionVariable_test, TimedWaitBlocksUntilTimeout, Repeat(5), [&] { ConditionListener listener(m_condVarData); NotificationVector_t activeNotifications; - iox::posix::Semaphore threadSetupSemaphore = - iox::posix::Semaphore::create(iox::posix::CreateUnnamedSingleProcessSemaphore, 0U).value(); std::atomic_bool hasWaited{false}; std::thread waiter([&] { @@ -255,8 +254,6 @@ TIMING_TEST_F(ConditionVariable_test, TimedWaitBlocksUntilTimeout, Repeat(5), [& TIMING_TEST_F(ConditionVariable_test, TimedWaitBlocksUntilNotification, Repeat(5), [&] { ConditionListener listener(m_condVarData); NotificationVector_t activeNotifications; - iox::posix::Semaphore threadSetupSemaphore = - iox::posix::Semaphore::create(iox::posix::CreateUnnamedSingleProcessSemaphore, 0U).value(); std::atomic_bool hasWaited{false}; std::thread waiter([&] { @@ -369,19 +366,19 @@ TIMING_TEST_F(ConditionVariable_test, WaitBlocks, Repeat(5), [&] { ConditionNotifier notifier(m_condVarData, EVENT_INDEX); ConditionListener listener(m_condVarData); NotificationVector_t activeNotifications; - iox::posix::Semaphore threadSetupSemaphore = - iox::posix::Semaphore::create(iox::posix::CreateUnnamedSingleProcessSemaphore, 0U).value(); + std::atomic_bool isThreadStarted{false}; std::atomic_bool hasWaited{false}; std::thread waiter([&] { - IOX_DISCARD_RESULT(threadSetupSemaphore.post()); + isThreadStarted = true; activeNotifications = listener.wait(); hasWaited.store(true, std::memory_order_relaxed); ASSERT_THAT(activeNotifications.size(), Eq(1U)); EXPECT_THAT(activeNotifications[0], Eq(EVENT_INDEX)); }); - IOX_DISCARD_RESULT(threadSetupSemaphore.wait()); + iox::cxx::internal::adaptive_wait().wait_loop([&] { return !isThreadStarted; }); + std::this_thread::sleep_for(std::chrono::milliseconds(10)); EXPECT_THAT(hasWaited, Eq(false)); notifier.notify(); @@ -396,8 +393,6 @@ TIMING_TEST_F(ConditionVariable_test, SecondWaitBlocksUntilNewNotification, Repe ConditionNotifier notifier1(m_condVarData, FIRST_EVENT_INDEX); ConditionNotifier notifier2(m_condVarData, SECOND_EVENT_INDEX); ConditionListener listener(m_condVarData); - iox::posix::Semaphore threadSetupSemaphore = - iox::posix::Semaphore::create(iox::posix::CreateUnnamedSingleProcessSemaphore, 0U).value(); std::atomic_bool hasWaited{false}; Watchdog watchdogFirstWait(m_timeToWait); @@ -414,8 +409,9 @@ TIMING_TEST_F(ConditionVariable_test, SecondWaitBlocksUntilNewNotification, Repe Watchdog watchdogSecondWait(m_timeToWait); watchdogSecondWait.watchAndActOnFailure([&] { listener.destroy(); }); + std::atomic_bool isThreadStarted{false}; std::thread waiter([&] { - IOX_DISCARD_RESULT(threadSetupSemaphore.post()); + isThreadStarted = true; activeNotifications = listener.wait(); hasWaited.store(true, std::memory_order_relaxed); ASSERT_THAT(activeNotifications.size(), Eq(1U)); @@ -426,7 +422,8 @@ TIMING_TEST_F(ConditionVariable_test, SecondWaitBlocksUntilNewNotification, Repe } }); - IOX_DISCARD_RESULT(threadSetupSemaphore.wait()); + iox::cxx::internal::adaptive_wait().wait_loop([&] { return !isThreadStarted; }); + std::this_thread::sleep_for(std::chrono::milliseconds(10)); EXPECT_THAT(hasWaited, Eq(false)); notifier1.notify(); From 42d1bde478208a774b255338a15333d25f8a0fbb Mon Sep 17 00:00:00 2001 From: Christian Eltzschig Date: Thu, 16 Jun 2022 19:20:43 +0200 Subject: [PATCH 05/17] iox-#751 Replace ShutdownManager with SignalWatcher in dds gateway, replace semaphores in test with adaptive wait Signed-off-by: Christian Eltzschig --- .../test/moduletests/test_listener.cpp | 4 +- .../test/moduletests/test_wait_set.cpp | 6 +-- iceoryx_dds/source/gateway/main.cpp | 37 +------------------ ...ice_callbacks_listener_as_class_member.cpp | 1 - .../callbacks/ice_callbacks_subscriber.cpp | 1 - .../integrationtests/test_client_server.cpp | 12 +++--- ...est_publisher_subscriber_communication.cpp | 22 +++++------ .../test/moduletests/test_popo_listener.cpp | 13 ++++--- .../test/moduletests/test_posh_runtime.cpp | 19 +++++----- .../moduletests/test_roudi_portmanager.cpp | 6 +-- 10 files changed, 43 insertions(+), 78 deletions(-) diff --git a/iceoryx_binding_c/test/moduletests/test_listener.cpp b/iceoryx_binding_c/test/moduletests/test_listener.cpp index 1c37c1b205..b6ba72b3ed 100644 --- a/iceoryx_binding_c/test/moduletests/test_listener.cpp +++ b/iceoryx_binding_c/test/moduletests/test_listener.cpp @@ -441,7 +441,7 @@ void notifyClient(ClientPortData& portData) portData.m_connectionState = iox::ConnectionState::CONNECTED; iox::popo::ChunkQueuePusher pusher{&portData.m_chunkReceiverData}; pusher.push(iox::mepoo::SharedChunk()); - EXPECT_FALSE(portData.m_chunkReceiverData.m_conditionVariableDataPtr->m_semaphore.post().has_error()); + EXPECT_FALSE(portData.m_chunkReceiverData.m_conditionVariableDataPtr->semaphore->post().has_error()); } TIMING_TEST_F(iox_listener_test, NotifyingClientEventWorks, Repeat(5), [&] { @@ -491,7 +491,7 @@ void notifyServer(ServerPortData& portData) { iox::popo::ChunkQueuePusher pusher{&portData.m_chunkReceiverData}; pusher.push(iox::mepoo::SharedChunk()); - EXPECT_FALSE(portData.m_chunkReceiverData.m_conditionVariableDataPtr->m_semaphore.post().has_error()); + EXPECT_FALSE(portData.m_chunkReceiverData.m_conditionVariableDataPtr->semaphore->post().has_error()); } TEST_F(iox_listener_test, AttachingServerWorks) diff --git a/iceoryx_binding_c/test/moduletests/test_wait_set.cpp b/iceoryx_binding_c/test/moduletests/test_wait_set.cpp index 7e5754edc8..87b0a83004 100644 --- a/iceoryx_binding_c/test/moduletests/test_wait_set.cpp +++ b/iceoryx_binding_c/test/moduletests/test_wait_set.cpp @@ -796,7 +796,7 @@ void notifyClient(ClientPortData& portData) portData.m_connectionState = iox::ConnectionState::CONNECTED; iox::popo::ChunkQueuePusher pusher{&portData.m_chunkReceiverData}; pusher.push(iox::mepoo::SharedChunk()); - EXPECT_FALSE(portData.m_chunkReceiverData.m_conditionVariableDataPtr->m_semaphore.post().has_error()); + EXPECT_FALSE(portData.m_chunkReceiverData.m_conditionVariableDataPtr->semaphore->post().has_error()); } TEST_F(iox_ws_test, NotifyingClientEventWorks) @@ -928,7 +928,7 @@ void notifyServer(ServerPortData& portData) { iox::popo::ChunkQueuePusher pusher{&portData.m_chunkReceiverData}; pusher.push(iox::mepoo::SharedChunk()); - EXPECT_FALSE(portData.m_chunkReceiverData.m_conditionVariableDataPtr->m_semaphore.post().has_error()); + EXPECT_FALSE(portData.m_chunkReceiverData.m_conditionVariableDataPtr->semaphore->post().has_error()); } TEST_F(iox_ws_test, AttachingServerEventWorks) @@ -1140,7 +1140,7 @@ void notifyServiceDiscovery(SubscriberPortData& portData) { iox::popo::ChunkQueuePusher pusher{&portData.m_chunkReceiverData}; pusher.push(iox::mepoo::SharedChunk()); - EXPECT_FALSE(portData.m_chunkReceiverData.m_conditionVariableDataPtr->m_semaphore.post().has_error()); + EXPECT_FALSE(portData.m_chunkReceiverData.m_conditionVariableDataPtr->semaphore->post().has_error()); } TEST_F(iox_ws_test, NotifyingServiceDiscoveryEventWorks) diff --git a/iceoryx_dds/source/gateway/main.cpp b/iceoryx_dds/source/gateway/main.cpp index f3f49fbb7d..9ad5fa16aa 100644 --- a/iceoryx_dds/source/gateway/main.cpp +++ b/iceoryx_dds/source/gateway/main.cpp @@ -21,46 +21,13 @@ #include "iceoryx_hoofs/cxx/helplets.hpp" #include "iceoryx_hoofs/cxx/optional.hpp" #include "iceoryx_hoofs/platform/signal.hpp" -#include "iceoryx_hoofs/posix_wrapper/semaphore.hpp" -#include "iceoryx_hoofs/posix_wrapper/signal_handler.hpp" +#include "iceoryx_hoofs/posix_wrapper/signal_watcher.hpp" #include "iceoryx_posh/gateway/gateway_config.hpp" #include "iceoryx_posh/gateway/toml_gateway_config_parser.hpp" #include "iceoryx_posh/runtime/posh_runtime.hpp" -class ShutdownManager -{ - public: - static void scheduleShutdown(int num) - { - char reason = '\0'; - psignal(num, &reason); - s_semaphore.post().or_else([](auto) { - std::cerr << "failed to call post on shutdown semaphore" << std::endl; - std::terminate(); - }); - } - static void waitUntilShutdown() - { - s_semaphore.wait().or_else([](auto) { - std::cerr << "failed to call wait on shutdown semaphore" << std::endl; - std::terminate(); - }); - } - - private: - static iox::posix::Semaphore s_semaphore; - ShutdownManager() = default; -}; -iox::posix::Semaphore ShutdownManager::s_semaphore = - iox::posix::Semaphore::create(iox::posix::CreateUnnamedSingleProcessSemaphore, 0u).value(); - int main() { - // Set OS signal handlers - auto signalGuardInt = iox::posix::registerSignalHandler(iox::posix::Signal::INT, ShutdownManager::scheduleShutdown); - auto signalGuardTerm = - iox::posix::registerSignalHandler(iox::posix::Signal::TERM, ShutdownManager::scheduleShutdown); - // Start application iox::runtime::PoshRuntime::initRuntime("iox-dds-gateway"); @@ -84,7 +51,7 @@ int main() dds2ioxGateway.runMultithreaded(); // Run until SIGINT or SIGTERM - ShutdownManager::waitUntilShutdown(); + iox::posix::waitForTerminationRequest(); return 0; } diff --git a/iceoryx_examples/callbacks/ice_callbacks_listener_as_class_member.cpp b/iceoryx_examples/callbacks/ice_callbacks_listener_as_class_member.cpp index 07e4248a43..5d85c8612d 100644 --- a/iceoryx_examples/callbacks/ice_callbacks_listener_as_class_member.cpp +++ b/iceoryx_examples/callbacks/ice_callbacks_listener_as_class_member.cpp @@ -15,7 +15,6 @@ // SPDX-License-Identifier: Apache-2.0 #include "iceoryx_hoofs/cxx/optional.hpp" -#include "iceoryx_hoofs/posix_wrapper/semaphore.hpp" #include "iceoryx_hoofs/posix_wrapper/signal_watcher.hpp" #include "iceoryx_posh/popo/listener.hpp" #include "iceoryx_posh/popo/subscriber.hpp" diff --git a/iceoryx_examples/callbacks/ice_callbacks_subscriber.cpp b/iceoryx_examples/callbacks/ice_callbacks_subscriber.cpp index e2cd85e78b..a969f34a08 100644 --- a/iceoryx_examples/callbacks/ice_callbacks_subscriber.cpp +++ b/iceoryx_examples/callbacks/ice_callbacks_subscriber.cpp @@ -15,7 +15,6 @@ // SPDX-License-Identifier: Apache-2.0 #include "iceoryx_hoofs/cxx/optional.hpp" -#include "iceoryx_hoofs/posix_wrapper/semaphore.hpp" #include "iceoryx_hoofs/posix_wrapper/signal_watcher.hpp" #include "iceoryx_posh/popo/listener.hpp" #include "iceoryx_posh/popo/subscriber.hpp" diff --git a/iceoryx_posh/test/integrationtests/test_client_server.cpp b/iceoryx_posh/test/integrationtests/test_client_server.cpp index 1b9852aa2d..318bb82bb1 100644 --- a/iceoryx_posh/test/integrationtests/test_client_server.cpp +++ b/iceoryx_posh/test/integrationtests/test_client_server.cpp @@ -345,10 +345,10 @@ TEST_F(ClientServer_test, ServerTakeRequestUnblocksClientSendingRequest) ASSERT_TRUE(server.hasClients()); ASSERT_THAT(client.getConnectionState(), Eq(iox::ConnectionState::CONNECTED)); - auto threadSyncSemaphore = iox::posix::Semaphore::create(iox::posix::CreateUnnamedSingleProcessSemaphore, 0U); std::atomic_bool wasRequestSent{false}; // block in a separate thread + std::atomic_bool isThreadStarted{false}; std::thread blockingClient([&] { auto sendRequest = [&]() { auto loanResult = client.loan(); @@ -363,14 +363,14 @@ TEST_F(ClientServer_test, ServerTakeRequestUnblocksClientSendingRequest) } // signal that an blocking send is expected - ASSERT_FALSE(threadSyncSemaphore->post().has_error()); + isThreadStarted = true; sendRequest(); wasRequestSent = true; }); // wait some time to check if the client is blocked constexpr std::chrono::milliseconds SLEEP_TIME{100U}; - ASSERT_FALSE(threadSyncSemaphore->wait().has_error()); + iox::cxx::internal::adaptive_wait().wait_loop([&] { return !isThreadStarted.load(); }); std::this_thread::sleep_for(SLEEP_TIME); EXPECT_THAT(wasRequestSent.load(), Eq(false)); @@ -406,10 +406,10 @@ TEST_F(ClientServer_test, ClientTakesResponseUnblocksServerSendingResponse) EXPECT_FALSE(clientLoanResult.value().send().has_error()); } - auto threadSyncSemaphore = iox::posix::Semaphore::create(iox::posix::CreateUnnamedSingleProcessSemaphore, 0U); std::atomic_bool wasResponseSent{false}; // block in a separate thread + std::atomic_bool isThreadStarted{false}; std::thread blockingServer([&] { auto processRequest = [&]() { auto takeResult = server.take(); @@ -424,14 +424,14 @@ TEST_F(ClientServer_test, ClientTakesResponseUnblocksServerSendingResponse) processRequest(); } - ASSERT_FALSE(threadSyncSemaphore->post().has_error()); + isThreadStarted = true; processRequest(); wasResponseSent = true; }); // wait some time to check if the server is blocked constexpr std::chrono::milliseconds SLEEP_TIME{100U}; - ASSERT_FALSE(threadSyncSemaphore->wait().has_error()); + iox::cxx::internal::adaptive_wait().wait_loop([&] { return !isThreadStarted.load(); }); std::this_thread::sleep_for(SLEEP_TIME); EXPECT_THAT(wasResponseSent.load(), Eq(false)); diff --git a/iceoryx_posh/test/integrationtests/test_publisher_subscriber_communication.cpp b/iceoryx_posh/test/integrationtests/test_publisher_subscriber_communication.cpp index 57b6508c71..d07ee37da4 100644 --- a/iceoryx_posh/test/integrationtests/test_publisher_subscriber_communication.cpp +++ b/iceoryx_posh/test/integrationtests/test_publisher_subscriber_communication.cpp @@ -21,7 +21,6 @@ #include "iceoryx_hoofs/cxx/string.hpp" #include "iceoryx_hoofs/cxx/variant.hpp" #include "iceoryx_hoofs/cxx/vector.hpp" -#include "iceoryx_hoofs/posix_wrapper/semaphore.hpp" #include "iceoryx_hoofs/testing/watch_dog.hpp" #include "iceoryx_posh/popo/publisher.hpp" #include "iceoryx_posh/popo/subscriber.hpp" @@ -551,18 +550,17 @@ TEST_F(PublisherSubscriberCommunication_test, PublisherBlocksWhenBlockingActivat EXPECT_FALSE(publisher->publishCopyOf("start your day with a smile").has_error()); EXPECT_FALSE(publisher->publishCopyOf("and hypnotoad will smile back").has_error()); - auto threadSyncSemaphore = posix::Semaphore::create(posix::CreateUnnamedSingleProcessSemaphore, 0U); - std::atomic_bool wasSampleDelivered{false}; + std::atomic_bool isThreadStarted{false}; std::thread t1([&] { - ASSERT_FALSE(threadSyncSemaphore->post().has_error()); + isThreadStarted = true; EXPECT_FALSE(publisher->publishCopyOf("oh no hypnotoad is staring at me").has_error()); wasSampleDelivered.store(true); }); constexpr int64_t TIMEOUT_IN_MS = 100; - ASSERT_FALSE(threadSyncSemaphore->wait().has_error()); + iox::cxx::internal::adaptive_wait().wait_loop([&] { return !isThreadStarted.load(); }); std::this_thread::sleep_for(std::chrono::milliseconds(TIMEOUT_IN_MS)); EXPECT_FALSE(wasSampleDelivered.load()); @@ -595,16 +593,15 @@ TEST_F(PublisherSubscriberCommunication_test, PublisherDoesNotBlockAndDiscardsSa EXPECT_FALSE(publisher->publishCopyOf("first there was a blubb named mantua").has_error()); EXPECT_FALSE(publisher->publishCopyOf("second hypnotoad ate it").has_error()); - auto threadSyncSemaphore = posix::Semaphore::create(posix::CreateUnnamedSingleProcessSemaphore, 0U); - std::atomic_bool wasSampleDelivered{false}; + std::atomic_bool isThreadStarted{false}; std::thread t1([&] { - ASSERT_FALSE(threadSyncSemaphore->post().has_error()); + isThreadStarted = true; EXPECT_FALSE(publisher->publishCopyOf("third a tiny black hole smells like butter").has_error()); wasSampleDelivered.store(true); }); - ASSERT_FALSE(threadSyncSemaphore->wait().has_error()); + iox::cxx::internal::adaptive_wait().wait_loop([&] { return !isThreadStarted.load(); }); t1.join(); EXPECT_TRUE(wasSampleDelivered.load()); @@ -666,18 +663,17 @@ TEST_F(PublisherSubscriberCommunication_test, MixedOptionsSetupWorksWithBlocking EXPECT_FALSE(publisherBlocking->publishCopyOf("hypnotoad wants a cookie").has_error()); EXPECT_FALSE(publisherNonBlocking->publishCopyOf("hypnotoad has a sister named hypnoodle").has_error()); - auto threadSyncSemaphore = posix::Semaphore::create(posix::CreateUnnamedSingleProcessSemaphore, 0U); - std::atomic_bool wasSampleDelivered{false}; + std::atomic_bool isThreadStarted{false}; std::thread t1([&] { - ASSERT_FALSE(threadSyncSemaphore->post().has_error()); + isThreadStarted = true; EXPECT_FALSE(publisherBlocking->publishCopyOf("chucky is the only one who can ride the hypnotoad").has_error()); wasSampleDelivered.store(true); }); constexpr int64_t TIMEOUT_IN_MS = 100; - ASSERT_FALSE(threadSyncSemaphore->wait().has_error()); + iox::cxx::internal::adaptive_wait().wait_loop([&] { return !isThreadStarted.load(); }); std::this_thread::sleep_for(std::chrono::milliseconds(TIMEOUT_IN_MS)); EXPECT_FALSE(wasSampleDelivered.load()); diff --git a/iceoryx_posh/test/moduletests/test_popo_listener.cpp b/iceoryx_posh/test/moduletests/test_popo_listener.cpp index d3d260f008..1b982216ca 100644 --- a/iceoryx_posh/test/moduletests/test_popo_listener.cpp +++ b/iceoryx_posh/test/moduletests/test_popo_listener.cpp @@ -18,7 +18,7 @@ #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/posix_wrapper/unnamed_semaphore.hpp" #include "iceoryx_hoofs/testing/timing_test.hpp" #include "iceoryx_hoofs/testing/watch_dog.hpp" #include "iceoryx_posh/iceoryx_posh_types.hpp" @@ -156,7 +156,7 @@ iox::concurrent::smart_lock> g_toBeAttached; iox::concurrent::smart_lock> g_toBeDetached; std::array g_triggerCallbackArg; uint64_t g_triggerCallbackRuntimeInMs = 0U; -iox::cxx::optional g_callbackBlocker; +iox::cxx::optional g_callbackBlocker; class Listener_test : public Test { @@ -227,8 +227,11 @@ class Listener_test : public Test void activateTriggerCallbackBlocker() noexcept { - g_callbackBlocker.emplace( - iox::posix::Semaphore::create(iox::posix::CreateUnnamedSingleProcessSemaphore, 0U).value()); + iox::posix::UnnamedSemaphoreBuilder() + .initialValue(0U) + .isInterProcessCapable(false) + .create(g_callbackBlocker) + .expect("Unable to create callback blocker semaphore"); } void unblockTriggerCallback(const uint64_t numberOfUnblocks) noexcept @@ -1213,4 +1216,4 @@ TIMING_TEST_F(Listener_test, AttachingInCallbackWorks, Repeat(5), [&] { // END ////////////////////////////////// -} // namespace \ No newline at end of file +} // namespace diff --git a/iceoryx_posh/test/moduletests/test_posh_runtime.cpp b/iceoryx_posh/test/moduletests/test_posh_runtime.cpp index b7d31fd18a..684ad92089 100644 --- a/iceoryx_posh/test/moduletests/test_posh_runtime.cpp +++ b/iceoryx_posh/test/moduletests/test_posh_runtime.cpp @@ -16,6 +16,7 @@ // SPDX-License-Identifier: Apache-2.0 #include "iceoryx_hoofs/cxx/convert.hpp" +#include "iceoryx_hoofs/internal/cxx/adaptive_wait.hpp" #include "iceoryx_hoofs/testing/timing_test.hpp" #include "iceoryx_hoofs/testing/watch_dog.hpp" #include "iceoryx_posh/iceoryx_posh_types.hpp" @@ -949,7 +950,6 @@ TEST_F(PoshRuntime_test, ShutdownUnblocksBlockingPublisher) // send samples to fill subscriber queue ASSERT_FALSE(publisher.publishCopyOf(42U).has_error()); - auto threadSyncSemaphore = iox::posix::Semaphore::create(iox::posix::CreateUnnamedSingleProcessSemaphore, 0U); std::atomic_bool wasSampleSent{false}; constexpr iox::units::Duration DEADLOCK_TIMEOUT{5_s}; @@ -957,15 +957,16 @@ TEST_F(PoshRuntime_test, ShutdownUnblocksBlockingPublisher) deadlockWatchdog.watchAndActOnFailure([] { std::terminate(); }); // block in a separate thread + std::atomic_bool isThreadStarted{false}; std::thread blockingPublisher([&] { - ASSERT_FALSE(threadSyncSemaphore->post().has_error()); + isThreadStarted = true; ASSERT_FALSE(publisher.publishCopyOf(42U).has_error()); wasSampleSent = true; }); // wait some time to check if the publisher is blocked + iox::cxx::internal::adaptive_wait().wait_loop([&] { return !isThreadStarted.load(); }); constexpr std::chrono::milliseconds SLEEP_TIME{100U}; - ASSERT_FALSE(threadSyncSemaphore->wait().has_error()); std::this_thread::sleep_for(SLEEP_TIME); EXPECT_THAT(wasSampleSent.load(), Eq(false)); @@ -997,7 +998,6 @@ TEST_F(PoshRuntime_test, ShutdownUnblocksBlockingClient) ASSERT_TRUE(server.hasClients()); ASSERT_THAT(client.getConnectionState(), Eq(iox::ConnectionState::CONNECTED)); - auto threadSyncSemaphore = iox::posix::Semaphore::create(iox::posix::CreateUnnamedSingleProcessSemaphore, 0U); std::atomic_bool wasRequestSent{false}; constexpr iox::units::Duration DEADLOCK_TIMEOUT{5_s}; @@ -1005,6 +1005,7 @@ TEST_F(PoshRuntime_test, ShutdownUnblocksBlockingClient) deadlockWatchdog.watchAndActOnFailure([] { std::terminate(); }); // block in a separate thread + std::atomic_bool isThreadStarted{false}; std::thread blockingClient([&] { auto sendRequest = [&](bool expectError) { auto clientLoanResult = client.loan(sizeof(uint64_t), alignof(uint64_t)); @@ -1025,7 +1026,7 @@ TEST_F(PoshRuntime_test, ShutdownUnblocksBlockingClient) } // signal that an blocking send is expected - ASSERT_FALSE(threadSyncSemaphore->post().has_error()); + isThreadStarted = true; constexpr bool EXPECT_ERROR_INDICATOR{true}; sendRequest(EXPECT_ERROR_INDICATOR); wasRequestSent = true; @@ -1033,7 +1034,7 @@ TEST_F(PoshRuntime_test, ShutdownUnblocksBlockingClient) // wait some time to check if the client is blocked constexpr std::chrono::milliseconds SLEEP_TIME{100U}; - ASSERT_FALSE(threadSyncSemaphore->wait().has_error()); + iox::cxx::internal::adaptive_wait().wait_loop([&] { return !isThreadStarted.load(); }); std::this_thread::sleep_for(SLEEP_TIME); EXPECT_THAT(wasRequestSent.load(), Eq(false)); @@ -1073,7 +1074,6 @@ TEST_F(PoshRuntime_test, ShutdownUnblocksBlockingServer) EXPECT_FALSE(client.send(clientLoanResult.value()).has_error()); } - auto threadSyncSemaphore = iox::posix::Semaphore::create(iox::posix::CreateUnnamedSingleProcessSemaphore, 0U); std::atomic_bool wasResponseSent{false}; constexpr iox::units::Duration DEADLOCK_TIMEOUT{5_s}; @@ -1081,6 +1081,7 @@ TEST_F(PoshRuntime_test, ShutdownUnblocksBlockingServer) deadlockWatchdog.watchAndActOnFailure([] { std::terminate(); }); // block in a separate thread + std::atomic_bool isThreadStarted{false}; std::thread blockingServer([&] { auto processRequest = [&](bool expectError) { auto takeResult = server.take(); @@ -1102,7 +1103,7 @@ TEST_F(PoshRuntime_test, ShutdownUnblocksBlockingServer) processRequest(EXPECT_ERROR_INDICATOR); } - ASSERT_FALSE(threadSyncSemaphore->post().has_error()); + isThreadStarted = true; constexpr bool EXPECT_ERROR_INDICATOR{true}; processRequest(EXPECT_ERROR_INDICATOR); wasResponseSent = true; @@ -1110,7 +1111,7 @@ TEST_F(PoshRuntime_test, ShutdownUnblocksBlockingServer) // wait some time to check if the server is blocked constexpr std::chrono::milliseconds SLEEP_TIME{100U}; - ASSERT_FALSE(threadSyncSemaphore->wait().has_error()); + iox::cxx::internal::adaptive_wait().wait_loop([&] { return !isThreadStarted.load(); }); std::this_thread::sleep_for(SLEEP_TIME); EXPECT_THAT(wasResponseSent.load(), Eq(false)); diff --git a/iceoryx_posh/test/moduletests/test_roudi_portmanager.cpp b/iceoryx_posh/test/moduletests/test_roudi_portmanager.cpp index 4b897f54b1..0f99b35f4f 100644 --- a/iceoryx_posh/test/moduletests/test_roudi_portmanager.cpp +++ b/iceoryx_posh/test/moduletests/test_roudi_portmanager.cpp @@ -835,7 +835,6 @@ void PortManager_test::setupAndTestBlockingPublisher(const iox::RuntimeName_t& p ASSERT_FALSE(maybeChunk.has_error()); publisher.sendChunk(maybeChunk.value()); - auto threadSyncSemaphore = iox::posix::Semaphore::create(iox::posix::CreateUnnamedSingleProcessSemaphore, 0U); std::atomic_bool wasChunkSent{false}; constexpr iox::units::Duration DEADLOCK_TIMEOUT{5_s}; @@ -843,17 +842,18 @@ void PortManager_test::setupAndTestBlockingPublisher(const iox::RuntimeName_t& p deadlockWatchdog.watchAndActOnFailure([] { std::terminate(); }); // block in a separate thread + std::atomic_bool isThreadStarted{false}; std::thread blockingPublisher([&] { auto maybeChunk = publisher.tryAllocateChunk(42U, 8U); ASSERT_FALSE(maybeChunk.has_error()); - ASSERT_FALSE(threadSyncSemaphore->post().has_error()); + isThreadStarted = true; publisher.sendChunk(maybeChunk.value()); wasChunkSent = true; }); // wait some time to check if the publisher is blocked constexpr int64_t SLEEP_IN_MS = 100; - ASSERT_FALSE(threadSyncSemaphore->wait().has_error()); + iox::cxx::internal::adaptive_wait().wait_loop([&] { return !isThreadStarted.load(); }); std::this_thread::sleep_for(std::chrono::milliseconds(SLEEP_IN_MS)); EXPECT_THAT(wasChunkSent.load(), Eq(false)); From 1164205f249cbafdc7e102806691fd448e5394fd Mon Sep 17 00:00:00 2001 From: Christian Eltzschig Date: Thu, 16 Jun 2022 19:33:33 +0200 Subject: [PATCH 06/17] iox-#751 Add code examples and feature note to release notes Signed-off-by: Christian Eltzschig --- .../release-notes/iceoryx-unreleased.md | 57 +++++++++++++++++++ .../internal/cxx/adaptive_wait.hpp | 1 - .../internal/cxx/functional_interface.inl | 4 +- .../moduletests/test_cxx_adaptive_wait.cpp | 2 + 4 files changed, 61 insertions(+), 3 deletions(-) diff --git a/doc/website/release-notes/iceoryx-unreleased.md b/doc/website/release-notes/iceoryx-unreleased.md index 8d6d2ba787..4cde701812 100644 --- a/doc/website/release-notes/iceoryx-unreleased.md +++ b/doc/website/release-notes/iceoryx-unreleased.md @@ -17,6 +17,7 @@ - Refactor semaphore [\#751](https://github.com/eclipse-iceoryx/iceoryx/issues/751) - Introduce `UnnamedSemaphore` - Introduce `NamedSemaphore` + - Remove old `Semaphore` - Extend `concatenate`, `operator+`, `unsafe_append` and `append` of `iox::cxx::string` for chars [\#208](https://github.com/eclipse-iceoryx/iceoryx/issues/208) - Extend `unsafe_append` and `append` methods of `iox::cxx::string` for `std::string` [\#208](https://github.com/eclipse-iceoryx/iceoryx/issues/208) - The iceoryx development environment supports multiple running docker containers [\#1410](https://github.com/eclipse-iceoryx/iceoryx/issues/1410) @@ -79,3 +80,59 @@ // after #include "iceoryx_hoofs/design_pattern/builder.hpp" ``` + +3. Replace `Semaphore` with `NamedSemaphore` and `UnnamedSemaphore` + + ```cpp + // before + #include "iceoryx_hoofs/posix_wrapper/semaphore.hpp" + + // named semaphore + auto semaphore = iox::posix::Semaphore::create(iox::posix::CreateNamedSemaphore, + "mySemaphoreName", + S_IRUSR | S_IWUSR, + 0); + + // unnamed semaphore + auto semaphore = iox::posix::Semaphore::create(iox::posix::CreateUnnamedSingleProcessSemaphore, 0); + + // after + // named semaphore + #include "iceoryx_hoofs/posix_wrapper/named_semaphore.hpp" + + iox::cxx::optional semaphore; + auto result = iox::posix::NamedSemaphoreBuilder() + .name("mySemaphoreName") + .openMode(iox::posix::OpenMode::OPEN_OR_CREATE) + .permissions(iox::cxx::perms::owner_all) + .initialValue(0U) + .create(semaphore); + + // unnamed semaphore + #include "iceoryx_hoofs/posix_wrapper/unnamed_semaphore.hpp" + + iox::cxx::optional semaphore; + auto result = iox::posix::UnnamedSemaphoreBuilder() + .initialValue(0U) + .isInterProcessCapable(true) + .create(semaphore); + ``` + +4. `RoudiApp::waitForSignal` is deprecated + ```cpp + // before + // in my custom roudi app implementation + uint8_t MyCustomRoudiApp::run() noexcept { + // ... + + waitForSignal(); + } + + // after + // in my custom roudi app implementation + uint8_t MyCustomRoudiApp::run() noexcept { + // ... + + iox::posix::waitForTerminationRequest(); + } + ``` diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/adaptive_wait.hpp b/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/adaptive_wait.hpp index 694e173efc..17c4093792 100644 --- a/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/adaptive_wait.hpp +++ b/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/adaptive_wait.hpp @@ -20,7 +20,6 @@ #include "iceoryx_hoofs/cxx/function_ref.hpp" #include "iceoryx_hoofs/internal/units/duration.hpp" -#include #include namespace iox diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/functional_interface.inl b/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/functional_interface.inl index 3e5967cc68..eed508f15d 100644 --- a/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/functional_interface.inl +++ b/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/functional_interface.inl @@ -37,7 +37,7 @@ inline void Expect::expect(const StringType& msg) const noexcept if (!(*static_cast(this))) { - // it is possible that expect is called inside an error handler therefore we + // it is possible that expect is called inside a signal handler therefore we // use write IOX_DISCARD_RESULT(write(STDERR_FILENO, &msg[0], strlen(&msg[0]))); Ensures(false); @@ -55,7 +55,7 @@ inline ValueType& ExpectWithValue::expect(const StringType& if (!(*derivedThis)) { - // it is possible that expect is called inside an error handler therefore we + // it is possible that expect is called inside a signal handler therefore we // use write IOX_DISCARD_RESULT(write(STDERR_FILENO, &msg[0], strlen(&msg[0]))); Ensures(false); diff --git a/iceoryx_hoofs/test/moduletests/test_cxx_adaptive_wait.cpp b/iceoryx_hoofs/test/moduletests/test_cxx_adaptive_wait.cpp index bb67c6b751..9c6d8018d6 100644 --- a/iceoryx_hoofs/test/moduletests/test_cxx_adaptive_wait.cpp +++ b/iceoryx_hoofs/test/moduletests/test_cxx_adaptive_wait.cpp @@ -108,5 +108,7 @@ TEST(AdaptiveWaitTest, wait_loopWaitsAtLeastAsLongAsTheConditionsReturnsTrue) EXPECT_THAT(std::chrono::nanoseconds(end - start).count(), Ge(iox::units::Duration::fromMilliseconds(waitTime.count()).toNanoseconds())); + + waitThread.join(); } } // namespace From 3b371aaf0ded74696931a350bb6b233388652b01 Mon Sep 17 00:00:00 2001 From: Christian Eltzschig Date: Thu, 16 Jun 2022 19:42:21 +0200 Subject: [PATCH 07/17] iox-#751 Remove unused SemaphoreError enum values Signed-off-by: Christian Eltzschig --- .../internal/posix_wrapper/semaphore_interface.hpp | 4 ---- iceoryx_hoofs/test/moduletests/test_cxx_adaptive_wait.cpp | 1 + 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/semaphore_interface.hpp b/iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/semaphore_interface.hpp index 95a3cbc946..5b7b5310ea 100644 --- a/iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/semaphore_interface.hpp +++ b/iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/semaphore_interface.hpp @@ -26,13 +26,9 @@ namespace posix { enum class SemaphoreError { - CREATION_FAILED, - NAME_TOO_LONG, - UNABLE_TO_OPEN_HANDLE, INVALID_SEMAPHORE_HANDLE, SEMAPHORE_OVERFLOW, INTERRUPTED_BY_SIGNAL_HANDLER, - INVALID_NAME, PERMISSION_DENIED, ALREADY_EXIST, FILE_DESCRIPTOR_LIMIT_REACHED, diff --git a/iceoryx_hoofs/test/moduletests/test_cxx_adaptive_wait.cpp b/iceoryx_hoofs/test/moduletests/test_cxx_adaptive_wait.cpp index 9c6d8018d6..4e756d8bb2 100644 --- a/iceoryx_hoofs/test/moduletests/test_cxx_adaptive_wait.cpp +++ b/iceoryx_hoofs/test/moduletests/test_cxx_adaptive_wait.cpp @@ -19,6 +19,7 @@ using namespace ::testing; #include "iceoryx_hoofs/internal/cxx/adaptive_wait.hpp" +#include #include using namespace iox::cxx::internal; From af640e153e66e5a0d5dfc6872a91d4c0993ab34e Mon Sep 17 00:00:00 2001 From: Christian Eltzschig Date: Thu, 16 Jun 2022 19:50:44 +0200 Subject: [PATCH 08/17] iox-#751 the platform IOX_SEM_VALUE_MAX is a constexpr to ensure type safety, fix gcc warning unused value Signed-off-by: Christian Eltzschig --- .../iceoryx_hoofs/internal/cxx/functional_interface.inl | 6 ++++-- .../internal/posix_wrapper/semaphore_interface.hpp | 1 + .../include/iceoryx_hoofs/posix_wrapper/named_pipe.hpp | 2 +- .../linux/include/iceoryx_hoofs/platform/semaphore.hpp | 4 +++- .../mac/include/iceoryx_hoofs/platform/semaphore.hpp | 3 ++- .../qnx/include/iceoryx_hoofs/platform/semaphore.hpp | 3 ++- .../unix/include/iceoryx_hoofs/platform/semaphore.hpp | 3 ++- .../win/include/iceoryx_hoofs/platform/semaphore.hpp | 3 ++- 8 files changed, 17 insertions(+), 8 deletions(-) diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/functional_interface.inl b/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/functional_interface.inl index eed508f15d..81630fc550 100644 --- a/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/functional_interface.inl +++ b/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/functional_interface.inl @@ -39,7 +39,8 @@ inline void Expect::expect(const StringType& msg) const noexcept { // it is possible that expect is called inside a signal handler therefore we // use write - IOX_DISCARD_RESULT(write(STDERR_FILENO, &msg[0], strlen(&msg[0]))); + auto result = write(STDERR_FILENO, &msg[0], strlen(&msg[0])); + IOX_DISCARD_RESULT(result); Ensures(false); } } @@ -57,7 +58,8 @@ inline ValueType& ExpectWithValue::expect(const StringType& { // it is possible that expect is called inside a signal handler therefore we // use write - IOX_DISCARD_RESULT(write(STDERR_FILENO, &msg[0], strlen(&msg[0]))); + auto result = write(STDERR_FILENO, &msg[0], strlen(&msg[0])); + IOX_DISCARD_RESULT(result); Ensures(false); } diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/semaphore_interface.hpp b/iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/semaphore_interface.hpp index 5b7b5310ea..acc0bac828 100644 --- a/iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/semaphore_interface.hpp +++ b/iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/semaphore_interface.hpp @@ -26,6 +26,7 @@ namespace posix { enum class SemaphoreError { + INVALID_NAME, INVALID_SEMAPHORE_HANDLE, SEMAPHORE_OVERFLOW, INTERRUPTED_BY_SIGNAL_HANDLER, diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/named_pipe.hpp b/iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/named_pipe.hpp index 9f4e189a51..b8a7dd6b0c 100644 --- a/iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/named_pipe.hpp +++ b/iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/named_pipe.hpp @@ -39,7 +39,7 @@ class NamedPipe : public DesignPattern::Creation static constexpr uint64_t MAX_MESSAGE_SIZE = 4U * 1024U; static constexpr uint32_t MAX_NUMBER_OF_MESSAGES = 10U; static_assert( - MAX_NUMBER_OF_MESSAGES < 51, + MAX_NUMBER_OF_MESSAGES < IOX_SEM_VALUE_MAX, "The maximum number of supported messages must be less or equal to the maximum allowed semaphore value"); static constexpr uint64_t NULL_TERMINATOR_SIZE = 0U; diff --git a/iceoryx_hoofs/platform/linux/include/iceoryx_hoofs/platform/semaphore.hpp b/iceoryx_hoofs/platform/linux/include/iceoryx_hoofs/platform/semaphore.hpp index cbb0790932..2f3abeb5f2 100644 --- a/iceoryx_hoofs/platform/linux/include/iceoryx_hoofs/platform/semaphore.hpp +++ b/iceoryx_hoofs/platform/linux/include/iceoryx_hoofs/platform/semaphore.hpp @@ -17,12 +17,14 @@ #ifndef IOX_HOOFS_LINUX_PLATFORM_SEMAPHORE_HPP #define IOX_HOOFS_LINUX_PLATFORM_SEMAPHORE_HPP +#include +#include #include using iox_sem_t = sem_t; #define IOX_SEM_FAILED SEM_FAILED -#define IOX_SEM_VALUE_MAX SEM_VALUE_MAX +constexpr uint32_t IOX_SEM_VALUE_MAX = SEM_VALUE_MAX; inline int iox_sem_getvalue(iox_sem_t* sem, int* sval) { diff --git a/iceoryx_hoofs/platform/mac/include/iceoryx_hoofs/platform/semaphore.hpp b/iceoryx_hoofs/platform/mac/include/iceoryx_hoofs/platform/semaphore.hpp index 13677af2ca..899c886279 100644 --- a/iceoryx_hoofs/platform/mac/include/iceoryx_hoofs/platform/semaphore.hpp +++ b/iceoryx_hoofs/platform/mac/include/iceoryx_hoofs/platform/semaphore.hpp @@ -18,11 +18,12 @@ #define IOX_HOOFS_MAC_PLATFORM_SEMAPHORE_HPP #include +#include #include #include #define IOX_SEM_FAILED static_cast(nullptr) -#define IOX_SEM_VALUE_MAX SEM_VALUE_MAX +constexpr uint32_t IOX_SEM_VALUE_MAX = SEM_VALUE_MAX; struct iox_sem_t { diff --git a/iceoryx_hoofs/platform/qnx/include/iceoryx_hoofs/platform/semaphore.hpp b/iceoryx_hoofs/platform/qnx/include/iceoryx_hoofs/platform/semaphore.hpp index 27331fdf02..341e025541 100644 --- a/iceoryx_hoofs/platform/qnx/include/iceoryx_hoofs/platform/semaphore.hpp +++ b/iceoryx_hoofs/platform/qnx/include/iceoryx_hoofs/platform/semaphore.hpp @@ -17,12 +17,13 @@ #ifndef IOX_HOOFS_QNX_PLATFORM_SEMAPHORE_HPP #define IOX_HOOFS_QNX_PLATFORM_SEMAPHORE_HPP +#include #include using iox_sem_t = sem_t; #define IOX_SEM_FAILED SEM_FAILED -#define IOX_SEM_VALUE_MAX SEM_VALUE_MAX +constexpr uint32_t IOX_SEM_VALUE_MAX = SEM_VALUE_MAX; inline int iox_sem_getvalue(iox_sem_t* sem, int* sval) { diff --git a/iceoryx_hoofs/platform/unix/include/iceoryx_hoofs/platform/semaphore.hpp b/iceoryx_hoofs/platform/unix/include/iceoryx_hoofs/platform/semaphore.hpp index 10f2e17c3a..34a2031025 100644 --- a/iceoryx_hoofs/platform/unix/include/iceoryx_hoofs/platform/semaphore.hpp +++ b/iceoryx_hoofs/platform/unix/include/iceoryx_hoofs/platform/semaphore.hpp @@ -17,12 +17,13 @@ #ifndef IOX_HOOFS_UNIX_PLATFORM_SEMAPHORE_HPP #define IOX_HOOFS_UNIX_PLATFORM_SEMAPHORE_HPP +#include #include using iox_sem_t = sem_t; #define IOX_SEM_FAILED SEM_FAILED -#define IOX_SEM_VALUE_MAX SEM_VALUE_MAX +constexpr uint32_t IOX_SEM_VALUE_MAX = SEM_VALUE_MAX; inline int iox_sem_getvalue(iox_sem_t* sem, int* sval) { diff --git a/iceoryx_hoofs/platform/win/include/iceoryx_hoofs/platform/semaphore.hpp b/iceoryx_hoofs/platform/win/include/iceoryx_hoofs/platform/semaphore.hpp index c7616bb589..2d9020b5d0 100644 --- a/iceoryx_hoofs/platform/win/include/iceoryx_hoofs/platform/semaphore.hpp +++ b/iceoryx_hoofs/platform/win/include/iceoryx_hoofs/platform/semaphore.hpp @@ -24,6 +24,7 @@ #include "iceoryx_hoofs/platform/win32_errorHandling.hpp" #include "iceoryx_hoofs/platform/windows.hpp" +#include #include #include #include @@ -34,7 +35,7 @@ #define IOX_SEM_FAILED static_cast(nullptr) // win32 API page talks about maximum allowed value without defining it or how to obtain. // We use the IOX_SEM_VALUE_MAX from linux which is INT_MAX -#define IOX_SEM_VALUE_MAX INT_MAX +constexpr uint32_t IOX_SEM_VALUE_MAX = INT_MAX; struct iox_sem_t { From 1a15a5459ccfe5e60589f98e3f26841560773092 Mon Sep 17 00:00:00 2001 From: Christian Eltzschig Date: Mon, 20 Jun 2022 20:30:03 +0200 Subject: [PATCH 09/17] iox-#751 Release note simplified, m_ added for consistency reasons, fix issue in SignalWatcher test Signed-off-by: Christian Eltzschig --- .../release-notes/iceoryx-unreleased.md | 81 ++++++++++--------- .../moduletests/test_posix_signal_watcher.cpp | 2 +- .../condition_variable_data.hpp | 2 +- .../building_blocks/condition_listener.cpp | 4 +- .../building_blocks/condition_notifier.cpp | 2 +- .../roudi/application/iceoryx_roudi_app.cpp | 2 +- 6 files changed, 48 insertions(+), 45 deletions(-) diff --git a/doc/website/release-notes/iceoryx-unreleased.md b/doc/website/release-notes/iceoryx-unreleased.md index 4cde701812..5b626fbe5e 100644 --- a/doc/website/release-notes/iceoryx-unreleased.md +++ b/doc/website/release-notes/iceoryx-unreleased.md @@ -81,58 +81,61 @@ #include "iceoryx_hoofs/design_pattern/builder.hpp" ``` -3. Replace `Semaphore` with `NamedSemaphore` and `UnnamedSemaphore` +3. `UnnamedSemaphore` replaces `Semaphore` with `CreateUnnamed*` option ```cpp // before - #include "iceoryx_hoofs/posix_wrapper/semaphore.hpp" + #include "iceoryx_hoofs/posix_wrapper/semaphore.hpp" - // named semaphore - auto semaphore = iox::posix::Semaphore::create(iox::posix::CreateNamedSemaphore, - "mySemaphoreName", - S_IRUSR | S_IWUSR, - 0); + auto semaphore = iox::posix::Semaphore::create(iox::posix::CreateUnnamedSingleProcessSemaphore, 0); + + // after + #include "iceoryx_hoofs/posix_wrapper/unnamed_semaphore.hpp" + + iox::cxx::optional semaphore; + auto result = iox::posix::UnnamedSemaphoreBuilder() + .initialValue(0U) + .isInterProcessCapable(true) + .create(semaphore); + ``` - // unnamed semaphore - auto semaphore = iox::posix::Semaphore::create(iox::posix::CreateUnnamedSingleProcessSemaphore, 0); +4. `NamedSemaphore` replaces `Semaphore` with `CreateNamedSemaphore` option + ```cpp + // before + #include "iceoryx_hoofs/posix_wrapper/semaphore.hpp" + + auto semaphore = iox::posix::Semaphore::create(iox::posix::CreateNamedSemaphore, + "mySemaphoreName", + S_IRUSR | S_IWUSR, + 0); // after - // named semaphore - #include "iceoryx_hoofs/posix_wrapper/named_semaphore.hpp" - - iox::cxx::optional semaphore; - auto result = iox::posix::NamedSemaphoreBuilder() - .name("mySemaphoreName") - .openMode(iox::posix::OpenMode::OPEN_OR_CREATE) - .permissions(iox::cxx::perms::owner_all) - .initialValue(0U) - .create(semaphore); - - // unnamed semaphore - #include "iceoryx_hoofs/posix_wrapper/unnamed_semaphore.hpp" - - iox::cxx::optional semaphore; - auto result = iox::posix::UnnamedSemaphoreBuilder() - .initialValue(0U) - .isInterProcessCapable(true) - .create(semaphore); + #include "iceoryx_hoofs/posix_wrapper/named_semaphore.hpp" + + iox::cxx::optional semaphore; + auto result = iox::posix::NamedSemaphoreBuilder() + .name("mySemaphoreName") + .openMode(iox::posix::OpenMode::OPEN_OR_CREATE) + .permissions(iox::cxx::perms::owner_all) + .initialValue(0U) + .create(semaphore); ``` -4. `RoudiApp::waitForSignal` is deprecated +5. `RoudiApp::waitForSignal` is deprecated ```cpp // before - // in my custom roudi app implementation - uint8_t MyCustomRoudiApp::run() noexcept { - // ... + //// in my custom roudi app implementation + uint8_t MyCustomRoudiApp::run() noexcept { + // ... - waitForSignal(); - } + waitForSignal(); + } // after - // in my custom roudi app implementation - uint8_t MyCustomRoudiApp::run() noexcept { - // ... + //// in my custom roudi app implementation + uint8_t MyCustomRoudiApp::run() noexcept { + // ... - iox::posix::waitForTerminationRequest(); - } + iox::posix::waitForTerminationRequest(); + } ``` diff --git a/iceoryx_hoofs/test/moduletests/test_posix_signal_watcher.cpp b/iceoryx_hoofs/test/moduletests/test_posix_signal_watcher.cpp index eed8ae5311..3a9d2ffc5f 100644 --- a/iceoryx_hoofs/test/moduletests/test_posix_signal_watcher.cpp +++ b/iceoryx_hoofs/test/moduletests/test_posix_signal_watcher.cpp @@ -99,7 +99,7 @@ void unblocksWhenSignalWasRaisedForWaiters(SignalWatcher_test& test, }); } - iox::cxx::internal::adaptive_wait().wait_loop([&] { return !isThreadStarted; }); + iox::cxx::internal::adaptive_wait().wait_loop([&] { return (isThreadStarted != numberOfWaiters); }); std::this_thread::sleep_for(test.waitingTime); 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 index 8dba56533c..b3f106258f 100644 --- 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 @@ -42,7 +42,7 @@ struct ConditionVariableData RuntimeName_t m_runtimeName; std::atomic_bool m_toBeDestroyed{false}; std::atomic_bool m_activeNotifications[MAX_NUMBER_OF_NOTIFIERS]; - std::atomic_bool wasNotified{false}; + std::atomic_bool m_wasNotified{false}; }; } // namespace popo diff --git a/iceoryx_posh/source/popo/building_blocks/condition_listener.cpp b/iceoryx_posh/source/popo/building_blocks/condition_listener.cpp index c697958e6e..8c01d2c1af 100644 --- a/iceoryx_posh/source/popo/building_blocks/condition_listener.cpp +++ b/iceoryx_posh/source/popo/building_blocks/condition_listener.cpp @@ -53,7 +53,7 @@ void ConditionListener::destroy() noexcept bool ConditionListener::wasNotified() const noexcept { - return getMembers()->wasNotified.load(std::memory_order_relaxed); + return getMembers()->m_wasNotified.load(std::memory_order_relaxed); } ConditionListener::NotificationVector_t ConditionListener::wait() noexcept @@ -110,7 +110,7 @@ ConditionListener::NotificationVector_t ConditionListener::waitImpl(const cxx::f void ConditionListener::reset(const uint64_t index) noexcept { getMembers()->m_activeNotifications[index].store(false, std::memory_order_relaxed); - getMembers()->wasNotified.store(false, std::memory_order_relaxed); + getMembers()->m_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 index 00953bd3df..2a7fdf3ded 100644 --- a/iceoryx_posh/source/popo/building_blocks/condition_notifier.cpp +++ b/iceoryx_posh/source/popo/building_blocks/condition_notifier.cpp @@ -37,7 +37,7 @@ ConditionNotifier::ConditionNotifier(ConditionVariableData& condVarDataRef, cons void ConditionNotifier::notify() noexcept { getMembers()->m_activeNotifications[m_notificationIndex].store(true, std::memory_order_release); - getMembers()->wasNotified.store(true, std::memory_order_relaxed); + getMembers()->m_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/roudi/application/iceoryx_roudi_app.cpp b/iceoryx_posh/source/roudi/application/iceoryx_roudi_app.cpp index 00eeea5267..718bf7697c 100644 --- a/iceoryx_posh/source/roudi/application/iceoryx_roudi_app.cpp +++ b/iceoryx_posh/source/roudi/application/iceoryx_roudi_app.cpp @@ -1,5 +1,5 @@ // Copyright (c) 2019 - 2020 by Robert Bosch GmbH. All rights reserved. -// Copyright (c) 2019 - 2020 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. From 394f231edfb71e51222e01d69f6b908215898c1f Mon Sep 17 00:00:00 2001 From: Christian Eltzschig Date: Tue, 21 Jun 2022 11:20:36 +0200 Subject: [PATCH 10/17] iox-#751 .expect prints error message with LogFatal, SignalWatcher made signal safe and added comments to explain signal safe nature Signed-off-by: Christian Eltzschig --- iceoryx_hoofs/CMakeLists.txt | 1 + .../cxx/functional_interface.hpp | 2 ++ .../internal/cxx/functional_interface.inl | 10 ++---- .../source/cxx/functional_interface.cpp | 34 +++++++++++++++++++ .../source/posix_wrapper/signal_watcher.cpp | 15 ++++++-- 5 files changed, 52 insertions(+), 10 deletions(-) create mode 100644 iceoryx_hoofs/source/cxx/functional_interface.cpp diff --git a/iceoryx_hoofs/CMakeLists.txt b/iceoryx_hoofs/CMakeLists.txt index 0de0e6aa33..44e11847f8 100644 --- a/iceoryx_hoofs/CMakeLists.txt +++ b/iceoryx_hoofs/CMakeLists.txt @@ -51,6 +51,7 @@ iox_add_library( source/cxx/adaptive_wait.cpp source/cxx/deadline_timer.cpp source/cxx/filesystem.cpp + source/cxx/functional_interface.cpp source/cxx/helplets.cpp source/cxx/requires.cpp source/cxx/unique_id.cpp diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/cxx/functional_interface.hpp b/iceoryx_hoofs/include/iceoryx_hoofs/cxx/functional_interface.hpp index 1bd97246d6..e83d4f501e 100644 --- a/iceoryx_hoofs/include/iceoryx_hoofs/cxx/functional_interface.hpp +++ b/iceoryx_hoofs/include/iceoryx_hoofs/cxx/functional_interface.hpp @@ -56,6 +56,8 @@ struct HasGetErrorMethod().g { }; +void print_expect_message(const char* message) noexcept; + template struct Expect { diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/functional_interface.inl b/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/functional_interface.inl index 81630fc550..01e591da0f 100644 --- a/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/functional_interface.inl +++ b/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/functional_interface.inl @@ -37,10 +37,7 @@ inline void Expect::expect(const StringType& msg) const noexcept if (!(*static_cast(this))) { - // it is possible that expect is called inside a signal handler therefore we - // use write - auto result = write(STDERR_FILENO, &msg[0], strlen(&msg[0])); - IOX_DISCARD_RESULT(result); + print_expect_message(&msg[0]); Ensures(false); } } @@ -56,10 +53,7 @@ inline ValueType& ExpectWithValue::expect(const StringType& if (!(*derivedThis)) { - // it is possible that expect is called inside a signal handler therefore we - // use write - auto result = write(STDERR_FILENO, &msg[0], strlen(&msg[0])); - IOX_DISCARD_RESULT(result); + print_expect_message(&msg[0]); Ensures(false); } diff --git a/iceoryx_hoofs/source/cxx/functional_interface.cpp b/iceoryx_hoofs/source/cxx/functional_interface.cpp new file mode 100644 index 0000000000..5d43eaf654 --- /dev/null +++ b/iceoryx_hoofs/source/cxx/functional_interface.cpp @@ -0,0 +1,34 @@ +// Copyright (c) 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. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +#include "iceoryx_hoofs/cxx/functional_interface.hpp" +#include "iceoryx_hoofs/internal/log/hoofs_logging.hpp" + +namespace iox +{ +namespace cxx +{ +namespace internal +{ +void print_expect_message(const char* message) noexcept +{ + // print_expect_message is only called from expect. expect allows only + // cxx::string or char arrays which are both correctly null terminated + LogFatal() << message; +} +} // namespace internal +} // namespace cxx +} // namespace iox diff --git a/iceoryx_hoofs/source/posix_wrapper/signal_watcher.cpp b/iceoryx_hoofs/source/posix_wrapper/signal_watcher.cpp index 6003370e77..69eb2f4547 100644 --- a/iceoryx_hoofs/source/posix_wrapper/signal_watcher.cpp +++ b/iceoryx_hoofs/source/posix_wrapper/signal_watcher.cpp @@ -15,6 +15,7 @@ // SPDX-License-Identifier: Apache-2.0 #include "iceoryx_hoofs/posix_wrapper/signal_watcher.hpp" #include "iceoryx_hoofs/cxx/helplets.hpp" +#include "iceoryx_hoofs/internal/log/hoofs_logging.hpp" #include "iceoryx_hoofs/platform/unistd.hpp" namespace iox @@ -29,7 +30,14 @@ void internalSignalHandler(int) noexcept for (uint64_t remainingNumberOfWaiters = instance.m_numberOfWaiters.load(); remainingNumberOfWaiters > 0; --remainingNumberOfWaiters) { - instance.m_semaphore->post().expect("Unable to increment semaphore in signal handler"); + if (instance.m_semaphore->post().has_error()) + { + // we use write since internalSignalHandler can be called from within a + // signal handler and write is signal safe + constexpr const char MSG[] = "Unable to increment semaphore in signal handler"; + auto result = write(STDERR_FILENO, &MSG[0], strlen(&MSG[0])); + IOX_DISCARD_RESULT(result); + } } } @@ -40,6 +48,10 @@ SignalWatcher::SignalWatcher() noexcept UnnamedSemaphoreBuilder() .isInterProcessCapable(false) .create(m_semaphore) + + // This can be safely used despite getInstance is used in the internalSignalHandler + // since this object has to be created first before internalSignalHandler can be called. + // The only way this object can be created is by calling getInstance. .expect("Unable to create semaphore for signal watcher"); } @@ -74,6 +86,5 @@ bool hasTerminationRequested() noexcept { return SignalWatcher::getInstance().wasSignalTriggered(); } - } // namespace posix } // namespace iox From 4866c4bfcb6f71130297347bb05e432bd95200d0 Mon Sep 17 00:00:00 2001 From: Christian Eltzschig Date: Tue, 21 Jun 2022 19:13:22 +0200 Subject: [PATCH 11/17] iox-#751 Fix rpc c example by using iox_ws_mark_for_destruction Signed-off-by: Christian Eltzschig --- .../request_response_in_c/client_c_waitset.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/iceoryx_examples/request_response_in_c/client_c_waitset.c b/iceoryx_examples/request_response_in_c/client_c_waitset.c index 9095d3ec3e..1420516d4c 100644 --- a/iceoryx_examples/request_response_in_c/client_c_waitset.c +++ b/iceoryx_examples/request_response_in_c/client_c_waitset.c @@ -18,6 +18,7 @@ #include "iceoryx_binding_c/request_header.h" #include "iceoryx_binding_c/response_header.h" #include "iceoryx_binding_c/runtime.h" +#include "iceoryx_binding_c/user_trigger.h" #include "iceoryx_binding_c/wait_set.h" #include "request_and_response_c_types.h" #include "sleep_for.h" @@ -32,10 +33,14 @@ bool keepRunning = true; const char APP_NAME[] = "iox-c-request-response-client-waitset"; +iox_ws_t waitset; +iox_ws_storage_t waitsetStorage; + void sigHandler(int signalValue) { (void)signalValue; keepRunning = false; + iox_ws_mark_for_destruction(waitset); } int main() @@ -54,7 +59,6 @@ int main() int64_t expectedResponseSequenceId = requestSequenceId; //! [create waitset and attach client] - iox_ws_storage_t waitsetStorage; iox_ws_t waitset = iox_ws_init(&waitsetStorage); if (iox_ws_attach_client_state(waitset, client, ClientState_HAS_RESPONSE, 0U, NULL) != WaitSetResult_SUCCESS) @@ -132,8 +136,11 @@ int main() } //! [process responses] - const uint32_t SLEEP_TIME_IN_MS = 950U; - sleep_for(SLEEP_TIME_IN_MS); + if (keepRunning) + { + const uint32_t SLEEP_TIME_IN_MS = 950U; + sleep_for(SLEEP_TIME_IN_MS); + } } //! [cleanup] From 7a32fdd1f8f6aa5c31b8ffc99654ae7412eb6de3 Mon Sep 17 00:00:00 2001 From: Christian Eltzschig Date: Thu, 23 Jun 2022 11:19:36 +0200 Subject: [PATCH 12/17] iox-#751 Fix rpc c++ example by signalling waitset SIGTERM Signed-off-by: Christian Eltzschig --- .../request_response/client_cxx_waitset.cpp | 23 +++++++++++++++---- .../request_response_in_c/client_c_waitset.c | 9 +++----- .../posix_wrapper/unnamed_semaphore.cpp | 2 +- 3 files changed, 22 insertions(+), 12 deletions(-) diff --git a/iceoryx_examples/request_response/client_cxx_waitset.cpp b/iceoryx_examples/request_response/client_cxx_waitset.cpp index 88704092d4..9c00a1f017 100644 --- a/iceoryx_examples/request_response/client_cxx_waitset.cpp +++ b/iceoryx_examples/request_response/client_cxx_waitset.cpp @@ -17,16 +17,19 @@ //! [iceoryx includes] #include "request_and_response_types.hpp" +#include "iceoryx_hoofs/posix_wrapper/signal_handler.hpp" #include "iceoryx_hoofs/posix_wrapper/signal_watcher.hpp" #include "iceoryx_posh/popo/client.hpp" #include "iceoryx_posh/popo/wait_set.hpp" #include "iceoryx_posh/runtime/posh_runtime.hpp" //! [iceoryx includes] +#include #include constexpr char APP_NAME[] = "iox-cpp-request-response-client-waitset"; - +std::atomic_bool keepRunning = {true}; +iox::cxx::optional> waitset; //! [context data to store Fibonacci numbers and sequence ids] struct ContextData @@ -38,8 +41,17 @@ struct ContextData }; //! [context data to store Fibonacci numbers and sequence ids] +void signalHandler(int) +{ + keepRunning = false; + waitset.and_then([&](auto& w) { w.markForDestruction(); }); +} + int main() { + auto sigTermGuard = iox::posix::registerSignalHandler(iox::posix::Signal::TERM, signalHandler); + auto sigIntGuard = iox::posix::registerSignalHandler(iox::posix::Signal::INT, signalHandler); + //! [initialize runtime] iox::runtime::PoshRuntime::initRuntime(APP_NAME); //! [initialize runtime] @@ -47,7 +59,7 @@ int main() ContextData ctx; //! [create waitset] - iox::popo::WaitSet<> waitset; + waitset.emplace(); //! [create client] iox::popo::ClientOptions options; @@ -56,14 +68,14 @@ int main() //! [create client] // attach client to waitset - waitset.attachState(client, iox::popo::ClientState::HAS_RESPONSE).or_else([](auto) { + waitset->attachState(client, iox::popo::ClientState::HAS_RESPONSE).or_else([](auto) { std::cerr << "failed to attach client" << std::endl; std::exit(EXIT_FAILURE); }); //! [create waitset] //! [mainloop] - while (!iox::posix::hasTerminationRequested()) + while (keepRunning) { //! [send request] client.loan() @@ -84,7 +96,7 @@ int main() // We block and wait for samples to arrive, when the time is up we send the request again //! [wait and check if the client triggered] - auto notificationVector = waitset.timedWait(iox::units::Duration::fromSeconds(5)); + auto notificationVector = waitset->timedWait(iox::units::Duration::fromSeconds(5)); for (auto& notification : notificationVector) { @@ -117,6 +129,7 @@ int main() } //! [mainloop] + waitset.reset(); std::cout << "shutting down" << std::endl; return (EXIT_SUCCESS); diff --git a/iceoryx_examples/request_response_in_c/client_c_waitset.c b/iceoryx_examples/request_response_in_c/client_c_waitset.c index 1420516d4c..5c6e8df157 100644 --- a/iceoryx_examples/request_response_in_c/client_c_waitset.c +++ b/iceoryx_examples/request_response_in_c/client_c_waitset.c @@ -59,7 +59,7 @@ int main() int64_t expectedResponseSequenceId = requestSequenceId; //! [create waitset and attach client] - iox_ws_t waitset = iox_ws_init(&waitsetStorage); + waitset = iox_ws_init(&waitsetStorage); if (iox_ws_attach_client_state(waitset, client, ClientState_HAS_RESPONSE, 0U, NULL) != WaitSetResult_SUCCESS) { @@ -136,11 +136,8 @@ int main() } //! [process responses] - if (keepRunning) - { - const uint32_t SLEEP_TIME_IN_MS = 950U; - sleep_for(SLEEP_TIME_IN_MS); - } + const uint32_t SLEEP_TIME_IN_MS = 950U; + sleep_for(SLEEP_TIME_IN_MS); } //! [cleanup] diff --git a/iceoryx_hoofs/source/posix_wrapper/unnamed_semaphore.cpp b/iceoryx_hoofs/source/posix_wrapper/unnamed_semaphore.cpp index 342a5a9b1c..3ff469a414 100644 --- a/iceoryx_hoofs/source/posix_wrapper/unnamed_semaphore.cpp +++ b/iceoryx_hoofs/source/posix_wrapper/unnamed_semaphore.cpp @@ -37,7 +37,7 @@ UnnamedSemaphoreBuilder::create(cxx::optional& uninitializedSe uninitializedSemaphore.emplace(); auto result = posixCall(iox_sem_init)(&uninitializedSemaphore.value().m_handle, - (m_isInterProcessCapable) ? 0 : 1, + (m_isInterProcessCapable) ? 1 : 0, static_cast(m_initialValue)) .failureReturnValue(-1) .evaluate(); From 3513d5f001f828c03c0b0ba08e360022519c2a8a Mon Sep 17 00:00:00 2001 From: Christian Eltzschig Date: Thu, 23 Jun 2022 18:02:55 +0200 Subject: [PATCH 13/17] iox-#751 Renaming, update copyright year, improve code readability Signed-off-by: Christian Eltzschig --- .../test/moduletests/test_listener.cpp | 4 ++-- .../test/moduletests/test_wait_set.cpp | 6 +++--- .../request_response_in_c/client_c_waitset.c | 1 + .../iceoryx_hoofs/cxx/functional_interface.hpp | 1 - .../iceoryx_hoofs/posix_wrapper/named_pipe.hpp | 5 ++--- .../source/posix_wrapper/signal_watcher.cpp | 3 ++- .../include/iceoryx_hoofs/testing/watch_dog.hpp | 2 +- .../popo/building_blocks/condition_listener.hpp | 2 +- .../building_blocks/condition_variable_data.hpp | 4 ++-- .../popo/building_blocks/condition_listener.cpp | 14 +++++++------- .../popo/building_blocks/condition_notifier.cpp | 4 ++-- .../building_blocks/condition_variable_data.cpp | 4 ++-- .../moduletests/test_popo_condition_variable.cpp | 2 +- 13 files changed, 26 insertions(+), 26 deletions(-) diff --git a/iceoryx_binding_c/test/moduletests/test_listener.cpp b/iceoryx_binding_c/test/moduletests/test_listener.cpp index b6ba72b3ed..58ae7deaa4 100644 --- a/iceoryx_binding_c/test/moduletests/test_listener.cpp +++ b/iceoryx_binding_c/test/moduletests/test_listener.cpp @@ -441,7 +441,7 @@ void notifyClient(ClientPortData& portData) portData.m_connectionState = iox::ConnectionState::CONNECTED; iox::popo::ChunkQueuePusher pusher{&portData.m_chunkReceiverData}; pusher.push(iox::mepoo::SharedChunk()); - EXPECT_FALSE(portData.m_chunkReceiverData.m_conditionVariableDataPtr->semaphore->post().has_error()); + EXPECT_FALSE(portData.m_chunkReceiverData.m_conditionVariableDataPtr->m_semaphore->post().has_error()); } TIMING_TEST_F(iox_listener_test, NotifyingClientEventWorks, Repeat(5), [&] { @@ -491,7 +491,7 @@ void notifyServer(ServerPortData& portData) { iox::popo::ChunkQueuePusher pusher{&portData.m_chunkReceiverData}; pusher.push(iox::mepoo::SharedChunk()); - EXPECT_FALSE(portData.m_chunkReceiverData.m_conditionVariableDataPtr->semaphore->post().has_error()); + EXPECT_FALSE(portData.m_chunkReceiverData.m_conditionVariableDataPtr->m_semaphore->post().has_error()); } TEST_F(iox_listener_test, AttachingServerWorks) diff --git a/iceoryx_binding_c/test/moduletests/test_wait_set.cpp b/iceoryx_binding_c/test/moduletests/test_wait_set.cpp index 87b0a83004..07f6c39767 100644 --- a/iceoryx_binding_c/test/moduletests/test_wait_set.cpp +++ b/iceoryx_binding_c/test/moduletests/test_wait_set.cpp @@ -796,7 +796,7 @@ void notifyClient(ClientPortData& portData) portData.m_connectionState = iox::ConnectionState::CONNECTED; iox::popo::ChunkQueuePusher pusher{&portData.m_chunkReceiverData}; pusher.push(iox::mepoo::SharedChunk()); - EXPECT_FALSE(portData.m_chunkReceiverData.m_conditionVariableDataPtr->semaphore->post().has_error()); + EXPECT_FALSE(portData.m_chunkReceiverData.m_conditionVariableDataPtr->m_semaphore->post().has_error()); } TEST_F(iox_ws_test, NotifyingClientEventWorks) @@ -928,7 +928,7 @@ void notifyServer(ServerPortData& portData) { iox::popo::ChunkQueuePusher pusher{&portData.m_chunkReceiverData}; pusher.push(iox::mepoo::SharedChunk()); - EXPECT_FALSE(portData.m_chunkReceiverData.m_conditionVariableDataPtr->semaphore->post().has_error()); + EXPECT_FALSE(portData.m_chunkReceiverData.m_conditionVariableDataPtr->m_semaphore->post().has_error()); } TEST_F(iox_ws_test, AttachingServerEventWorks) @@ -1140,7 +1140,7 @@ void notifyServiceDiscovery(SubscriberPortData& portData) { iox::popo::ChunkQueuePusher pusher{&portData.m_chunkReceiverData}; pusher.push(iox::mepoo::SharedChunk()); - EXPECT_FALSE(portData.m_chunkReceiverData.m_conditionVariableDataPtr->semaphore->post().has_error()); + EXPECT_FALSE(portData.m_chunkReceiverData.m_conditionVariableDataPtr->m_semaphore->post().has_error()); } TEST_F(iox_ws_test, NotifyingServiceDiscoveryEventWorks) diff --git a/iceoryx_examples/request_response_in_c/client_c_waitset.c b/iceoryx_examples/request_response_in_c/client_c_waitset.c index 5c6e8df157..0c53e79905 100644 --- a/iceoryx_examples/request_response_in_c/client_c_waitset.c +++ b/iceoryx_examples/request_response_in_c/client_c_waitset.c @@ -27,6 +27,7 @@ #include #include #include +#include #define NUMBER_OF_NOTIFICATIONS 1 diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/cxx/functional_interface.hpp b/iceoryx_hoofs/include/iceoryx_hoofs/cxx/functional_interface.hpp index e83d4f501e..a3f759da41 100644 --- a/iceoryx_hoofs/include/iceoryx_hoofs/cxx/functional_interface.hpp +++ b/iceoryx_hoofs/include/iceoryx_hoofs/cxx/functional_interface.hpp @@ -21,7 +21,6 @@ #include "iceoryx_hoofs/cxx/type_traits.hpp" #include "iceoryx_hoofs/platform/unistd.hpp" -#include #include namespace iox diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/named_pipe.hpp b/iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/named_pipe.hpp index b8a7dd6b0c..72ba6e83c9 100644 --- a/iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/named_pipe.hpp +++ b/iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/named_pipe.hpp @@ -38,9 +38,8 @@ class NamedPipe : public DesignPattern::Creation // increased as long as there is enough memory available static constexpr uint64_t MAX_MESSAGE_SIZE = 4U * 1024U; static constexpr uint32_t MAX_NUMBER_OF_MESSAGES = 10U; - static_assert( - MAX_NUMBER_OF_MESSAGES < IOX_SEM_VALUE_MAX, - "The maximum number of supported messages must be less or equal to the maximum allowed semaphore value"); + static_assert(MAX_NUMBER_OF_MESSAGES < IOX_SEM_VALUE_MAX, + "The maximum number of supported messages must be less to the maximum allowed semaphore value"); static constexpr uint64_t NULL_TERMINATOR_SIZE = 0U; static constexpr units::Duration CYCLE_TIME = units::Duration::fromMilliseconds(10); diff --git a/iceoryx_hoofs/source/posix_wrapper/signal_watcher.cpp b/iceoryx_hoofs/source/posix_wrapper/signal_watcher.cpp index 69eb2f4547..bc74599e69 100644 --- a/iceoryx_hoofs/source/posix_wrapper/signal_watcher.cpp +++ b/iceoryx_hoofs/source/posix_wrapper/signal_watcher.cpp @@ -35,8 +35,9 @@ void internalSignalHandler(int) noexcept // we use write since internalSignalHandler can be called from within a // signal handler and write is signal safe constexpr const char MSG[] = "Unable to increment semaphore in signal handler"; - auto result = write(STDERR_FILENO, &MSG[0], strlen(&MSG[0])); + auto result = write(STDERR_FILENO, MSG, strlen(MSG)); IOX_DISCARD_RESULT(result); + std::abort(); } } } diff --git a/iceoryx_hoofs/testing/include/iceoryx_hoofs/testing/watch_dog.hpp b/iceoryx_hoofs/testing/include/iceoryx_hoofs/testing/watch_dog.hpp index c722f1d707..7d922f923c 100644 --- a/iceoryx_hoofs/testing/include/iceoryx_hoofs/testing/watch_dog.hpp +++ b/iceoryx_hoofs/testing/include/iceoryx_hoofs/testing/watch_dog.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_posh/include/iceoryx_posh/internal/popo/building_blocks/condition_listener.hpp b/iceoryx_posh/include/iceoryx_posh/internal/popo/building_blocks/condition_listener.hpp index 0e3ff5c027..22eac79490 100644 --- a/iceoryx_posh/include/iceoryx_posh/internal/popo/building_blocks/condition_listener.hpp +++ b/iceoryx_posh/include/iceoryx_posh/internal/popo/building_blocks/condition_listener.hpp @@ -69,7 +69,7 @@ class ConditionListener ConditionVariableData* getMembers() noexcept; private: - void reset(const uint64_t index) noexcept; + void resetUnchecked(const uint64_t index) noexcept; void resetSemaphore() noexcept; NotificationVector_t waitImpl(const cxx::function_ref& waitCall) noexcept; 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 index b3f106258f..e44ab6e910 100644 --- 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 @@ -1,5 +1,5 @@ // Copyright (c) 2020 by Robert Bosch GmbH. All rights reserved. -// 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. @@ -38,7 +38,7 @@ struct ConditionVariableData ConditionVariableData& operator=(ConditionVariableData&& rhs) = delete; ~ConditionVariableData() noexcept = default; - cxx::optional semaphore; + cxx::optional m_semaphore; RuntimeName_t m_runtimeName; std::atomic_bool m_toBeDestroyed{false}; std::atomic_bool m_activeNotifications[MAX_NUMBER_OF_NOTIFIERS]; diff --git a/iceoryx_posh/source/popo/building_blocks/condition_listener.cpp b/iceoryx_posh/source/popo/building_blocks/condition_listener.cpp index 8c01d2c1af..6b0fc5d6cf 100644 --- a/iceoryx_posh/source/popo/building_blocks/condition_listener.cpp +++ b/iceoryx_posh/source/popo/building_blocks/condition_listener.cpp @@ -1,5 +1,5 @@ // Copyright (c) 2020 by Robert Bosch GmbH. All rights reserved. -// 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. @@ -33,7 +33,7 @@ void ConditionListener::resetSemaphore() noexcept bool hasFatalError = false; while (!hasFatalError && getMembers() - ->semaphore->tryWait() + ->m_semaphore->tryWait() .or_else([&](posix::SemaphoreError) { errorHandler(PoshError::POPO__CONDITION_LISTENER_SEMAPHORE_CORRUPTED_IN_RESET, ErrorLevel::FATAL); hasFatalError = true; @@ -46,7 +46,7 @@ void ConditionListener::resetSemaphore() noexcept void ConditionListener::destroy() noexcept { m_toBeDestroyed.store(true, std::memory_order_relaxed); - getMembers()->semaphore->post().or_else([](auto) { + getMembers()->m_semaphore->post().or_else([](auto) { errorHandler(PoshError::POPO__CONDITION_LISTENER_SEMAPHORE_CORRUPTED_IN_DESTROY, ErrorLevel::FATAL); }); } @@ -59,7 +59,7 @@ bool ConditionListener::wasNotified() const noexcept ConditionListener::NotificationVector_t ConditionListener::wait() noexcept { return waitImpl([this]() -> bool { - if (this->getMembers()->semaphore->wait().has_error()) + if (this->getMembers()->m_semaphore->wait().has_error()) { errorHandler(PoshError::POPO__CONDITION_LISTENER_SEMAPHORE_CORRUPTED_IN_WAIT, ErrorLevel::FATAL); return false; @@ -71,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()->semaphore->timedWait(timeToWait).has_error()) + if (this->getMembers()->m_semaphore->timedWait(timeToWait).has_error()) { errorHandler(PoshError::POPO__CONDITION_LISTENER_SEMAPHORE_CORRUPTED_IN_TIMED_WAIT, ErrorLevel::FATAL); } @@ -92,7 +92,7 @@ ConditionListener::NotificationVector_t ConditionListener::waitImpl(const cxx::f { if (getMembers()->m_activeNotifications[i].load(std::memory_order_relaxed)) { - reset(i); + resetUnchecked(i); activeNotifications.emplace_back(i); } } @@ -107,7 +107,7 @@ ConditionListener::NotificationVector_t ConditionListener::waitImpl(const cxx::f return activeNotifications; } -void ConditionListener::reset(const uint64_t index) noexcept +void ConditionListener::resetUnchecked(const uint64_t index) noexcept { getMembers()->m_activeNotifications[index].store(false, std::memory_order_relaxed); getMembers()->m_wasNotified.store(false, std::memory_order_relaxed); diff --git a/iceoryx_posh/source/popo/building_blocks/condition_notifier.cpp b/iceoryx_posh/source/popo/building_blocks/condition_notifier.cpp index 2a7fdf3ded..cca8cff7cb 100644 --- a/iceoryx_posh/source/popo/building_blocks/condition_notifier.cpp +++ b/iceoryx_posh/source/popo/building_blocks/condition_notifier.cpp @@ -1,5 +1,5 @@ // Copyright (c) 2020 by Robert Bosch GmbH. All rights reserved. -// 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. @@ -38,7 +38,7 @@ void ConditionNotifier::notify() noexcept { getMembers()->m_activeNotifications[m_notificationIndex].store(true, std::memory_order_release); getMembers()->m_wasNotified.store(true, std::memory_order_relaxed); - getMembers()->semaphore->post().or_else( + getMembers()->m_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 index dabbc2c813..e1793c0de9 100644 --- a/iceoryx_posh/source/popo/building_blocks/condition_variable_data.cpp +++ b/iceoryx_posh/source/popo/building_blocks/condition_variable_data.cpp @@ -1,5 +1,5 @@ // Copyright (c) 2020 by Robert Bosch GmbH. All rights reserved. -// 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. @@ -29,7 +29,7 @@ ConditionVariableData::ConditionVariableData() noexcept ConditionVariableData::ConditionVariableData(const RuntimeName_t& runtimeName) noexcept : m_runtimeName(runtimeName) { - posix::UnnamedSemaphoreBuilder().initialValue(0U).isInterProcessCapable(true).create(semaphore).or_else([](auto) { + posix::UnnamedSemaphoreBuilder().initialValue(0U).isInterProcessCapable(true).create(m_semaphore).or_else([](auto) { errorHandler(PoshError::POPO__CONDITION_VARIABLE_DATA_FAILED_TO_CREATE_SEMAPHORE, ErrorLevel::FATAL); }); diff --git a/iceoryx_posh/test/moduletests/test_popo_condition_variable.cpp b/iceoryx_posh/test/moduletests/test_popo_condition_variable.cpp index 5ebdd0ff6a..b0c1dc065c 100644 --- a/iceoryx_posh/test/moduletests/test_popo_condition_variable.cpp +++ b/iceoryx_posh/test/moduletests/test_popo_condition_variable.cpp @@ -1,5 +1,5 @@ // Copyright (c) 2020 by Robert Bosch GmbH. All rights reserved. -// 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. From c6dce9372af432f7bf086c0a74569be06c6e27af Mon Sep 17 00:00:00 2001 From: Christian Eltzschig Date: Thu, 23 Jun 2022 18:25:34 +0200 Subject: [PATCH 14/17] iox-#751 replace adaptive_wait with Barrier Signed-off-by: Christian Eltzschig --- .../moduletests/test_posix_signal_watcher.cpp | 8 ++++---- .../integrationtests/test_client_server.cpp | 13 ++++++------ ...est_publisher_subscriber_communication.cpp | 7 ++++--- .../test_popo_chunk_distributor.cpp | 19 +++++++++--------- .../test_popo_condition_variable.cpp | 20 +++++++++---------- .../test/moduletests/test_posh_runtime.cpp | 20 +++++++++---------- .../moduletests/test_roudi_portmanager.cpp | 7 ++++--- 7 files changed, 49 insertions(+), 45 deletions(-) diff --git a/iceoryx_hoofs/test/moduletests/test_posix_signal_watcher.cpp b/iceoryx_hoofs/test/moduletests/test_posix_signal_watcher.cpp index 3a9d2ffc5f..70d6825883 100644 --- a/iceoryx_hoofs/test/moduletests/test_posix_signal_watcher.cpp +++ b/iceoryx_hoofs/test/moduletests/test_posix_signal_watcher.cpp @@ -14,8 +14,8 @@ // // SPDX-License-Identifier: Apache-2.0 -#include "iceoryx_hoofs/internal/cxx/adaptive_wait.hpp" #include "iceoryx_hoofs/posix_wrapper/signal_watcher.hpp" +#include "iceoryx_hoofs/testing/barrier.hpp" #include "iceoryx_hoofs/testing/watch_dog.hpp" #include "test.hpp" #include @@ -85,7 +85,7 @@ void unblocksWhenSignalWasRaisedForWaiters(SignalWatcher_test& test, const uint64_t numberOfWaiters, const std::function& wait) { - std::atomic isThreadStarted{0}; + Barrier isThreadStarted(numberOfWaiters); std::atomic isThreadFinished{0}; std::vector threads; @@ -93,13 +93,13 @@ void unblocksWhenSignalWasRaisedForWaiters(SignalWatcher_test& test, for (uint64_t i = 0; i < numberOfWaiters; ++i) { threads.emplace_back([&] { - ++isThreadStarted; + isThreadStarted.notify(); wait(); ++isThreadFinished; }); } - iox::cxx::internal::adaptive_wait().wait_loop([&] { return (isThreadStarted != numberOfWaiters); }); + isThreadStarted.wait(); std::this_thread::sleep_for(test.waitingTime); diff --git a/iceoryx_posh/test/integrationtests/test_client_server.cpp b/iceoryx_posh/test/integrationtests/test_client_server.cpp index 318bb82bb1..5e9e3a3d27 100644 --- a/iceoryx_posh/test/integrationtests/test_client_server.cpp +++ b/iceoryx_posh/test/integrationtests/test_client_server.cpp @@ -14,6 +14,7 @@ // // SPDX-License-Identifier: Apache-2.0 +#include "iceoryx_hoofs/testing/barrier.hpp" #include "iceoryx_hoofs/testing/watch_dog.hpp" #include "iceoryx_posh/popo/client.hpp" #include "iceoryx_posh/popo/server.hpp" @@ -348,7 +349,7 @@ TEST_F(ClientServer_test, ServerTakeRequestUnblocksClientSendingRequest) std::atomic_bool wasRequestSent{false}; // block in a separate thread - std::atomic_bool isThreadStarted{false}; + Barrier isThreadStarted(1U); std::thread blockingClient([&] { auto sendRequest = [&]() { auto loanResult = client.loan(); @@ -363,14 +364,14 @@ TEST_F(ClientServer_test, ServerTakeRequestUnblocksClientSendingRequest) } // signal that an blocking send is expected - isThreadStarted = true; + isThreadStarted.notify(); sendRequest(); wasRequestSent = true; }); // wait some time to check if the client is blocked constexpr std::chrono::milliseconds SLEEP_TIME{100U}; - iox::cxx::internal::adaptive_wait().wait_loop([&] { return !isThreadStarted.load(); }); + isThreadStarted.wait(); std::this_thread::sleep_for(SLEEP_TIME); EXPECT_THAT(wasRequestSent.load(), Eq(false)); @@ -409,7 +410,7 @@ TEST_F(ClientServer_test, ClientTakesResponseUnblocksServerSendingResponse) std::atomic_bool wasResponseSent{false}; // block in a separate thread - std::atomic_bool isThreadStarted{false}; + Barrier isThreadStarted(1U); std::thread blockingServer([&] { auto processRequest = [&]() { auto takeResult = server.take(); @@ -424,14 +425,14 @@ TEST_F(ClientServer_test, ClientTakesResponseUnblocksServerSendingResponse) processRequest(); } - isThreadStarted = true; + isThreadStarted.notify(); processRequest(); wasResponseSent = true; }); // wait some time to check if the server is blocked constexpr std::chrono::milliseconds SLEEP_TIME{100U}; - iox::cxx::internal::adaptive_wait().wait_loop([&] { return !isThreadStarted.load(); }); + isThreadStarted.wait(); std::this_thread::sleep_for(SLEEP_TIME); EXPECT_THAT(wasResponseSent.load(), Eq(false)); diff --git a/iceoryx_posh/test/integrationtests/test_publisher_subscriber_communication.cpp b/iceoryx_posh/test/integrationtests/test_publisher_subscriber_communication.cpp index d07ee37da4..9176e1e742 100644 --- a/iceoryx_posh/test/integrationtests/test_publisher_subscriber_communication.cpp +++ b/iceoryx_posh/test/integrationtests/test_publisher_subscriber_communication.cpp @@ -21,6 +21,7 @@ #include "iceoryx_hoofs/cxx/string.hpp" #include "iceoryx_hoofs/cxx/variant.hpp" #include "iceoryx_hoofs/cxx/vector.hpp" +#include "iceoryx_hoofs/testing/barrier.hpp" #include "iceoryx_hoofs/testing/watch_dog.hpp" #include "iceoryx_posh/popo/publisher.hpp" #include "iceoryx_posh/popo/subscriber.hpp" @@ -551,16 +552,16 @@ TEST_F(PublisherSubscriberCommunication_test, PublisherBlocksWhenBlockingActivat EXPECT_FALSE(publisher->publishCopyOf("and hypnotoad will smile back").has_error()); std::atomic_bool wasSampleDelivered{false}; - std::atomic_bool isThreadStarted{false}; + Barrier isThreadStarted(1U); std::thread t1([&] { - isThreadStarted = true; + isThreadStarted.notify(); EXPECT_FALSE(publisher->publishCopyOf("oh no hypnotoad is staring at me").has_error()); wasSampleDelivered.store(true); }); constexpr int64_t TIMEOUT_IN_MS = 100; - iox::cxx::internal::adaptive_wait().wait_loop([&] { return !isThreadStarted.load(); }); + isThreadStarted.wait(); std::this_thread::sleep_for(std::chrono::milliseconds(TIMEOUT_IN_MS)); EXPECT_FALSE(wasSampleDelivered.load()); diff --git a/iceoryx_posh/test/moduletests/test_popo_chunk_distributor.cpp b/iceoryx_posh/test/moduletests/test_popo_chunk_distributor.cpp index 164eb82e6c..5652d47a91 100644 --- a/iceoryx_posh/test/moduletests/test_popo_chunk_distributor.cpp +++ b/iceoryx_posh/test/moduletests/test_popo_chunk_distributor.cpp @@ -16,6 +16,7 @@ // SPDX-License-Identifier: Apache-2.0 #include "iceoryx_hoofs/cxx/variant_queue.hpp" +#include "iceoryx_hoofs/testing/barrier.hpp" #include "iceoryx_hoofs/testing/watch_dog.hpp" #include "iceoryx_posh/internal/mepoo/shared_chunk.hpp" #include "iceoryx_posh/internal/popo/building_blocks/chunk_distributor.hpp" @@ -620,16 +621,16 @@ TYPED_TEST(ChunkDistributor_test, DeliverToQueueWithBlockingOptionBlocksDelivery ASSERT_FALSE(sut.deliverToQueue(queueData->m_uniqueId, EXPECTED_QUEUE_INDEX, chunk).has_error()); } - std::atomic_bool isThreadStarted{false}; + Barrier isThreadStarted(1U); auto chunk = this->allocateChunk(7373); std::atomic_bool wasChunkDelivered{false}; std::thread t1([&] { - isThreadStarted = true; + isThreadStarted.notify(); ASSERT_FALSE(sut.deliverToQueue(queueData->m_uniqueId, EXPECTED_QUEUE_INDEX, chunk).has_error()); wasChunkDelivered = true; }); - iox::cxx::internal::adaptive_wait().wait_loop([&] { return !isThreadStarted; }); + isThreadStarted.wait(); std::this_thread::sleep_for(this->BLOCKING_DURATION); EXPECT_THAT(wasChunkDelivered.load(), Eq(false)); @@ -738,15 +739,15 @@ TYPED_TEST(ChunkDistributor_test, DeliverToSingleQueueBlocksWhenOptionsAreSetToB ASSERT_FALSE(sut.tryAddQueue(queueData.get(), 0U).has_error()); sut.deliverToAllStoredQueues(this->allocateChunk(155U)); - std::atomic_bool isThreadStarted{false}; + Barrier isThreadStarted(1U); std::atomic_bool wasChunkDelivered{false}; std::thread t1([&] { - isThreadStarted = true; + isThreadStarted.notify(); sut.deliverToAllStoredQueues(this->allocateChunk(152U)); wasChunkDelivered = true; }); - iox::cxx::internal::adaptive_wait().wait_loop([&] { return !isThreadStarted; }); + isThreadStarted.wait(); std::this_thread::sleep_for(this->BLOCKING_DURATION); EXPECT_THAT(wasChunkDelivered.load(), Eq(false)); @@ -785,15 +786,15 @@ TYPED_TEST(ChunkDistributor_test, MultipleBlockingQueuesWillBeFilledWhenThereBec sut.deliverToAllStoredQueues(this->allocateChunk(425U)); - std::atomic_bool isThreadStarted{false}; + Barrier isThreadStarted(1U); std::atomic_bool wasChunkDelivered{false}; std::thread t1([&] { - isThreadStarted.store(true); + isThreadStarted.notify(); sut.deliverToAllStoredQueues(this->allocateChunk(1152U)); wasChunkDelivered = true; }); - iox::cxx::internal::adaptive_wait().wait_loop([&] { return !isThreadStarted; }); + isThreadStarted.wait(); std::this_thread::sleep_for(this->BLOCKING_DURATION); EXPECT_THAT(wasChunkDelivered.load(), Eq(false)); diff --git a/iceoryx_posh/test/moduletests/test_popo_condition_variable.cpp b/iceoryx_posh/test/moduletests/test_popo_condition_variable.cpp index b0c1dc065c..32b3970195 100644 --- a/iceoryx_posh/test/moduletests/test_popo_condition_variable.cpp +++ b/iceoryx_posh/test/moduletests/test_popo_condition_variable.cpp @@ -15,7 +15,7 @@ // // SPDX-License-Identifier: Apache-2.0 -#include "iceoryx_hoofs/internal/cxx/adaptive_wait.hpp" +#include "iceoryx_hoofs/testing/barrier.hpp" #include "iceoryx_hoofs/testing/timing_test.hpp" #include "iceoryx_hoofs/testing/watch_dog.hpp" #include "iceoryx_posh/internal/popo/building_blocks/condition_listener.hpp" @@ -117,14 +117,14 @@ TEST_F(ConditionVariable_test, WaitAndNotifyResultsInImmediateTriggerMultiThread { ::testing::Test::RecordProperty("TEST_ID", "39b40c73-3dcc-4af6-9682-b62816c69854"); std::atomic counter{0}; - std::atomic_bool isThreadStarted{false}; + Barrier isThreadStarted(1U); std::thread waiter([&] { EXPECT_THAT(counter, Eq(0)); - isThreadStarted = true; + isThreadStarted.notify(); m_waiter.wait(); EXPECT_THAT(counter, Eq(1)); }); - iox::cxx::internal::adaptive_wait().wait_loop([&] { return !isThreadStarted; }); + isThreadStarted.wait(); counter++; m_signaler.notify(); @@ -366,18 +366,18 @@ TIMING_TEST_F(ConditionVariable_test, WaitBlocks, Repeat(5), [&] { ConditionNotifier notifier(m_condVarData, EVENT_INDEX); ConditionListener listener(m_condVarData); NotificationVector_t activeNotifications; - std::atomic_bool isThreadStarted{false}; + Barrier isThreadStarted(1U); std::atomic_bool hasWaited{false}; std::thread waiter([&] { - isThreadStarted = true; + isThreadStarted.notify(); activeNotifications = listener.wait(); hasWaited.store(true, std::memory_order_relaxed); ASSERT_THAT(activeNotifications.size(), Eq(1U)); EXPECT_THAT(activeNotifications[0], Eq(EVENT_INDEX)); }); - iox::cxx::internal::adaptive_wait().wait_loop([&] { return !isThreadStarted; }); + isThreadStarted.wait(); std::this_thread::sleep_for(std::chrono::milliseconds(10)); EXPECT_THAT(hasWaited, Eq(false)); @@ -409,9 +409,9 @@ TIMING_TEST_F(ConditionVariable_test, SecondWaitBlocksUntilNewNotification, Repe Watchdog watchdogSecondWait(m_timeToWait); watchdogSecondWait.watchAndActOnFailure([&] { listener.destroy(); }); - std::atomic_bool isThreadStarted{false}; + Barrier isThreadStarted(1U); std::thread waiter([&] { - isThreadStarted = true; + isThreadStarted.notify(); activeNotifications = listener.wait(); hasWaited.store(true, std::memory_order_relaxed); ASSERT_THAT(activeNotifications.size(), Eq(1U)); @@ -422,7 +422,7 @@ TIMING_TEST_F(ConditionVariable_test, SecondWaitBlocksUntilNewNotification, Repe } }); - iox::cxx::internal::adaptive_wait().wait_loop([&] { return !isThreadStarted; }); + isThreadStarted.wait(); std::this_thread::sleep_for(std::chrono::milliseconds(10)); EXPECT_THAT(hasWaited, Eq(false)); diff --git a/iceoryx_posh/test/moduletests/test_posh_runtime.cpp b/iceoryx_posh/test/moduletests/test_posh_runtime.cpp index 684ad92089..2c92a0cc03 100644 --- a/iceoryx_posh/test/moduletests/test_posh_runtime.cpp +++ b/iceoryx_posh/test/moduletests/test_posh_runtime.cpp @@ -16,7 +16,7 @@ // SPDX-License-Identifier: Apache-2.0 #include "iceoryx_hoofs/cxx/convert.hpp" -#include "iceoryx_hoofs/internal/cxx/adaptive_wait.hpp" +#include "iceoryx_hoofs/testing/barrier.hpp" #include "iceoryx_hoofs/testing/timing_test.hpp" #include "iceoryx_hoofs/testing/watch_dog.hpp" #include "iceoryx_posh/iceoryx_posh_types.hpp" @@ -957,15 +957,15 @@ TEST_F(PoshRuntime_test, ShutdownUnblocksBlockingPublisher) deadlockWatchdog.watchAndActOnFailure([] { std::terminate(); }); // block in a separate thread - std::atomic_bool isThreadStarted{false}; + Barrier isThreadStarted(1U); std::thread blockingPublisher([&] { - isThreadStarted = true; + isThreadStarted.notify(); ASSERT_FALSE(publisher.publishCopyOf(42U).has_error()); wasSampleSent = true; }); // wait some time to check if the publisher is blocked - iox::cxx::internal::adaptive_wait().wait_loop([&] { return !isThreadStarted.load(); }); + isThreadStarted.wait(); constexpr std::chrono::milliseconds SLEEP_TIME{100U}; std::this_thread::sleep_for(SLEEP_TIME); EXPECT_THAT(wasSampleSent.load(), Eq(false)); @@ -1005,7 +1005,7 @@ TEST_F(PoshRuntime_test, ShutdownUnblocksBlockingClient) deadlockWatchdog.watchAndActOnFailure([] { std::terminate(); }); // block in a separate thread - std::atomic_bool isThreadStarted{false}; + Barrier isThreadStarted(1U); std::thread blockingClient([&] { auto sendRequest = [&](bool expectError) { auto clientLoanResult = client.loan(sizeof(uint64_t), alignof(uint64_t)); @@ -1026,7 +1026,7 @@ TEST_F(PoshRuntime_test, ShutdownUnblocksBlockingClient) } // signal that an blocking send is expected - isThreadStarted = true; + isThreadStarted.notify(); constexpr bool EXPECT_ERROR_INDICATOR{true}; sendRequest(EXPECT_ERROR_INDICATOR); wasRequestSent = true; @@ -1034,7 +1034,7 @@ TEST_F(PoshRuntime_test, ShutdownUnblocksBlockingClient) // wait some time to check if the client is blocked constexpr std::chrono::milliseconds SLEEP_TIME{100U}; - iox::cxx::internal::adaptive_wait().wait_loop([&] { return !isThreadStarted.load(); }); + isThreadStarted.wait(); std::this_thread::sleep_for(SLEEP_TIME); EXPECT_THAT(wasRequestSent.load(), Eq(false)); @@ -1081,7 +1081,7 @@ TEST_F(PoshRuntime_test, ShutdownUnblocksBlockingServer) deadlockWatchdog.watchAndActOnFailure([] { std::terminate(); }); // block in a separate thread - std::atomic_bool isThreadStarted{false}; + Barrier isThreadStarted(1U); std::thread blockingServer([&] { auto processRequest = [&](bool expectError) { auto takeResult = server.take(); @@ -1103,7 +1103,7 @@ TEST_F(PoshRuntime_test, ShutdownUnblocksBlockingServer) processRequest(EXPECT_ERROR_INDICATOR); } - isThreadStarted = true; + isThreadStarted.notify(); constexpr bool EXPECT_ERROR_INDICATOR{true}; processRequest(EXPECT_ERROR_INDICATOR); wasResponseSent = true; @@ -1111,7 +1111,7 @@ TEST_F(PoshRuntime_test, ShutdownUnblocksBlockingServer) // wait some time to check if the server is blocked constexpr std::chrono::milliseconds SLEEP_TIME{100U}; - iox::cxx::internal::adaptive_wait().wait_loop([&] { return !isThreadStarted.load(); }); + isThreadStarted.wait(); std::this_thread::sleep_for(SLEEP_TIME); EXPECT_THAT(wasResponseSent.load(), Eq(false)); diff --git a/iceoryx_posh/test/moduletests/test_roudi_portmanager.cpp b/iceoryx_posh/test/moduletests/test_roudi_portmanager.cpp index 0f99b35f4f..2982cfcfe4 100644 --- a/iceoryx_posh/test/moduletests/test_roudi_portmanager.cpp +++ b/iceoryx_posh/test/moduletests/test_roudi_portmanager.cpp @@ -15,6 +15,7 @@ // // SPDX-License-Identifier: Apache-2.0 +#include "iceoryx_hoofs/testing/barrier.hpp" #include "test_roudi_portmanager_fixture.hpp" namespace iox_test_roudi_portmanager @@ -842,18 +843,18 @@ void PortManager_test::setupAndTestBlockingPublisher(const iox::RuntimeName_t& p deadlockWatchdog.watchAndActOnFailure([] { std::terminate(); }); // block in a separate thread - std::atomic_bool isThreadStarted{false}; + Barrier isThreadStarted(1U); std::thread blockingPublisher([&] { auto maybeChunk = publisher.tryAllocateChunk(42U, 8U); ASSERT_FALSE(maybeChunk.has_error()); - isThreadStarted = true; + isThreadStarted.notify(); publisher.sendChunk(maybeChunk.value()); wasChunkSent = true; }); // wait some time to check if the publisher is blocked constexpr int64_t SLEEP_IN_MS = 100; - iox::cxx::internal::adaptive_wait().wait_loop([&] { return !isThreadStarted.load(); }); + isThreadStarted.wait(); std::this_thread::sleep_for(std::chrono::milliseconds(SLEEP_IN_MS)); EXPECT_THAT(wasChunkSent.load(), Eq(false)); From 795f0bd59d7f089ec3d0336c7f63b48dbe516702 Mon Sep 17 00:00:00 2001 From: Christian Eltzschig Date: Thu, 23 Jun 2022 18:44:37 +0200 Subject: [PATCH 15/17] iox-#751 Update readmes and copyright year Signed-off-by: Christian Eltzschig --- iceoryx_dds/source/gateway/main.cpp | 2 +- iceoryx_examples/request_response/README.md | 7 ++++--- iceoryx_examples/request_response_in_c/README.md | 3 +-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/iceoryx_dds/source/gateway/main.cpp b/iceoryx_dds/source/gateway/main.cpp index 9ad5fa16aa..d9a661d1f4 100644 --- a/iceoryx_dds/source/gateway/main.cpp +++ b/iceoryx_dds/source/gateway/main.cpp @@ -1,5 +1,5 @@ // Copyright (c) 2020 by Robert Bosch GmbH. All rights reserved. -// 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_examples/request_response/README.md b/iceoryx_examples/request_response/README.md index 4fad9c0d5e..9927e0f30c 100644 --- a/iceoryx_examples/request_response/README.md +++ b/iceoryx_examples/request_response/README.md @@ -35,6 +35,7 @@ At first, the includes for the client port, request-response types, WaitSet, and ```cpp #include "request_and_response_types.hpp" +#include "iceoryx_hoofs/posix_wrapper/signal_handler.hpp" #include "iceoryx_hoofs/posix_wrapper/signal_watcher.hpp" #include "iceoryx_posh/popo/client.hpp" #include "iceoryx_posh/popo/wait_set.hpp" @@ -70,14 +71,14 @@ full or the server is too slow. The `ClientOptions` are similar to `PublisherOpt ```cpp -iox::popo::WaitSet<> waitset; +waitset.emplace(); iox::popo::ClientOptions options; options.responseQueueCapacity = 2U; iox::popo::Client client({"Example", "Request-Response", "Add"}, options); // attach client to waitset -waitset.attachState(client, iox::popo::ClientState::HAS_RESPONSE).or_else([](auto) { +waitset->attachState(client, iox::popo::ClientState::HAS_RESPONSE).or_else([](auto) { std::cerr << "failed to attach client" << std::endl; std::exit(EXIT_FAILURE); }); @@ -131,7 +132,7 @@ Once the request has been sent, we block and wait for samples to arrive. Then we iterate over the notification vector to check if we were triggered from our client: ```cpp -auto notificationVector = waitset.timedWait(iox::units::Duration::fromSeconds(5)); +auto notificationVector = waitset->timedWait(iox::units::Duration::fromSeconds(5)); for (auto& notification : notificationVector) { diff --git a/iceoryx_examples/request_response_in_c/README.md b/iceoryx_examples/request_response_in_c/README.md index 9f0f98544a..0958f888e1 100644 --- a/iceoryx_examples/request_response_in_c/README.md +++ b/iceoryx_examples/request_response_in_c/README.md @@ -159,8 +159,7 @@ Afterwards we create our waitset and attach the client state `ClientState_HAS_RE to it. ```c -iox_ws_storage_t waitsetStorage; -iox_ws_t waitset = iox_ws_init(&waitsetStorage); +waitset = iox_ws_init(&waitsetStorage); if (iox_ws_attach_client_state(waitset, client, ClientState_HAS_RESPONSE, 0U, NULL) != WaitSetResult_SUCCESS) { From b30396024a440b5a09081efa29f1ff31726391c4 Mon Sep 17 00:00:00 2001 From: Christian Eltzschig Date: Thu, 23 Jun 2022 18:09:01 +0100 Subject: [PATCH 16/17] iox-#751 Do not include unistd.h since it is not available in windows Signed-off-by: Christian Eltzschig --- iceoryx_examples/request_response_in_c/client_c_waitset.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/iceoryx_examples/request_response_in_c/client_c_waitset.c b/iceoryx_examples/request_response_in_c/client_c_waitset.c index 0c53e79905..5f274bb311 100644 --- a/iceoryx_examples/request_response_in_c/client_c_waitset.c +++ b/iceoryx_examples/request_response_in_c/client_c_waitset.c @@ -27,7 +27,10 @@ #include #include #include + +#if !defined(_WIN32) #include +#endif #define NUMBER_OF_NOTIFICATIONS 1 From 34fd386603bbc125c73cde433739a5675c678bed Mon Sep 17 00:00:00 2001 From: Christian Eltzschig Date: Thu, 23 Jun 2022 19:24:11 +0200 Subject: [PATCH 17/17] iox-#751 Replace adaptive_wait with Barrier Signed-off-by: Christian Eltzschig --- .../iceoryx_hoofs/posix_wrapper/named_pipe.hpp | 2 +- .../test_publisher_subscriber_communication.cpp | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/named_pipe.hpp b/iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/named_pipe.hpp index 72ba6e83c9..4d248e6a9b 100644 --- a/iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/named_pipe.hpp +++ b/iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/named_pipe.hpp @@ -39,7 +39,7 @@ class NamedPipe : public DesignPattern::Creation static constexpr uint64_t MAX_MESSAGE_SIZE = 4U * 1024U; static constexpr uint32_t MAX_NUMBER_OF_MESSAGES = 10U; static_assert(MAX_NUMBER_OF_MESSAGES < IOX_SEM_VALUE_MAX, - "The maximum number of supported messages must be less to the maximum allowed semaphore value"); + "The maximum number of supported messages must be less than the maximum allowed semaphore value"); static constexpr uint64_t NULL_TERMINATOR_SIZE = 0U; static constexpr units::Duration CYCLE_TIME = units::Duration::fromMilliseconds(10); diff --git a/iceoryx_posh/test/integrationtests/test_publisher_subscriber_communication.cpp b/iceoryx_posh/test/integrationtests/test_publisher_subscriber_communication.cpp index 9176e1e742..9bd90f0f57 100644 --- a/iceoryx_posh/test/integrationtests/test_publisher_subscriber_communication.cpp +++ b/iceoryx_posh/test/integrationtests/test_publisher_subscriber_communication.cpp @@ -595,14 +595,14 @@ TEST_F(PublisherSubscriberCommunication_test, PublisherDoesNotBlockAndDiscardsSa EXPECT_FALSE(publisher->publishCopyOf("second hypnotoad ate it").has_error()); std::atomic_bool wasSampleDelivered{false}; - std::atomic_bool isThreadStarted{false}; + Barrier isThreadStarted(1U); std::thread t1([&] { - isThreadStarted = true; + isThreadStarted.notify(); EXPECT_FALSE(publisher->publishCopyOf("third a tiny black hole smells like butter").has_error()); wasSampleDelivered.store(true); }); - iox::cxx::internal::adaptive_wait().wait_loop([&] { return !isThreadStarted.load(); }); + isThreadStarted.wait(); t1.join(); EXPECT_TRUE(wasSampleDelivered.load()); @@ -665,16 +665,16 @@ TEST_F(PublisherSubscriberCommunication_test, MixedOptionsSetupWorksWithBlocking EXPECT_FALSE(publisherNonBlocking->publishCopyOf("hypnotoad has a sister named hypnoodle").has_error()); std::atomic_bool wasSampleDelivered{false}; - std::atomic_bool isThreadStarted{false}; + Barrier isThreadStarted(1U); std::thread t1([&] { - isThreadStarted = true; + isThreadStarted.notify(); EXPECT_FALSE(publisherBlocking->publishCopyOf("chucky is the only one who can ride the hypnotoad").has_error()); wasSampleDelivered.store(true); }); constexpr int64_t TIMEOUT_IN_MS = 100; - iox::cxx::internal::adaptive_wait().wait_loop([&] { return !isThreadStarted.load(); }); + isThreadStarted.wait(); std::this_thread::sleep_for(std::chrono::milliseconds(TIMEOUT_IN_MS)); EXPECT_FALSE(wasSampleDelivered.load());