apr-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From wr...@apache.org
Subject cvs commit: apr/locks/unix thread_mutex.c
Date Thu, 07 Aug 2003 22:16:24 GMT
wrowe       2003/08/07 15:16:24

  Modified:    include/arch/unix apr_arch_thread_mutex.h
               locks/unix thread_mutex.c
  Log:
    Revamp apr_thread_mutex to be as thread safe, and occasionally
    as reentrant as possible.  Switched to atomics to preserve the
    incr/decr integrity.
  
    Although we previously reset the thread_id to zero, it's been
    pointed out on list that this is less than desireable.  However,
    negative one isn't necessarily a good choice because several
    platforms have unsigned thread_t's.
  
  Revision  Changes    Path
  1.2       +3 -2      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.1
  retrieving revision 1.2
  diff -u -r1.1 -r1.2
  --- apr_arch_thread_mutex.h	6 Jan 2003 23:44:26 -0000	1.1
  +++ apr_arch_thread_mutex.h	7 Aug 2003 22:16:24 -0000	1.2
  @@ -60,6 +60,7 @@
   #include "apr_general.h"
   #include "apr_thread_mutex.h"
   #include "apr_portable.h"
  +#include "apr_atomic.h"
   
   #if APR_HAVE_PTHREAD_H
   #include <pthread.h>
  @@ -69,8 +70,8 @@
   struct apr_thread_mutex_t {
       apr_pool_t *pool;
       pthread_mutex_t mutex;
  -    apr_os_thread_t owner;
  -    int owner_ref;
  +    volatile apr_os_thread_t owner;
  +    volatile apr_atomic_t owner_ref;
       char nested; /* a boolean */
   };
   #endif
  
  
  
  1.19      +57 -23    apr/locks/unix/thread_mutex.c
  
  Index: thread_mutex.c
  ===================================================================
  RCS file: /home/cvs/apr/locks/unix/thread_mutex.c,v
  retrieving revision 1.18
  retrieving revision 1.19
  diff -u -r1.18 -r1.19
  --- thread_mutex.c	8 Jun 2003 13:42:20 -0000	1.18
  +++ thread_mutex.c	7 Aug 2003 22:16:24 -0000	1.19
  @@ -108,8 +108,13 @@
       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())) {
  -            mutex->owner_ref++;
  +            apr_atomic_inc(mutex->owner_ref);
               return APR_SUCCESS;
           }
   
  @@ -121,8 +126,17 @@
               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();
  -        mutex->owner_ref = 1;
           return rv;
       }
       else {
  @@ -141,8 +155,14 @@
       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())) {
  -            mutex->owner_ref++;
  +            apr_atomic_inc(mutex->owner_ref);
               return APR_SUCCESS;
           }
   
  @@ -154,8 +174,17 @@
               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();
  -        mutex->owner_ref = 1;
       }
       else {
           rv = pthread_mutex_trylock(&mutex->mutex);
  @@ -175,32 +204,37 @@
       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())) {
  -            mutex->owner_ref--;
  -            if (mutex->owner_ref > 0)
  +            /*
  +             * 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 = 0;
           }
  -        status = pthread_mutex_unlock(&mutex->mutex);
  -        if (status) {
  -#ifdef PTHREAD_SETS_ERRNO
  -            status = errno;
  -#endif
  -            return status;
  +        /*
  +         * This should never occur, and indicates an application error
  +         */
  +        else {
  +            return APR_EINVAL;
           }
  -
  -        memset(&mutex->owner, 0, sizeof mutex->owner);
  -        mutex->owner_ref = 0;
  -        return status;
       }
  -    else {
  -        status = pthread_mutex_unlock(&mutex->mutex);
  +
  +    status = pthread_mutex_unlock(&mutex->mutex);
   #ifdef PTHREAD_SETS_ERRNO
  -        if (status) {
  -            status = errno;
  -        }
  -#endif
  -        return status;
  +    if (status) {
  +        status = errno;
       }
  +#endif
  +
  +    return status;
   }
   
   APR_DECLARE(apr_status_t) apr_thread_mutex_destroy(apr_thread_mutex_t *mutex)
  
  
  

Mime
View raw message