You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by jo...@apache.org on 2004/06/04 17:23:34 UTC
cvs commit: apr/locks/unix thread_mutex.c
jorton 2004/06/04 08:23:34
Modified: . configure.in
build apr_threads.m4
include/arch/unix apr_arch_thread_mutex.h
locks/unix thread_mutex.c
Log:
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.
Revision Changes Path
1.583 +1 -0 apr/configure.in
Index: configure.in
===================================================================
RCS file: /home/cvs/apr/configure.in,v
retrieving revision 1.582
retrieving revision 1.583
diff -d -w -u -r1.582 -r1.583
--- configure.in 29 May 2004 23:16:38 -0000 1.582
+++ configure.in 4 Jun 2004 15:23:34 -0000 1.583
@@ -577,6 +577,7 @@
if test "$pthreadh" = "1"; then
APR_CHECK_PTHREAD_GETSPECIFIC_TWO_ARGS
APR_CHECK_PTHREAD_ATTR_GETDETACHSTATE_ONE_ARG
+ APR_CHECK_PTHREAD_RECURSIVE_MUTEX
AC_CHECK_FUNCS(pthread_key_delete pthread_rwlock_init)
if test "$ac_cv_func_pthread_rwlock_init" = "yes"; then
1.12 +21 -0 apr/build/apr_threads.m4
Index: apr_threads.m4
===================================================================
RCS file: /home/cvs/apr/build/apr_threads.m4,v
retrieving revision 1.11
retrieving revision 1.12
diff -d -w -u -r1.11 -r1.12
--- apr_threads.m4 16 Apr 2004 12:49:59 -0000 1.11
+++ apr_threads.m4 4 Jun 2004 15:23:34 -0000 1.12
@@ -204,3 +204,24 @@
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_COMPILE([#include <sys/types.h>
+#include <pthread.h>
+#include <stdlib.h>], [
+ 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
+])
1.5 +0 -3 apr/include/arch/unix/apr_arch_thread_mutex.h
Index: apr_arch_thread_mutex.h
===================================================================
RCS file: /home/cvs/apr/include/arch/unix/apr_arch_thread_mutex.h,v
retrieving revision 1.4
retrieving revision 1.5
diff -d -w -u -r1.4 -r1.5
--- apr_arch_thread_mutex.h 13 Feb 2004 09:38:30 -0000 1.4
+++ apr_arch_thread_mutex.h 4 Jun 2004 15:23:34 -0000 1.5
@@ -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_uint32_t owner_ref;
- char nested; /* a boolean */
};
#endif
1.24 +36 -109 apr/locks/unix/thread_mutex.c
Index: thread_mutex.c
===================================================================
RCS file: /home/cvs/apr/locks/unix/thread_mutex.c,v
retrieving revision 1.23
retrieving revision 1.24
diff -d -w -u -r1.23 -r1.24
--- thread_mutex.c 13 Feb 2004 09:38:31 -0000 1.23
+++ thread_mutex.c 4 Jun 2004 15:23:34 -0000 1.24
@@ -40,16 +40,36 @@
apr_thread_mutex_t *new_mutex;
apr_status_t rv;
- new_mutex = apr_pcalloc(pool, sizeof(apr_thread_mutex_t));
+#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;
- if ((rv = pthread_mutex_init(&new_mutex->mutex, NULL))) {
+ 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) {
#ifdef PTHREAD_SETS_ERRNO
rv = errno;
#endif
@@ -68,86 +88,20 @@
{
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_inc32(&mutex->owner_ref);
- return APR_SUCCESS;
- }
-
- rv = pthread_mutex_lock(&mutex->mutex);
- if (rv) {
-#ifdef PTHREAD_SETS_ERRNO
- rv = errno;
-#endif
- return rv;
- }
-
- if (apr_atomic_cas32(&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;
- }
- else {
rv = pthread_mutex_lock(&mutex->mutex);
#ifdef PTHREAD_SETS_ERRNO
if (rv) {
rv = errno;
}
#endif
+
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_inc32(&mutex->owner_ref);
- return APR_SUCCESS;
- }
-
- rv = pthread_mutex_trylock(&mutex->mutex);
- if (rv) {
-#ifdef PTHREAD_SETS_ERRNO
- rv = errno;
-#endif
- return (rv == EBUSY) ? APR_EBUSY : rv;
- }
-
- if (apr_atomic_cas32(&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
@@ -155,40 +109,13 @@
#endif
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_dec32(&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