Merge pull request #160 from mutouyun/issue/156

Fix FreeBSD test failures in POSIX implementation

Author: mutouyun

src/libipc/platform/detail.h

@@ -72,15 +72,9 @@
 
 #if __cplusplus >= 201703L
 
-namespace std {
 
-// deduction guides for std::unique_ptr
-template <typename T>
-unique_ptr(T* p) -> unique_ptr<T>;
-template <typename T, typename D>
-unique_ptr(T* p, D&& d) -> unique_ptr<T, std::decay_t<D>>;
-
-} // 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 {

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;

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<unsigned>(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<unsigned>(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);
     }