apr-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jor...@apache.org
Subject svn commit: r106085 - /apr/apr/branches/0.9.x/build/apr_threads.m4 /apr/apr/branches/0.9.x/configure.in /apr/apr/branches/0.9.x/include/arch/unix/apr_arch_thread_mutex.h /apr/apr/branches/0.9.x/locks/unix/thread_mutex.c
Date Sun, 21 Nov 2004 12:58:06 GMT
Author: jorton
Date: Sun Nov 21 04:58:06 2004
New Revision: 106085

Modified:
   apr/apr/branches/0.9.x/build/apr_threads.m4
   apr/apr/branches/0.9.x/configure.in
   apr/apr/branches/0.9.x/include/arch/unix/apr_arch_thread_mutex.h
   apr/apr/branches/0.9.x/locks/unix/thread_mutex.c
Log:
Merge r65157, r65158 from trunk: (fixing random lockups in pool-debug
builds on x86_64)

Drop racy/broken Unix nested mutex implementation; use SUSv3-style
recursive mutex support if available:

* build/apr_threads.m4 (APR_CHECK_PTHREAD_RECURSIVE_MUTEX): New macro.

* configure.in: Use it.

* include/arch/unix/apr_arch_thread_mutex.h (struct
apr_thread_mutex_t): Drop nested mutex tracking fields.

* locks/unix/thread_mutex.c (apr_thread_mutex_create): Return ENOTIMPL
if lacking recursive mutex support, else create a recursive mutex.
(apr_thread_mutex_lock, apr_thread_mutex_unlock,
apr_thread_mutex_trylock): Remove nested mutex tracking.

* build/apr_threads.m4 (APR_CHECK_PTHREAD_RECURSIVE_MUTEX): Run rather
than just compile the test program.



Modified: apr/apr/branches/0.9.x/build/apr_threads.m4
==============================================================================
--- apr/apr/branches/0.9.x/build/apr_threads.m4	(original)
+++ apr/apr/branches/0.9.x/build/apr_threads.m4	Sun Nov 21 04:58:06 2004
@@ -204,3 +204,26 @@
     AC_DEFINE(SIGWAIT_TAKES_ONE_ARG,1,[ ])
   fi
 ])
+
+dnl Check for recursive mutex support (per SUSv3).
+AC_DEFUN([APR_CHECK_PTHREAD_RECURSIVE_MUTEX], [
+  AC_CACHE_CHECK([for recursive mutex support], [apr_cv_mutex_recursive],
+[AC_TRY_RUN([#include <sys/types.h>
+#include <pthread.h>
+#include <stdlib.h>
+
+int main() {
+    pthread_mutexattr_t attr;
+    pthread_mutex_t m;
+
+    exit (pthread_mutexattr_init(&attr) 
+          || pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE)
+          || pthread_mutex_init(&m, &attr));
+}], [apr_cv_mutex_recursive=yes], [apr_cv_mutex_recursive=no], 
+[apr_cv_mutex_recursive=no])])
+
+if test "$apr_cv_mutex_recursive" = "yes"; then
+   AC_DEFINE([HAVE_PTHREAD_MUTEX_RECURSIVE], 1,
+             [Define if recursive pthread mutexes are available])
+fi
+])

Modified: apr/apr/branches/0.9.x/configure.in
==============================================================================
--- apr/apr/branches/0.9.x/configure.in	(original)
+++ apr/apr/branches/0.9.x/configure.in	Sun Nov 21 04:58:06 2004
@@ -555,6 +555,9 @@
                APR_ADDTO(CPPFLAGS, [-D_XOPEN_SOURCE=500 -D_BSD_SOURCE -D_SVID_SOURCE])
             fi
         fi
+        # this might also require -DXOPEN_SOURCE=500, so leave after the
+        # rwlock check.
+        APR_CHECK_PTHREAD_RECURSIVE_MUTEX
     fi
 fi
 

Modified: apr/apr/branches/0.9.x/include/arch/unix/apr_arch_thread_mutex.h
==============================================================================
--- apr/apr/branches/0.9.x/include/arch/unix/apr_arch_thread_mutex.h	(original)
+++ apr/apr/branches/0.9.x/include/arch/unix/apr_arch_thread_mutex.h	Sun Nov 21 04:58:06 2004
@@ -31,9 +31,6 @@
 struct apr_thread_mutex_t {
     apr_pool_t *pool;
     pthread_mutex_t mutex;
-    volatile apr_os_thread_t owner;
-    volatile apr_atomic_t owner_ref;
-    char nested; /* a boolean */
 };
 #endif
 

Modified: apr/apr/branches/0.9.x/locks/unix/thread_mutex.c
==============================================================================
--- apr/apr/branches/0.9.x/locks/unix/thread_mutex.c	(original)
+++ apr/apr/branches/0.9.x/locks/unix/thread_mutex.c	Sun Nov 21 04:58:06 2004
@@ -39,17 +39,37 @@
 {
     apr_thread_mutex_t *new_mutex;
     apr_status_t rv;
+    
+#ifndef HAVE_PTHREAD_MUTEX_RECURSIVE
+    if (flags & APR_THREAD_MUTEX_NESTED) {
+        return APR_ENOTIMPL;
+    }
+#endif
 
     new_mutex = apr_pcalloc(pool, sizeof(apr_thread_mutex_t));
-
     new_mutex->pool = pool;
 
-    /* Optimal default is APR_THREAD_MUTEX_UNNESTED, 
-     * no additional checks required for either flag.
-     */
-    new_mutex->nested = flags & APR_THREAD_MUTEX_NESTED;
+#ifdef HAVE_PTHREAD_MUTEX_RECURSIVE
+    if (flags & APR_THREAD_MUTEX_NESTED) {
+        pthread_mutexattr_t mattr;
+        
+        rv = pthread_mutexattr_init(&mattr);
+        if (rv) return rv;
+        
+        rv = pthread_mutexattr_settype(&mattr, PTHREAD_MUTEX_RECURSIVE);
+        if (rv) {
+            pthread_mutexattr_destroy(&mattr);
+            return rv;
+        }
+         
+        rv = pthread_mutex_init(&new_mutex->mutex, &mattr);
+        
+        pthread_mutexattr_destroy(&mattr);
+    } else
+#endif
+        rv = pthread_mutex_init(&new_mutex->mutex, NULL);
 
-    if ((rv = pthread_mutex_init(&new_mutex->mutex, NULL))) {
+    if (rv) {
 #ifdef PTHREAD_SETS_ERRNO
         rv = errno;
 #endif
@@ -68,127 +88,34 @@
 {
     apr_status_t rv;
 
-    if (mutex->nested) {
-        /*
-         * Although threadsafe, this test is NOT reentrant.  
-         * The thread's main and reentrant attempts will both mismatch 
-         * testing the mutex is owned by this thread, so a deadlock is expected.
-         */
-        if (apr_os_thread_equal(mutex->owner, apr_os_thread_current())) {
-            apr_atomic_inc(&mutex->owner_ref);
-            return APR_SUCCESS;
-        }
-
-        rv = pthread_mutex_lock(&mutex->mutex);
-        if (rv) {
+    rv = pthread_mutex_lock(&mutex->mutex);
 #ifdef PTHREAD_SETS_ERRNO
-            rv = errno;
-#endif
-            return rv;
-        }
-
-        if (apr_atomic_cas(&mutex->owner_ref, 1, 0) != 0) {
-            /* The owner_ref should be zero when the lock is not held,
-             * if owner_ref was non-zero we have a mutex reference bug.
-             * XXX: so now what?
-             */
-            mutex->owner_ref = 1;
-        }
-        /* Note; do not claim ownership until the owner_ref has been
-         * incremented; limits a subtle race in reentrant code.
-         */
-        mutex->owner = apr_os_thread_current();
-        return rv;
+    if (rv) {
+        rv = errno;
     }
-    else {
-        rv = pthread_mutex_lock(&mutex->mutex);
-#ifdef PTHREAD_SETS_ERRNO
-        if (rv) {
-            rv = errno;
-        }
 #endif
-        return rv;
-    }
+    
+    return rv;
 }
 
 APR_DECLARE(apr_status_t) apr_thread_mutex_trylock(apr_thread_mutex_t *mutex)
 {
     apr_status_t rv;
 
-    if (mutex->nested) {
-        /*
-         * Although threadsafe, this test is NOT reentrant.  
-         * The thread's main and reentrant attempts will both mismatch 
-         * testing the mutex is owned by this thread, so one will fail 
-         * the trylock.
-         */
-        if (apr_os_thread_equal(mutex->owner, apr_os_thread_current())) {
-            apr_atomic_inc(&mutex->owner_ref);
-            return APR_SUCCESS;
-        }
-
-        rv = pthread_mutex_trylock(&mutex->mutex);
-        if (rv) {
+    rv = pthread_mutex_trylock(&mutex->mutex);
+    if (rv) {
 #ifdef PTHREAD_SETS_ERRNO
-            rv = errno;
-#endif
-            return (rv == EBUSY) ? APR_EBUSY : rv;
-        }
-
-        if (apr_atomic_cas(&mutex->owner_ref, 1, 0) != 0) {
-            /* The owner_ref should be zero when the lock is not held,
-             * if owner_ref was non-zero we have a mutex reference bug.
-             * XXX: so now what?
-             */
-            mutex->owner_ref = 1;
-        }
-        /* Note; do not claim ownership until the owner_ref has been
-         * incremented; limits a subtle race in reentrant code.
-         */
-        mutex->owner = apr_os_thread_current();
-    }
-    else {
-        rv = pthread_mutex_trylock(&mutex->mutex);
-        if (rv) {
-#ifdef PTHREAD_SETS_ERRNO
-            rv = errno;
+        rv = errno;
 #endif
-            return (rv == EBUSY) ? APR_EBUSY : rv;
-        }
+        return (rv == EBUSY) ? APR_EBUSY : rv;
     }
 
-    return rv;
+    return APR_SUCCESS;
 }
 
-static apr_os_thread_t invalid_thread_id; /* all zeroes */
-
 APR_DECLARE(apr_status_t) apr_thread_mutex_unlock(apr_thread_mutex_t *mutex)
 {
     apr_status_t status;
-
-    if (mutex->nested) {
-        /*
-         * The code below is threadsafe and reentrant.
-         */
-        if (apr_os_thread_equal(mutex->owner, apr_os_thread_current())) {
-            /*
-             * This should never occur, and indicates an application error
-             */
-            if (mutex->owner_ref == 0) {
-                return APR_EINVAL;
-            }
-
-            if (apr_atomic_dec(&mutex->owner_ref) != 0)
-                return APR_SUCCESS;
-            mutex->owner = invalid_thread_id;
-        }
-        /*
-         * This should never occur, and indicates an application error
-         */
-        else {
-            return APR_EINVAL;
-        }
-    }
 
     status = pthread_mutex_unlock(&mutex->mutex);
 #ifdef PTHREAD_SETS_ERRNO

Mime
View raw message