From 527b9f94faa1430b569dc2cab9bb9376bd52c468 Mon Sep 17 00:00:00 2001 From: Mathias Kraus Date: Sat, 2 Sep 2023 12:20:57 +0200 Subject: [PATCH] iox-#1036 Use builder API for 'Mutex' --- .../internal/posix_wrapper/mutex.hpp | 4 --- iceoryx_hoofs/source/posix_wrapper/mutex.cpp | 27 ------------------- .../popo/building_blocks/locking_policy.hpp | 4 ++- .../internal/runtime/posh_runtime_impl.hpp | 3 ++- .../popo/building_blocks/locking_policy.cpp | 15 ++++++++--- .../source/runtime/posh_runtime_impl.cpp | 7 ++++- 6 files changed, 23 insertions(+), 37 deletions(-) diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/mutex.hpp b/iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/mutex.hpp index 674a2d5f842..7037035311a 100644 --- a/iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/mutex.hpp +++ b/iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/mutex.hpp @@ -96,10 +96,6 @@ enum class MutexTryLock class mutex { public: - /// @attention the construction of the mutex can fail. This can lead to a program termination! - /// @todo iox-#1036 remove this, introduced to keep current API temporarily - explicit mutex(const bool f_isRecursive) noexcept; - /// @brief Destroys the mutex. When the mutex is still locked this will fail and the /// mutex is leaked! If the MutexThreadTerminationBehavior is set to RELEASE_WHEN_LOCKED /// a locked mutex is unlocked and the handle is cleaned up correctly. diff --git a/iceoryx_hoofs/source/posix_wrapper/mutex.cpp b/iceoryx_hoofs/source/posix_wrapper/mutex.cpp index e626e0b22e7..50486b8a03b 100644 --- a/iceoryx_hoofs/source/posix_wrapper/mutex.cpp +++ b/iceoryx_hoofs/source/posix_wrapper/mutex.cpp @@ -279,33 +279,6 @@ expected MutexBuilder::create(optional& uniniti return ok(); } -/// @todo iox-#1036 remove this, introduced to keep current API temporarily -mutex::mutex(bool f_isRecursive) noexcept -{ - pthread_mutexattr_t attr; - bool isInitialized{true}; - isInitialized &= !posixCall(pthread_mutexattr_init)(&attr).returnValueMatchesErrno().evaluate().has_error(); - isInitialized &= !posixCall(pthread_mutexattr_setpshared)(&attr, PTHREAD_PROCESS_SHARED) - .returnValueMatchesErrno() - .evaluate() - .has_error(); - isInitialized &= - !posixCall(pthread_mutexattr_settype)(&attr, f_isRecursive ? PTHREAD_MUTEX_RECURSIVE : PTHREAD_MUTEX_NORMAL) - .returnValueMatchesErrno() - .evaluate() - .has_error(); - isInitialized &= !posixCall(pthread_mutexattr_setprotocol)(&attr, PTHREAD_PRIO_NONE) - .returnValueMatchesErrno() - .evaluate() - .has_error(); - isInitialized &= !posixCall(pthread_mutex_init)(&m_handle, &attr).returnValueMatchesErrno().evaluate().has_error(); - isInitialized &= !posixCall(pthread_mutexattr_destroy)(&attr).returnValueMatchesErrno().evaluate().has_error(); - - /// NOLINTJUSTIFICATION is fixed in the PR iox-#1443 - /// NOLINTNEXTLINE(hicpp-no-array-decay,cppcoreguidelines-pro-bounds-array-to-pointer-decay) - cxx::Ensures(isInitialized && "Unable to create mutex"); -} - mutex::~mutex() noexcept { if (m_isDestructable) diff --git a/iceoryx_posh/include/iceoryx_posh/internal/popo/building_blocks/locking_policy.hpp b/iceoryx_posh/include/iceoryx_posh/internal/popo/building_blocks/locking_policy.hpp index 07b8dabaa2b..214e0ad1d61 100644 --- a/iceoryx_posh/include/iceoryx_posh/internal/popo/building_blocks/locking_policy.hpp +++ b/iceoryx_posh/include/iceoryx_posh/internal/popo/building_blocks/locking_policy.hpp @@ -25,13 +25,15 @@ namespace popo class ThreadSafePolicy { public: + ThreadSafePolicy() noexcept; + // needs to be public since we want to use std::lock_guard void lock() const noexcept; void unlock() const noexcept; bool tryLock() const noexcept; private: - mutable posix::mutex m_mutex{true}; // recursive lock + mutable optional m_mutex; }; class SingleThreadedPolicy diff --git a/iceoryx_posh/include/iceoryx_posh/internal/runtime/posh_runtime_impl.hpp b/iceoryx_posh/include/iceoryx_posh/internal/runtime/posh_runtime_impl.hpp index 35ee2aca06f..215b887de56 100644 --- a/iceoryx_posh/include/iceoryx_posh/internal/runtime/posh_runtime_impl.hpp +++ b/iceoryx_posh/include/iceoryx_posh/internal/runtime/posh_runtime_impl.hpp @@ -22,6 +22,7 @@ #include "iceoryx_posh/internal/runtime/shared_memory_user.hpp" #include "iceoryx_posh/runtime/posh_runtime.hpp" #include "iox/function.hpp" +#include "iox/optional.hpp" namespace iox { @@ -104,7 +105,7 @@ class PoshRuntimeImpl : public PoshRuntime expected requestConditionVariableFromRoudi(const IpcMessage& sendBuffer) noexcept; - mutable posix::mutex m_appIpcRequestMutex{false}; + mutable optional m_appIpcRequestMutex; IpcRuntimeInterface m_ipcChannelInterface; optional m_ShmInterface; diff --git a/iceoryx_posh/source/popo/building_blocks/locking_policy.cpp b/iceoryx_posh/source/popo/building_blocks/locking_policy.cpp index 13b5553ac0a..c98f86aa5d3 100644 --- a/iceoryx_posh/source/popo/building_blocks/locking_policy.cpp +++ b/iceoryx_posh/source/popo/building_blocks/locking_policy.cpp @@ -23,9 +23,18 @@ namespace iox { namespace popo { +ThreadSafePolicy::ThreadSafePolicy() noexcept +{ + posix::MutexBuilder() + .isInterProcessCapable(true) + .mutexType(posix::MutexType::RECURSIVE) + .create(m_mutex) + .expect("Valid Mutex"); +} + void ThreadSafePolicy::lock() const noexcept { - if (!m_mutex.lock()) + if (!m_mutex->lock()) { IOX_LOG(FATAL) << "Locking of an inter-process mutex failed! This indicates that the application holding the lock " @@ -36,7 +45,7 @@ void ThreadSafePolicy::lock() const noexcept void ThreadSafePolicy::unlock() const noexcept { - if (!m_mutex.unlock()) + if (!m_mutex->unlock()) { IOX_LOG(FATAL) << "Unlocking of an inter-process mutex failed! This indicates that the resources were cleaned up " @@ -47,7 +56,7 @@ void ThreadSafePolicy::unlock() const noexcept bool ThreadSafePolicy::tryLock() const noexcept { - auto tryLockResult = m_mutex.try_lock(); + auto tryLockResult = m_mutex->try_lock(); if (tryLockResult.has_error()) { errorHandler(PoshError::POPO__CHUNK_TRY_LOCK_ERROR, ErrorLevel::FATAL); diff --git a/iceoryx_posh/source/runtime/posh_runtime_impl.cpp b/iceoryx_posh/source/runtime/posh_runtime_impl.cpp index 36de773a8f8..7f967578cc3 100644 --- a/iceoryx_posh/source/runtime/posh_runtime_impl.cpp +++ b/iceoryx_posh/source/runtime/posh_runtime_impl.cpp @@ -45,6 +45,11 @@ PoshRuntimeImpl::PoshRuntimeImpl(optional name, const Runt m_ipcChannelInterface.getSegmentManagerAddressOffset()}); }()) { + posix::MutexBuilder() + .isInterProcessCapable(false) + .mutexType(posix::MutexType::NORMAL) + .create(m_appIpcRequestMutex) + .expect("Valid Mutex"); } PoshRuntimeImpl::~PoshRuntimeImpl() noexcept @@ -648,7 +653,7 @@ popo::ConditionVariableData* PoshRuntimeImpl::getMiddlewareConditionVariable() n bool PoshRuntimeImpl::sendRequestToRouDi(const IpcMessage& msg, IpcMessage& answer) noexcept { // runtime must be thread safe - std::lock_guard g(m_appIpcRequestMutex); + std::lock_guard g(m_appIpcRequestMutex.value()); return m_ipcChannelInterface.sendRequestToRouDi(msg, answer); }