apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "William A. Rowe, Jr." <wr...@rowe-clan.net>
Subject Re: apr_atomic_cas broken in 0.9.x
Date Fri, 22 Sep 2006 23:13:40 GMT
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 <troyh@debian.org> writes:
> 
>> In any case, this looks like the culprit.
> 
> Agreed, I've cc'd dev@a.a.o.  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;
> }
> 


Mime
View raw message