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 { diff --git a/src/libipc/platform/posix/mutex.h b/src/libipc/platform/posix/mutex.h index 2293678..8a4ac5f 100644 --- a/src/libipc/platform/posix/mutex.h +++ b/src/libipc/platform/posix/mutex.h @@ -150,6 +150,17 @@ class mutex { 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 @@ class mutex { 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; @@ -206,21 +220,17 @@ class mutex { 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; } - 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; - } + // After calling pthread_mutex_consistent(), the mutex is now in a + // consistent state and we hold the lock. Return success. + return true; } - break; // loop again default: ipc::error("fail pthread_mutex_lock[%d]\n", eno); return false; @@ -238,21 +248,17 @@ class mutex { 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); - 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()}; } + // After calling pthread_mutex_consistent(), the mutex is now in a + // consistent state and we hold the lock. Return success. + 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 @@ class semaphore { 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 @@ class semaphore { 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 @@ class semaphore { } 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); }