Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Message-ID: <3FFD75EE.3050808@wstoddard.com>
Date: Thu, 08 Jan 2004 10:23:26 -0500
From: Bill Stoddard <bill@wstoddard.com>
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US;
 rv:1.4) Gecko/20030624
MIME-Version: 1.0
To: dev@httpd.apache.org
Subject: Re: cvs commit: httpd-2.0/modules/experimental mod_mem_cache.c
References: <20040107193516.71865.qmail@minotaur.apache.org>
 <3FFD7593.7050508@wstoddard.com>
In-Reply-To: <3FFD7593.7050508@wstoddard.com>
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit

Here is the patch that fill fix the problem reported by 21287

http://cvs.apache.org/viewcvs.cgi/httpd-2.0/modules/experimental/mod_mem_cache.c?r1=1.99&r2=1.100

Bill
> Hi Jean-Jacques,
> What specific problem is this patch correcting? I committed a fix for 
> 21287 prior to the holidays. The idea behind using atomic operators on 
> refcount is to avoid the need to acquire the mutex when 
> incrementing/decrementing refcount.
> 
> Bill
> 
> clar@apache.org wrote:
> 
>> clar        2004/01/07 11:35:16
>>
>>   Modified:    modules/experimental mod_mem_cache.c
>>   Log:
>>   Fix for  Bug 21287.
>>   Patch extracted and modified from attachment in Bug 21285.
>>   The submitted patch was combining fix for 21285 and 21287.
>>   It is extending the mutex protection in decrement_refcount() and 
>> remove_url().
>>     Revision  Changes    Path
>>   1.103     +9 -26     httpd-2.0/modules/experimental/mod_mem_cache.c
>>     Index: mod_mem_cache.c
>>   ===================================================================
>>   RCS file: /home/cvs/httpd-2.0/modules/experimental/mod_mem_cache.c,v
>>   retrieving revision 1.102
>>   retrieving revision 1.103
>>   diff -u -r1.102 -r1.103
>>   --- mod_mem_cache.c    6 Jan 2004 21:57:50 -0000    1.102
>>   +++ mod_mem_cache.c    7 Jan 2004 19:35:16 -0000    1.103
>>   @@ -348,33 +348,26 @@
>>                cache_remove(sconf->cache_cache, obj);
>>                obj->cleanup = 1;
>>            }
>>   -        if (sconf->lock) {
>>   -            apr_thread_mutex_unlock(sconf->lock);
>>   -        }
>>   +    }
>>   +    else if (sconf->lock) {
>>   +        apr_thread_mutex_lock(sconf->lock);
>>        }
>>           /* Cleanup the cache object */
>>    #ifdef USE_ATOMICS
>>   -    if (!apr_atomic_dec32(&obj->refcount)) {
>>   -        if (obj->cleanup) {
>>   -            cleanup_cache_object(obj);
>>   -        }
>>   -    }
>>   +    if (!apr_atomic_dec32(&obj->refcount) && (obj->cleanup)) {
>>    #else
>>   -    if (sconf->lock) {
>>   -        apr_thread_mutex_lock(sconf->lock);
>>   -    }
>>        obj->refcount--;
>>        /* If the object is marked for cleanup and the refcount
>>         * has dropped to zero, cleanup the object
>>         */
>>        if ((obj->cleanup) && (!obj->refcount)) {
>>   +#endif
>>            cleanup_cache_object(obj);
>>        }
>>        if (sconf->lock) {
>>            apr_thread_mutex_unlock(sconf->lock);
>>        }
>>   -#endif
>>        return APR_SUCCESS;
>>    }
>>    static apr_status_t cleanup_cache_mem(void *sconfv)
>>   @@ -739,35 +732,25 @@
>>             obj = cache_find(sconf->cache_cache, key);              if 
>> (obj) {
>>   -        mem_cache_object_t *mobj;
>>            cache_remove(sconf->cache_cache, obj);
>>   -        mobj = (mem_cache_object_t *) obj->vobj;
>>       #ifdef USE_ATOMICS
>>            /* Refcount increment in this case MUST be made under 
>>             * protection of the lock             */
>>            apr_atomic_inc32(&obj->refcount);
>>   +        obj->cleanup = 1;
>>   +        if (!apr_atomic_dec32(&obj->refcount)) {
>>    #else
>>   +        obj->cleanup = 1;
>>            if (!obj->refcount) {
>>   -            cleanup_cache_object(obj);
>>   -            obj = NULL;
>>   -        }
>>    #endif
>>   -        if (obj) {
>>   -            obj->cleanup = 1;
>>   +            cleanup_cache_object(obj);
>>            }
>>        }
>>        if (sconf->lock) {
>>            apr_thread_mutex_unlock(sconf->lock);
>>        }
>>   -#ifdef USE_ATOMICS
>>   -    if (obj) {
>>   -        if (!apr_atomic_dec32(&obj->refcount)) {
>>   -            cleanup_cache_object(obj);
>>   -        }
>>   -    }
>>   -#endif
>>        return OK;
>>    }
>>         
> 
> 
>