Bug#387610: [PATCH] apr_atomic_cas broken in 0.9.x

[William A. Rowe, Jr.]

Philip Martin wrote:
 Philip Martin [EMAIL PROTECTED] writes:
 
 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.
 
 A less radical change to the code is simply to truncate the 64 bit
 values and rely on the application not to need those bits.  This
 should be enough to fix Subversion which only uses small +ve integers.

Thanks Filip!

This sure points out the need for 64 bit atomics but the compromise looks
great to solve the existing issue.

Thanks to Garrett for jumping on this patch.


-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]

Bug#387610: apr_atomic_cas broken in 0.9.x

[William A. Rowe, Jr.]

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?

Bill

Philip Martin wrote:
 Troy Heber [EMAIL PROTECTED] writes:
 
 In any case, this looks like the culprit.
 
 Agreed, I've cc'd [EMAIL PROTECTED]  The C implementation of apr_atomic_cas
 on the 0.9.x branch is broken on ia64, and probably any other 64 bit
 platform that uses the same code.  For the full story see:
 http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=387610
 
 apr_uint32_t apr_atomic_cas(volatile apr_uint32_t *mem, long with,
 long cmp)
 {
   long 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;

 On a 64-bit machine we end up with a size mismatch and a compare of
 junk. mem is defined as a pointer to a 32-bit, then a cast to long
 64-bit in this case. prev ends up with junk it it and fails the
 compare prev == cmp that passes on a 32-bit box. In any case this is a
 bug, not positive if it's the only one. 
 
 It looks like it has already been fixed in apr1.2 (as used by apache2.2):
 
 apr_uint32_t apr_atomic_cas32(volatile apr_uint32_t *mem, apr_uint32_t with,
   apr_uint32_t cmp)
 {
 apr_uint32_t prev;
 #if APR_HAS_THREADS
 apr_thread_mutex_t *lock = hash_mutex[ATOMIC_HASH(mem)];
 
 CHECK(apr_thread_mutex_lock(lock));
 prev = *mem;
 if (prev == cmp) {
 *mem = with;
 }
 CHECK(apr_thread_mutex_unlock(lock));
 #else
 prev = *mem;
 if (prev == cmp) {
 *mem = with;
 }
 #endif /* APR_HAS_THREADS */
 return prev;
 }
 



-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]