You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Bill Stoddard <bi...@wstoddard.com> on 2004/01/08 16:21:55 UTC
Re: cvs commit: httpd-2.0/modules/experimental mod_mem_cache.c
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;
> }
>
>
>
>
>
Re: cvs commit: httpd-2.0/modules/experimental mod_mem_cache.c
Posted by Bill Stoddard <bi...@wstoddard.com>.
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;
>> }
>>
>
>
>