From 37f16cea4715e3cc3c2b20d132610fd863057f21 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E6=9C=A8=E5=A4=B4=E4=BA=91?= <orz@orzz.org>
Date: Sat, 6 Dec 2025 06:52:56 +0000
Subject: [PATCH] Fix mutex unlock timing to avoid interfering with other
 references

Problem: The previous fix unconditionally called pthread_mutex_unlock() at the
beginning of close(), which could interfere with other threads/processes that
still had valid references to the mutex. This caused test failures on FreeBSD
when running tests multiple times (ShmTest.RemoveByName would fail on the second run).

Root cause: Calling unlock() too early could affect the mutex state for other
references that are still using it, leading to unexpected behavior.

Solution: Move pthread_mutex_unlock() to only be called when we're about to
destroy the mutex (i.e., when we're the last reference: shm_->ref() <= 1 &&
self_ref <= 1). This ensures:
1. We don't interfere with other threads/processes using the mutex
2. We still unlock before destroying to avoid FreeBSD robust list issues
3. The unlock happens at the correct time - right before pthread_mutex_destroy()

This is the correct approach because:
- Only the last reference holder should clean up the mutex
- Unlocking should be paired with destroying for the final cleanup
- Other references should not be affected by one reference closing

Fixes the second-run test failure on FreeBSD while maintaining the segfault fix.
---
 src/libipc/platform/posix/mutex.h | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/src/libipc/platform/posix/mutex.h b/src/libipc/platform/posix/mutex.h
index 8a4ac5f..a19ace8 100644
--- a/src/libipc/platform/posix/mutex.h
+++ b/src/libipc/platform/posix/mutex.h
@@ -150,21 +150,19 @@ 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);
                     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 @@ 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] {
+                    // 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);