From 543672b39d702fcd661f02d01f4945e21f0929e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9C=A8=E5=A4=B4=E4=BA=91?= Date: Sat, 6 Dec 2025 06:05:53 +0000 Subject: [PATCH 1/4] Fix FreeBSD-specific issues in POSIX implementation This commit addresses the test failures reported in issue #156 on FreeBSD platform. 1. Fix POSIX semaphore naming for FreeBSD compatibility - FreeBSD requires semaphore names to start with "/" - Add "_sem" suffix to avoid namespace conflicts with shm - Store semaphore name separately for proper cleanup - This fixes all 24 semaphore test failures 2. Fix robust mutex EOWNERDEAD handling - When pthread_mutex_lock returns EOWNERDEAD, the lock is already acquired by the calling thread - After calling pthread_mutex_consistent(), we should return success immediately, not unlock and retry - Previous behavior caused issues with FreeBSD's libthr robust mutex list management, leading to fatal errors - This fixes the random crashes in MutexTest Technical details: - EOWNERDEAD indicates the previous lock owner died while holding the lock, but the current thread has successfully acquired it - pthread_mutex_consistent() restores the mutex to a consistent state - The Linux implementation worked differently, but the new approach is more correct according to POSIX semantics and works on both Linux and FreeBSD Fixes #156 --- src/libipc/platform/posix/mutex.h | 22 ++++++------- src/libipc/platform/posix/semaphore_impl.h | 36 ++++++++++++++++------ 2 files changed, 35 insertions(+), 23 deletions(-) diff --git a/src/libipc/platform/posix/mutex.h b/src/libipc/platform/posix/mutex.h index 2293678..ce42929 100644 --- a/src/libipc/platform/posix/mutex.h +++ b/src/libipc/platform/posix/mutex.h @@ -214,13 +214,13 @@ public: ipc::error("fail pthread_mutex_lock[%d], pthread_mutex_consistent[%d]\n", eno, eno2); return false; } - int eno3 = ::pthread_mutex_unlock(mutex_); - if (eno3 != 0) { - ipc::error("fail pthread_mutex_lock[%d], pthread_mutex_unlock[%d]\n", eno, eno3); - return false; - } + // EOWNERDEAD means we have successfully acquired the lock, + // but the previous owner died. After calling pthread_mutex_consistent(), + // the mutex is now in a consistent state and we hold the lock. + // We should return success here, not unlock and retry. + // This avoids issues with FreeBSD's robust mutex list management. + return true; } - break; // loop again default: ipc::error("fail pthread_mutex_lock[%d]\n", eno); return false; @@ -244,15 +244,11 @@ public: int eno2 = ::pthread_mutex_consistent(mutex_); if (eno2 != 0) { ipc::error("fail pthread_mutex_timedlock[%d], pthread_mutex_consistent[%d]\n", eno, eno2); - break; - } - int eno3 = ::pthread_mutex_unlock(mutex_); - if (eno3 != 0) { - ipc::error("fail pthread_mutex_timedlock[%d], pthread_mutex_unlock[%d]\n", eno, eno3); - break; + throw std::system_error{eno2, std::system_category()}; } + // Successfully acquired the lock after making it consistent + return true; } - break; default: ipc::error("fail pthread_mutex_timedlock[%d]\n", eno); break; diff --git a/src/libipc/platform/posix/semaphore_impl.h b/src/libipc/platform/posix/semaphore_impl.h index 6d0da71..8d5a1e8 100644 --- a/src/libipc/platform/posix/semaphore_impl.h +++ b/src/libipc/platform/posix/semaphore_impl.h @@ -19,6 +19,7 @@ namespace sync { class semaphore { ipc::shm::handle shm_; sem_t *h_ = SEM_FAILED; + std::string sem_name_; // Store the actual semaphore name used public: semaphore() = default; @@ -38,9 +39,16 @@ public: ipc::error("[open_semaphore] fail shm.acquire: %s\n", name); return false; } - h_ = ::sem_open(name, O_CREAT, 0666, static_cast(count)); + // POSIX semaphore names must start with "/" on some platforms (e.g., FreeBSD) + // Use a separate namespace for semaphores to avoid conflicts with shm + if (name[0] == '/') { + sem_name_ = std::string(name) + "_sem"; + } else { + sem_name_ = std::string("/") + name + "_sem"; + } + h_ = ::sem_open(sem_name_.c_str(), O_CREAT, 0666, static_cast(count)); if (h_ == SEM_FAILED) { - ipc::error("fail sem_open[%d]: %s\n", errno, name); + ipc::error("fail sem_open[%d]: %s\n", errno, sem_name_.c_str()); return false; } return true; @@ -52,14 +60,14 @@ public: ipc::error("fail sem_close[%d]: %s\n", errno); } h_ = SEM_FAILED; - if (shm_.name() != nullptr) { - std::string name = shm_.name(); + if (!sem_name_.empty() && shm_.name() != nullptr) { if (shm_.release() <= 1) { - if (::sem_unlink(name.c_str()) != 0) { - ipc::error("fail sem_unlink[%d]: %s, name: %s\n", errno, name.c_str()); + if (::sem_unlink(sem_name_.c_str()) != 0) { + ipc::error("fail sem_unlink[%d]: %s, name: %s\n", errno, sem_name_.c_str()); } } } + sem_name_.clear(); } void clear() noexcept { @@ -69,14 +77,22 @@ public: } h_ = SEM_FAILED; } - char const *name = shm_.name(); - if (name == nullptr) return; - ::sem_unlink(name); + if (!sem_name_.empty()) { + ::sem_unlink(sem_name_.c_str()); + sem_name_.clear(); + } shm_.clear(); // Make sure the storage is cleaned up. } static void clear_storage(char const *name) noexcept { - ::sem_unlink(name); + // Construct the semaphore name same way as open() does + std::string sem_name; + if (name[0] == '/') { + sem_name = std::string(name) + "_sem"; + } else { + sem_name = std::string("/") + name + "_sem"; + } + ::sem_unlink(sem_name.c_str()); ipc::shm::handle::clear_storage(name); } From a82e3faf63eb96f84489dd62d3e805744b97639d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9C=A8=E5=A4=B4=E4=BA=91?= Date: Sat, 6 Dec 2025 06:13:58 +0000 Subject: [PATCH 2/4] Fix C++17 compilation error on FreeBSD GCC 13.3 Remove custom deduction guides for std::unique_ptr in C++17 mode. Issue: FreeBSD GCC 13.3 correctly rejects adding deduction guides to namespace std, which violates C++ standard [namespace.std]: "The behavior of a C++ program is undefined if it adds declarations or definitions to namespace std or to a namespace within namespace std." Root cause: The code attempted to add custom deduction guides for std::unique_ptr in namespace std when compiling in C++17 mode. This is not allowed by the C++ standard. Solution: Remove the custom deduction guides for C++17 and later, as the C++17 standard library already provides deduction guides for std::unique_ptr (added in C++17 via P0433R2). The custom deduction guide wrappers in the else branch (for C++14 and earlier) are kept as they provide helper functions, not actual deduction guides in namespace std. Tested-on: FreeBSD 15 with GCC 13.3 Fixes: Compilation error 'deduction guide must be declared in the same scope as template std::unique_ptr' --- src/libipc/platform/detail.h | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/libipc/platform/detail.h b/src/libipc/platform/detail.h index 69e6b89..17ccbb9 100755 --- a/src/libipc/platform/detail.h +++ b/src/libipc/platform/detail.h @@ -72,15 +72,9 @@ #if __cplusplus >= 201703L -namespace std { -// deduction guides for std::unique_ptr -template -unique_ptr(T* p) -> unique_ptr; -template -unique_ptr(T* p, D&& d) -> unique_ptr>; - -} // namespace std +// C++17 and later: std library already provides deduction guides +// No need to add custom ones, just use the standard ones directly namespace ipc { namespace detail { From 47fa303455f543577c3b89d89fb5087da272984f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9C=A8=E5=A4=B4=E4=BA=91?= Date: Sat, 6 Dec 2025 06:21:38 +0000 Subject: [PATCH 3/4] Fix segfault in EOWNERDEAD handling - remove incorrect ref count manipulation Root cause: The previous code incorrectly called shm_->sub_ref() when handling EOWNERDEAD, which could cause the shared memory to be freed prematurely while the mutex pointer was still in use, leading to segmentation fault. Fix: Remove the shm_->sub_ref() call. When EOWNERDEAD is returned, it means we have successfully acquired the lock. We only need to call pthread_mutex_consistent() to make the mutex usable again, then return success. The shared memory reference count should not be modified in this path. This fixes the segfault in MutexTest.TryLockExceptionSafety on FreeBSD 15. --- src/libipc/platform/posix/mutex.h | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/src/libipc/platform/posix/mutex.h b/src/libipc/platform/posix/mutex.h index ce42929..0df842b 100644 --- a/src/libipc/platform/posix/mutex.h +++ b/src/libipc/platform/posix/mutex.h @@ -206,19 +206,15 @@ public: case ETIMEDOUT: return false; case EOWNERDEAD: { - if (shm_->ref() > 1) { - shm_->sub_ref(); - } + // EOWNERDEAD means we have successfully acquired the lock, + // but the previous owner died. We need to make it consistent. int eno2 = ::pthread_mutex_consistent(mutex_); if (eno2 != 0) { ipc::error("fail pthread_mutex_lock[%d], pthread_mutex_consistent[%d]\n", eno, eno2); return false; } - // EOWNERDEAD means we have successfully acquired the lock, - // but the previous owner died. After calling pthread_mutex_consistent(), - // the mutex is now in a consistent state and we hold the lock. - // We should return success here, not unlock and retry. - // This avoids issues with FreeBSD's robust mutex list management. + // After calling pthread_mutex_consistent(), the mutex is now in a + // consistent state and we hold the lock. Return success. return true; } default: @@ -238,15 +234,15 @@ public: case ETIMEDOUT: return false; case EOWNERDEAD: { - if (shm_->ref() > 1) { - shm_->sub_ref(); - } + // EOWNERDEAD means we have successfully acquired the lock, + // but the previous owner died. We need to make it consistent. int eno2 = ::pthread_mutex_consistent(mutex_); if (eno2 != 0) { ipc::error("fail pthread_mutex_timedlock[%d], pthread_mutex_consistent[%d]\n", eno, eno2); throw std::system_error{eno2, std::system_category()}; } - // Successfully acquired the lock after making it consistent + // After calling pthread_mutex_consistent(), the mutex is now in a + // consistent state and we hold the lock. Return success. return true; } default: From 5517f48681f78995932eeac1fba12603c644d183 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9C=A8=E5=A4=B4=E4=BA=91?= Date: Sat, 6 Dec 2025 06:35:02 +0000 Subject: [PATCH 4/4] Fix FreeBSD segfault by unlocking mutex before destruction Root cause: FreeBSD's robust mutex implementation (libthr) maintains a per-thread robust list of mutexes. The error 'rb error 14' (EFAULT - Bad address) indicates that the robust list contained a dangling pointer to a destroyed mutex. When a mutex object is destroyed (via close() or clear()), if the mutex is still in the current thread's robust list, FreeBSD's libthr may try to access it later and encounter an invalid pointer, causing a segmentation fault. This happened in MutexTest.TryLockExceptionSafety because: 1. The test called try_lock() which successfully acquired the lock 2. The test ended without calling unlock() 3. The mutex destructor called close() 4. close() called pthread_mutex_destroy() on a mutex that was: - Still locked by the current thread, OR - Still in the thread's robust list Solution: Call pthread_mutex_unlock() before pthread_mutex_destroy() in both close() and clear() methods. This ensures: 1. The mutex is unlocked if we hold the lock 2. The mutex is removed from the thread's robust list 3. Subsequent pthread_mutex_destroy() is safe We ignore errors from pthread_mutex_unlock() because: - If we don't hold the lock, EPERM is expected and harmless - If the mutex is already unlocked, this is a no-op - Even if there's an error, we still want to proceed with cleanup This fix is platform-agnostic and should not affect Linux/QNX behavior, as both also use pthread robust mutexes with similar semantics. Fixes the segfault in MutexTest.TryLockExceptionSafety on FreeBSD 15. --- src/libipc/platform/posix/mutex.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/libipc/platform/posix/mutex.h b/src/libipc/platform/posix/mutex.h index 0df842b..8a4ac5f 100644 --- a/src/libipc/platform/posix/mutex.h +++ b/src/libipc/platform/posix/mutex.h @@ -150,6 +150,17 @@ public: void close() noexcept { if ((ref_ != nullptr) && (shm_ != nullptr) && (mutex_ != nullptr)) { + // Try to unlock the mutex before destroying it. + // This is important for robust mutexes on FreeBSD, which maintain + // a per-thread robust list. If we destroy a mutex while it's in + // the robust list (even if not locked), FreeBSD may encounter + // dangling pointers later, leading to segfaults. + // We ignore any errors from unlock() since: + // 1. If we don't hold the lock, EPERM is expected and harmless + // 2. If the mutex is already unlocked, this is a no-op + // 3. If there's an error, we still want to proceed with cleanup + ::pthread_mutex_unlock(mutex_); + if (shm_->name() != nullptr) { release_mutex(shm_->name(), [this] { auto self_ref = ref_->fetch_sub(1, std::memory_order_relaxed); @@ -171,6 +182,9 @@ public: void clear() noexcept { if ((shm_ != nullptr) && (mutex_ != nullptr)) { + // Try to unlock before destroying, same reasoning as in close() + ::pthread_mutex_unlock(mutex_); + if (shm_->name() != nullptr) { release_mutex(shm_->name(), [this] { int eno;