Merge r1733694, r1733708 from trunk:

apr_proc_mutex-pthread: Refcount shared mutexes usage to avoid
destruction while still is use by some process(es).
PR 49504.
 
apr_proc_mutex-pthread: follow up to r1733694.
Simplify #if/#else/#endif logic, no functional change.

Note: this merge additionnaly fixes the munmap()ing size of the
pthread_interproc mutex to match the mmap()ed one, which was done in
trunk by r1733775 (barely related and backported later), hence the
additional change for *this* bugfix to be self contained.



git-svn-id: http://svn.apache.org/repos/asf/apr/apr/branches/1.5.x@1738800 13f79535-47bb-0310-9956-ffa450edef68

2 files changed, 57 insertions, 13 deletions

diff --git a/CHANGES b/CHANGES
index 73d419326..061764fdd 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,10 @@
                                                      -*- coding: utf-8 -*-
 Changes for APR 1.5.3
 
+  *) apr_proc_mutex-pthread: Refcount shared mutexes usage to avoid
+     destruction while still is use by some process(es).  PR 49504.
+     [Yann Ylavic]
+
   *) apr_mcast_interface/apr_mcast_join: Fix comparison of IPv6 addresses.
      [ Andre Naujoks <nautsch2 gmail com> ]
 
diff --git a/locks/unix/proc_mutex.c b/locks/unix/proc_mutex.c
index 32012a761..13ba2ea67 100644
--- a/locks/unix/proc_mutex.c
+++ b/locks/unix/proc_mutex.c
@@ -19,6 +19,7 @@
 #include "apr_arch_proc_mutex.h"
 #include "apr_arch_file_io.h" /* for apr_mkstemp() */
 #include "apr_hash.h"
+#include "apr_atomic.h"
 
 APR_DECLARE(apr_status_t) apr_proc_mutex_destroy(apr_proc_mutex_t *mutex)
 {
@@ -325,7 +326,24 @@ static const apr_proc_mutex_unix_lock_methods_t mutex_sysv_methods =
 
 #if APR_HAS_PROC_PTHREAD_SERIALIZE
 
-static apr_status_t proc_mutex_proc_pthread_cleanup(void *mutex_)
+/* The mmap()ed pthread_interproc is the native pthread_mutex_t followed
+ * by a refcounter to track children using it.  We want to avoid calling
+ * pthread_mutex_destroy() on the shared mutex area while it is in use by
+ * another process, because this may mark the shared pthread_mutex_t as
+ * invalid for everyone, including forked children (unlike "sysvsem" for
+ * example), causing unexpected errors or deadlocks (PR 49504).  So the
+ * last process (parent or child) referencing the mutex will effectively
+ * destroy it.
+ */
+typedef struct {
+    pthread_mutex_t mutex;
+    apr_uint32_t refcount;
+} proc_pthread_mutex_t;
+
+#define proc_pthread_mutex_refcount(m) \
+    (((proc_pthread_mutex_t *)(m)->pthread_interproc)->refcount)
+
+static apr_status_t proc_pthread_mutex_unref(void *mutex_)
 {
     apr_proc_mutex_t *mutex=mutex_;
     apr_status_t rv;
@@ -338,8 +356,7 @@ static apr_status_t proc_mutex_proc_pthread_cleanup(void *mutex_)
             return rv;
         }
     }
-    /* curr_locked is set to -1 until the mutex has been created */
-    if (mutex->curr_locked != -1) {
+    if (!apr_atomic_dec32(&proc_pthread_mutex_refcount(mutex))) {
         if ((rv = pthread_mutex_destroy(mutex->pthread_interproc))) {
 #ifdef HAVE_ZOS_PTHREADS
             rv = errno;
@@ -347,7 +364,22 @@ static apr_status_t proc_mutex_proc_pthread_cleanup(void *mutex_)
             return rv;
         }
     }
-    if (munmap((caddr_t)mutex->pthread_interproc, sizeof(pthread_mutex_t))) {
+    return APR_SUCCESS;
+}
+
+static apr_status_t proc_mutex_proc_pthread_cleanup(void *mutex_)
+{
+    apr_proc_mutex_t *mutex=mutex_;
+    apr_status_t rv;
+
+    /* curr_locked is set to -1 until the mutex has been created */
+    if (mutex->curr_locked != -1) {
+        if ((rv = proc_pthread_mutex_unref(mutex))) {
+            return rv;
+        }
+    }
+    if (munmap((caddr_t)mutex->pthread_interproc,
+               sizeof(proc_pthread_mutex_t))) {
         return errno;
     }
     return APR_SUCCESS;
@@ -367,7 +399,7 @@ static apr_status_t proc_mutex_proc_pthread_create(apr_proc_mutex_t *new_mutex,
 
     new_mutex->pthread_interproc = (pthread_mutex_t *)mmap(
                                        (caddr_t) 0, 
-                                       sizeof(pthread_mutex_t), 
+                                       sizeof(proc_pthread_mutex_t),
                                        PROT_READ | PROT_WRITE, MAP_SHARED,
                                        fd, 0); 
     if (new_mutex->pthread_interproc == (pthread_mutex_t *) (caddr_t) -1) {
@@ -423,6 +455,7 @@ static apr_status_t proc_mutex_proc_pthread_create(apr_proc_mutex_t *new_mutex,
         return rv;
     }
 
+    proc_pthread_mutex_refcount(new_mutex) = 1; /* first/parent reference */
     new_mutex->curr_locked = 0; /* mutex created now */
 
     if ((rv = pthread_mutexattr_destroy(&mattr))) {
@@ -440,6 +473,17 @@ static apr_status_t proc_mutex_proc_pthread_create(apr_proc_mutex_t *new_mutex,
     return APR_SUCCESS;
 }
 
+static apr_status_t proc_mutex_proc_pthread_child_init(apr_proc_mutex_t **mutex,
+                                                       apr_pool_t *pool, 
+                                                       const char *fname)
+{
+    (*mutex)->curr_locked = 0;
+    apr_atomic_inc32(&proc_pthread_mutex_refcount(*mutex));
+    apr_pool_cleanup_register(pool, *mutex, proc_pthread_mutex_unref, 
+                              apr_pool_cleanup_null);
+    return APR_SUCCESS;
+}
+
 static apr_status_t proc_mutex_proc_pthread_acquire(apr_proc_mutex_t *mutex)
 {
     apr_status_t rv;
@@ -451,13 +495,12 @@ static apr_status_t proc_mutex_proc_pthread_acquire(apr_proc_mutex_t *mutex)
 #ifdef HAVE_PTHREAD_MUTEX_ROBUST
         /* Okay, our owner died.  Let's try to make it consistent again. */
         if (rv == EOWNERDEAD) {
+            apr_atomic_dec32(&proc_pthread_mutex_refcount(mutex));
             pthread_mutex_consistent_np(mutex->pthread_interproc);
         }
         else
-            return rv;
-#else
-        return rv;
 #endif
+        return rv;
     }
     mutex->curr_locked = 1;
     return APR_SUCCESS;
@@ -478,13 +521,10 @@ static apr_status_t proc_mutex_proc_pthread_tryacquire(apr_proc_mutex_t *mutex)
         /* Okay, our owner died.  Let's try to make it consistent again. */
         if (rv == EOWNERDEAD) {
             pthread_mutex_consistent_np(mutex->pthread_interproc);
-            rv = APR_SUCCESS;
         }
         else
-            return rv;
-#else
-        return rv;
 #endif
+        return rv;
     }
     mutex->curr_locked = 1;
     return rv;
@@ -512,7 +552,7 @@ static const apr_proc_mutex_unix_lock_methods_t mutex_proc_pthread_methods =
     proc_mutex_proc_pthread_tryacquire,
     proc_mutex_proc_pthread_release,
     proc_mutex_proc_pthread_cleanup,
-    proc_mutex_no_child_init,
+    proc_mutex_proc_pthread_child_init,
     "pthread"
 };