"William A. Rowe, Jr." <[EMAIL PROTECTED]> writes:
> Philip or Troy - would you care to prepare and test the backport to ensure
> we can commit this for the next 0.9 release, coming within days?
The 1.2.x branch has changed the name/signature to apr_atomic_cas32
and no longer passes longs; it's not really clear what the 0.9.x
branch should do with the high bits of the longs.
There's another problem with the 0.9.x code, the threaded
implementation provides no indication to the caller when mutex
operations fail, although most mutex implementations are unlikely to
fail. The 1.2.x code aborts when a mutex fails, so assuming the same
should occur when high bits are set here's a patch. I don't have an
IA64 but I believe the code gets used on x86:
APR Atomic Test
===============
Initializing the context OK
Initializing the atomics OK
testing apr_atomic_dec OK
testing CAS OK
testing CAS - match non-null OK
testing CAS - no match OK
testing CAS for pointers OK
testing CAS for pointers - match non-null OK
testing CAS for pointers - no match OK
testing add OK
testing add/inc OK
Starting all the threads OK
Waiting for threads to exit
(Note that this may take a while to complete.) OK
Checking if atomic worked OK
Checking if nolock worked no surprise
The no-locks didn't work. z = 864045 instead of 1000000
Port some of the atomic code from 1.2.x to 0.9.x, in particular make
mutex operations that fail cause an abort and make the generic C
implementation of apr_atomic_cas work on 64 bit platforms.
Index: atomic/unix/apr_atomic.c
===================================================================
--- atomic/unix/apr_atomic.c (revision 449122)
+++ atomic/unix/apr_atomic.c (working copy)
@@ -18,6 +18,8 @@
#include "apr_atomic.h"
#include "apr_thread_mutex.h"
+#include <stdlib.h>
+
#if !defined(apr_atomic_init) && !defined(APR_OVERRIDE_ATOMIC_INIT)
#if APR_HAS_THREADS
@@ -46,18 +48,18 @@
}
#endif /*!defined(apr_atomic_init) && !defined(APR_OVERRIDE_ATOMIC_INIT) */
+/* abort() if 'x' does not evaluate to APR_SUCCESS. */
+#define CHECK(x) do { if ((x) != APR_SUCCESS) abort(); } while (0)
+
#if !defined(apr_atomic_add) && !defined(APR_OVERRIDE_ATOMIC_ADD)
void apr_atomic_add(volatile apr_atomic_t *mem, apr_uint32_t val)
{
#if APR_HAS_THREADS
apr_thread_mutex_t *lock = hash_mutex[ATOMIC_HASH(mem)];
- apr_uint32_t prev;
- if (apr_thread_mutex_lock(lock) == APR_SUCCESS) {
- prev = *mem;
- *mem += val;
- apr_thread_mutex_unlock(lock);
- }
+ CHECK(apr_thread_mutex_lock(lock));
+ *mem += val;
+ CHECK(apr_thread_mutex_unlock(lock));
#else
*mem += val;
#endif /* APR_HAS_THREADS */
@@ -69,13 +71,10 @@
{
#if APR_HAS_THREADS
apr_thread_mutex_t *lock = hash_mutex[ATOMIC_HASH(mem)];
- apr_uint32_t prev;
- if (apr_thread_mutex_lock(lock) == APR_SUCCESS) {
- prev = *mem;
- *mem = val;
- apr_thread_mutex_unlock(lock);
- }
+ CHECK(apr_thread_mutex_lock(lock));
+ *mem = val;
+ CHECK(apr_thread_mutex_unlock(lock));
#else
*mem = val;
#endif /* APR_HAS_THREADS */
@@ -87,13 +86,10 @@
{
#if APR_HAS_THREADS
apr_thread_mutex_t *lock = hash_mutex[ATOMIC_HASH(mem)];
- apr_uint32_t prev;
- if (apr_thread_mutex_lock(lock) == APR_SUCCESS) {
- prev = *mem;
- (*mem)++;
- apr_thread_mutex_unlock(lock);
- }
+ CHECK(apr_thread_mutex_lock(lock));
+ (*mem)++;
+ CHECK(apr_thread_mutex_unlock(lock));
#else
(*mem)++;
#endif /* APR_HAS_THREADS */
@@ -107,12 +103,11 @@
apr_thread_mutex_t *lock = hash_mutex[ATOMIC_HASH(mem)];
apr_uint32_t new;
- if (apr_thread_mutex_lock(lock) == APR_SUCCESS) {
- (*mem)--;
- new = *mem;
- apr_thread_mutex_unlock(lock);
- return new;
- }
+ CHECK(apr_thread_mutex_lock(lock));
+ (*mem)--;
+ new = *mem;
+ CHECK(apr_thread_mutex_unlock(lock));
+ return new;
#else
(*mem)--;
#endif /* APR_HAS_THREADS */
@@ -123,26 +118,28 @@
#if !defined(apr_atomic_cas) && !defined(APR_OVERRIDE_ATOMIC_CAS)
apr_uint32_t apr_atomic_cas(volatile apr_uint32_t *mem, long with, long cmp)
{
- long prev;
+ apr_uint32_t prev;
#if APR_HAS_THREADS
apr_thread_mutex_t *lock = hash_mutex[ATOMIC_HASH(mem)];
- if (apr_thread_mutex_lock(lock) == APR_SUCCESS) {
- prev = *(long*)mem;
- if (prev == cmp) {
- *(long*)mem = with;
- }
- apr_thread_mutex_unlock(lock);
- return prev;
+ if ((long)(apr_uint32_t)with != with || (long)(apr_uint32_t)cmp != cmp)
+ abort();
+ CHECK(apr_thread_mutex_lock(lock));
+ prev = *mem;
+ if (prev == (apr_uint32_t)cmp) {
+ *mem = (apr_uint32_t)with;
}
- return *(long*)mem;
+ CHECK(apr_thread_mutex_unlock(lock));
#else
- prev = *(long*)mem;
- if (prev == cmp) {
- *(long*)mem = with;
+ if ((long)(apr_uint32_t)with != with || (long)(apr_uint32_t)cmp != cmp) {
+ abort();
}
- return prev;
+ prev = *mem;
+ if (prev == (apr_uint32_t)cmp) {
+ *mem = (apr_uint32_t)with;
+ }
#endif /* APR_HAS_THREADS */
+ return prev;
}
#endif /*!defined(apr_atomic_cas) && !defined(APR_OVERRIDE_ATOMIC_CAS) */
@@ -153,21 +150,18 @@
#if APR_HAS_THREADS
apr_thread_mutex_t *lock = hash_mutex[ATOMIC_HASH(mem)];
- if (apr_thread_mutex_lock(lock) == APR_SUCCESS) {
- prev = *(void **)mem;
- if (prev == cmp) {
- *mem = with;
- }
- apr_thread_mutex_unlock(lock);
- return prev;
+ CHECK(apr_thread_mutex_lock(lock));
+ prev = *(void **)mem;
+ if (prev == cmp) {
+ *mem = with;
}
- return *(void **)mem;
+ CHECK(apr_thread_mutex_unlock(lock));
#else
prev = *(void **)mem;
if (prev == cmp) {
*mem = with;
}
- return prev;
#endif /* APR_HAS_THREADS */
+ return prev;
}
#endif /*!defined(apr_atomic_cas) && !defined(APR_OVERRIDE_ATOMIC_CAS) */
--
Philip Martin
