Merge pull request #161 from mutouyun/fix/freebsd-mutex-unlock-timing

Fix mutex unlock timing for FreeBSD multi-run stability

Author: mutouyun

src/libipc/platform/posix/mutex.h

@@ -150,21 +150,19 @@ 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);
                     if ((shm_->ref() <= 1) && (self_ref <= 1)) {
+                        // Before destroying the mutex, try to unlock it.
+                        // This is important for robust mutexes on FreeBSD, which maintain
+                        // a per-thread robust list. If we destroy a mutex while it's locked
+                        // or still in the robust list, FreeBSD may encounter dangling pointers
+                        // later, leading to segfaults.
+                        // Only unlock here (when we're the last reference) to avoid
+                        // interfering with other threads that might be using the mutex.
+                        ::pthread_mutex_unlock(mutex_);
+                        
                         int eno;
                         if ((eno = ::pthread_mutex_destroy(mutex_)) != 0) {
                             ipc::error("fail pthread_mutex_destroy[%d]\n", eno);
@@ -182,11 +180,11 @@ 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] {
+                    // Unlock before destroying, same reasoning as in close()
+                    ::pthread_mutex_unlock(mutex_);
+                    
                     int eno;
                     if ((eno = ::pthread_mutex_destroy(mutex_)) != 0) {
                         ipc::error("fail pthread_mutex_destroy[%d]\n", eno);

src/libipc/platform/posix/shm_posix.cpp

@@ -173,7 +173,10 @@ std::int32_t release(id_t id) noexcept {
     else if ((ret = acc_of(ii->mem_, ii->size_).fetch_sub(1, std::memory_order_acq_rel)) <= 1) {
         ::munmap(ii->mem_, ii->size_);
         if (!ii->name_.empty()) {
-            ::shm_unlink(ii->name_.c_str());
+            int unlink_ret = ::shm_unlink(ii->name_.c_str());
+            if (unlink_ret == -1) {
+                ipc::error("fail shm_unlink[%d]: %s\n", errno, ii->name_.c_str());
+            }
         }
     }
     else ::munmap(ii->mem_, ii->size_);
@@ -190,7 +193,10 @@ void remove(id_t id) noexcept {
     auto name = std::move(ii->name_);
     release(id);
     if (!name.empty()) {
-        ::shm_unlink(name.c_str());
+        int unlink_ret = ::shm_unlink(name.c_str());
+        if (unlink_ret == -1) {
+            ipc::error("fail shm_unlink[%d]: %s\n", errno, name.c_str());
+        }
     }
 }
 
@@ -199,7 +205,12 @@ void remove(char const * name) noexcept {
         ipc::error("fail remove: name is empty\n");
         return;
     }
-    ::shm_unlink(name);
+    // For portable use, a shared memory object should be identified by name of the form /somename.
+    ipc::string op_name = ipc::string{"/"} + name;
+    int unlink_ret = ::shm_unlink(op_name.c_str());
+    if (unlink_ret == -1) {
+        ipc::error("fail shm_unlink[%d]: %s\n", errno, op_name.c_str());
+    }
 }
 
 } // namespace shm