diff options
author | wrowe <wrowe@13f79535-47bb-0310-9956-ffa450edef68> | 2003-08-07 22:16:24 +0000 |
---|---|---|
committer | wrowe <wrowe@13f79535-47bb-0310-9956-ffa450edef68> | 2003-08-07 22:16:24 +0000 |
commit | c0265e4c95bffe6678a1c05d58125b13f58d48ba (patch) | |
tree | 2fa2e2aff732dc0513cb255f36cd7ed24bc45cb2 /locks | |
parent | 51512c9451af20a0e763a56b97491034ae972ec4 (diff) | |
download | libapr-c0265e4c95bffe6678a1c05d58125b13f58d48ba.tar.gz |
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.
git-svn-id: http://svn.apache.org/repos/asf/apr/apr/trunk@64579 13f79535-47bb-0310-9956-ffa450edef68
Diffstat (limited to 'locks')
-rw-r--r-- | locks/unix/thread_mutex.c | 80 |
1 files changed, 57 insertions, 23 deletions
diff --git a/locks/unix/thread_mutex.c b/locks/unix/thread_mutex.c index 39062a2bd..e304bcb5c 100644 --- a/locks/unix/thread_mutex.c +++ b/locks/unix/thread_mutex.c @@ -108,8 +108,13 @@ APR_DECLARE(apr_status_t) apr_thread_mutex_lock(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 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 @@ APR_DECLARE(apr_status_t) apr_thread_mutex_lock(apr_thread_mutex_t *mutex) 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_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())) { - mutex->owner_ref++; + apr_atomic_inc(mutex->owner_ref); return APR_SUCCESS; } @@ -154,8 +174,17 @@ APR_DECLARE(apr_status_t) apr_thread_mutex_trylock(apr_thread_mutex_t *mutex) 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_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())) { - 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) |