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 0b84619178..d4a9ac1f16 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 @@ -29,7 +29,7 @@ class ThreadSafePolicy bool tryLock() const noexcept; private: - mutable posix::mutex m_mutex{true}; // recursive lock + mutable posix::mutex m_mutex{posix::mutex::Recursive::ON, posix::mutex::Robust::ON}; // recursive lock }; class SingleThreadedPolicy diff --git a/iceoryx_posh/include/iceoryx_posh/internal/popo/receiver_handler.hpp b/iceoryx_posh/include/iceoryx_posh/internal/popo/receiver_handler.hpp index 5e119574b0..c372d56d3f 100644 --- a/iceoryx_posh/include/iceoryx_posh/internal/popo/receiver_handler.hpp +++ b/iceoryx_posh/include/iceoryx_posh/internal/popo/receiver_handler.hpp @@ -35,7 +35,7 @@ class ThreadSafe void unlock(); private: - mutex_t m_mutex{true}; // recursive lock + mutex_t m_mutex{mutex_t::Recursive::ON, mutex_t::Robust::ON}; }; class SingleThreaded diff --git a/iceoryx_posh/include/iceoryx_posh/internal/popo/receiver_port_data.hpp b/iceoryx_posh/include/iceoryx_posh/internal/popo/receiver_port_data.hpp index 7462b40cd2..74ff90fd61 100644 --- a/iceoryx_posh/include/iceoryx_posh/internal/popo/receiver_port_data.hpp +++ b/iceoryx_posh/include/iceoryx_posh/internal/popo/receiver_port_data.hpp @@ -51,7 +51,7 @@ struct ReceiverPortData : public BasePortData // event callback related mutable std::atomic_bool m_chunkSendCallbackActive{false}; - mutable mutex_t m_chunkSendCallbackMutex{false}; + mutable mutex_t m_chunkSendCallbackMutex{mutex_t::Recursive::OFF, mutex_t::Robust::ON}; iox::relative_ptr m_chunkSendSemaphore{nullptr}; // offer semaphore that is stored in shared memory diff --git a/iceoryx_utils/include/iceoryx_utils/internal/concurrent/locked_loffli.hpp b/iceoryx_utils/include/iceoryx_utils/internal/concurrent/locked_loffli.hpp index ecec568c3b..5a72e7a357 100644 --- a/iceoryx_utils/include/iceoryx_utils/internal/concurrent/locked_loffli.hpp +++ b/iceoryx_utils/include/iceoryx_utils/internal/concurrent/locked_loffli.hpp @@ -33,7 +33,7 @@ class LockedLoFFLi uint32_t* m_freeIndices{nullptr}; using mutex_t = posix::mutex; - mutable mutex_t m_accessMutex{false}; + mutable mutex_t m_accessMutex{mutex_t::Recursive::OFF, mutex_t::Robust::OFF}; uint32_t m_invalidIndex{0}; diff --git a/iceoryx_utils/include/iceoryx_utils/internal/posix_wrapper/mutex.hpp b/iceoryx_utils/include/iceoryx_utils/internal/posix_wrapper/mutex.hpp index 1f59188f9b..b60fc0935a 100644 --- a/iceoryx_utils/include/iceoryx_utils/internal/posix_wrapper/mutex.hpp +++ b/iceoryx_utils/include/iceoryx_utils/internal/posix_wrapper/mutex.hpp @@ -32,7 +32,8 @@ namespace posix /// #include "iceoryx_utils/internal/posix_wrapper/mutex.hpp" /// /// int main() { -/// cxx::optional myMutex = posix::mutex::CreateMutex(false); +/// cxx::optional myMutex = posix::mutex::CreateMutex(iox::posix::mutex::Recursive::OFF, +/// iox::posix::mutex::Robust::OFF); /// /// // always verify if the mutex could be created since we aren't /// // throwing exceptions @@ -53,42 +54,55 @@ namespace posix class mutex { public: - /// @brief the construction of the mutex can fail which will lead to a call - /// to std::terminate which is alright for the moment since we are - /// intending to get rid of the mutex sooner or later - mutex(const bool f_isRecursive); + enum class Recursive + { + ON, + OFF + }; + + enum class Robust + { + ON, + OFF + }; + + /// @brief The construction of the mutex can fail, which will lead to a call to std::terminate, which is alright + /// for the moment since we are intending to get rid of the mutex sooner or later. + /// @param[in] recursive Sets the recursive attribute of the mutex. If recursive is ON, the same thread, which has + /// already locked the mutex, can lock the mutex again without getting blocked. + /// @param[in] robust If robust is set ON, a process or thread can exit while having the lock without causing an + /// invalid mutex. In the next lock call to this mutex the system recognizes the mutex is locked by a dead process + /// or thread and allows the mutex to be restored. + mutex(const Recursive recursive, const Robust robust); ~mutex(); - /// @brief all copy and move assignment methods need to be deleted otherwise - /// undefined behavior or race conditions will occure if you copy - /// or move mutexe when its possible that they are locked or will - /// be locked + /// @brief all copy and move assignment methods need to be deleted otherwise undefined behavior or race conditions + /// will occur if you copy or move mutex when its possible that they are locked or will be locked. mutex(const mutex&) = delete; mutex(mutex&&) = delete; mutex& operator=(const mutex&) = delete; mutex& operator=(mutex&&) = delete; - /// @brief Locks the mutex object and returns true if the underlying c - /// function did not returned any error. If the mutex is already - /// locked the method is blocking till the mutex can be locked. + /// @brief Locks the mutex object and returns true if the underlying c function did not returned any error. If the + /// mutex is already locked the method is blocking till the mutex can be locked. bool lock(); + /// @brief Unlocks the mutex object and returns true if the underlying c /// function did not returned any error. /// IMPORTANT! Unlocking and unlocked mutex is undefined behavior /// and the underlying c function will report success in this case! bool unlock(); - /// @brief Tries to lock the mutex object. If it is not possible to lock - /// the mutex object try_lock will return an error. If the c - /// function fails it will return false, otherwise true. + /// @brief Tries to lock the mutex object. If it is not possible to lock the mutex object try_lock will return an + /// error. If the c function fails it will return false, otherwise true. bool try_lock(); - /// @brief Returns the native handle which then can be used in - /// pthread_mutex_** calls. Required when a pthread_mutex_** - /// call is not abstracted with this wrapper. + /// @brief Returns the native handle which then can be used in pthread_mutex_** calls. Required when a + /// pthread_mutex_** call is not abstracted with this wrapper. pthread_mutex_t get_native_handle() const noexcept; + private: pthread_mutex_t m_handle; }; } // namespace posix diff --git a/iceoryx_utils/source/posix_wrapper/mutex.cpp b/iceoryx_utils/source/posix_wrapper/mutex.cpp index 6b24976ad2..c88444ef41 100644 --- a/iceoryx_utils/source/posix_wrapper/mutex.cpp +++ b/iceoryx_utils/source/posix_wrapper/mutex.cpp @@ -23,7 +23,7 @@ namespace iox { namespace posix { -mutex::mutex(bool f_isRecursive) +mutex::mutex(const Recursive recursive, const Robust robust) { pthread_mutexattr_t attr; bool isInitialized{true}; @@ -41,7 +41,7 @@ mutex::mutex(bool f_isRecursive) {0}, {}, &attr, - f_isRecursive ? PTHREAD_MUTEX_RECURSIVE_NP : PTHREAD_MUTEX_FAST_NP) + recursive == Recursive::ON ? PTHREAD_MUTEX_RECURSIVE_NP : PTHREAD_MUTEX_FAST_NP) .hasErrors(); isInitialized &= !cxx::makeSmartC(pthread_mutexattr_setprotocol, cxx::ReturnMode::PRE_DEFINED_SUCCESS_CODE, @@ -50,6 +50,16 @@ mutex::mutex(bool f_isRecursive) &attr, PTHREAD_PRIO_NONE) .hasErrors(); + if (robust == Robust::ON) + { + isInitialized &= !cxx::makeSmartC(pthread_mutexattr_setrobust, + cxx::ReturnMode::PRE_DEFINED_SUCCESS_CODE, + {0}, + {}, + &attr, + PTHREAD_MUTEX_ROBUST) + .hasErrors(); + } isInitialized &= !cxx::makeSmartC(pthread_mutex_init, cxx::ReturnMode::PRE_DEFINED_SUCCESS_CODE, {0}, {}, &m_handle, &attr) @@ -69,10 +79,8 @@ mutex::~mutex() if (destroyCall.hasErrors()) { - std::cerr << "could not destroy mutex ::: pthread_mutex_destroy returned " << destroyCall.getReturnValue() - << " " - << "( " << strerror(destroyCall.getReturnValue()) << ") " << std::endl; - std::terminate(); + std::cerr << "Could not destroy mutex ::: pthread_mutex_destroy returned " << destroyCall.getReturnValue() + << " (error: " << destroyCall.getErrorString() << "). This is a resource leak." << std::endl; } } @@ -83,8 +91,21 @@ pthread_mutex_t mutex::get_native_handle() const noexcept bool mutex::lock() { - return !cxx::makeSmartC(pthread_mutex_lock, cxx::ReturnMode::PRE_DEFINED_SUCCESS_CODE, {0}, {}, &m_handle) - .hasErrors(); + auto lockMutex = + cxx::makeSmartC(pthread_mutex_lock, cxx::ReturnMode::PRE_DEFINED_SUCCESS_CODE, {0, EOWNERDEAD}, {}, &m_handle); + if (lockMutex.getReturnValue() == EOWNERDEAD) + { + auto consistent = + cxx::makeSmartC(pthread_mutex_consistent, cxx::ReturnMode::PRE_DEFINED_SUCCESS_CODE, {0}, {}, &m_handle); + if (consistent.hasErrors()) + { + std::cerr << "Could not make robust mutex consistent :::: " + << "pthread_mutex_consistent returned " << consistent.getReturnValue() + << " (error: " << consistent.getErrorString() << ")." << std::endl; + return false; + } + } + return !lockMutex.hasErrors(); } bool mutex::unlock() diff --git a/iceoryx_utils/test/moduletests/test_posix_mutex.cpp b/iceoryx_utils/test/moduletests/test_posix_mutex.cpp index 523bdd55b6..cac3785b02 100644 --- a/iceoryx_utils/test/moduletests/test_posix_mutex.cpp +++ b/iceoryx_utils/test/moduletests/test_posix_mutex.cpp @@ -24,11 +24,12 @@ using namespace iox::units::duration_literals; class Mutex_test : public Test { public: - class MutexMock : public iox::posix::mutex + using mutex_t = iox::posix::mutex; + class MutexMock : public mutex_t { public: - MutexMock(const bool isRecursive) - : iox::posix::mutex(isRecursive) + MutexMock(const Recursive recursive, const Robust robust) + : mutex_t(recursive, robust) { } }; @@ -47,7 +48,7 @@ class Mutex_test : public Test } } - iox::posix::mutex sut{false}; + mutex_t sut{mutex_t::Recursive::OFF, mutex_t::Robust::OFF}; }; TEST_F(Mutex_test, TryLockWithNoLock) @@ -68,33 +69,3 @@ TEST_F(Mutex_test, LockAndUnlock) EXPECT_THAT(sut.lock(), Eq(true)); EXPECT_THAT(sut.unlock(), Eq(true)); } - -// in qnx you can destroy a locked mutex, without error if the thread holding the lock is destructing it. -TEST_F(Mutex_test, DestructorFailsOnLockedMutex) -{ - std::string output = internal::GetCapturedStderr(); - std::set_terminate([]() { std::cout << "", std::abort(); }); - - EXPECT_DEATH( - { - std::thread* t; - { - iox::posix::mutex mtx{false}; - iox::posix::Timer hold(1000_ms); - t = new std::thread([&] { - mtx.lock(); - iox::posix::Timer ct(5000_ms); - while (!ct.hasExpiredComparedToCreationTime()) // come back in any case! - ; - }); - - while (!hold.hasExpiredComparedToCreationTime()) - ; - } - t->join(); - delete t; - }, - ".*"); - - internal::CaptureStderr(); -}