You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by ia...@apache.org on 2002/02/12 23:11:30 UTC

cvs commit: httpd-2.0/modules/experimental mod_mem_cache.c

ianh        02/02/12 14:11:30

  Modified:    modules/experimental mod_mem_cache.c
  Log:
  fix a race condition.
  2 threads both trying to remove the same URL at the same time.
  
  Revision  Changes    Path
  1.17      +10 -4     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.16
  retrieving revision 1.17
  diff -u -r1.16 -r1.17
  --- mod_mem_cache.c	3 Feb 2002 19:04:15 -0000	1.16
  +++ mod_mem_cache.c	12 Feb 2002 22:11:30 -0000	1.17
  @@ -351,12 +351,18 @@
       if (sconf->lock) {
           apr_thread_mutex_lock(sconf->lock);
       }
  -    apr_hash_set(sconf->cacheht, obj->key, strlen(obj->key), NULL);
  +    /* 
  +     * RACE .. some one might have just deleted this object .. so test
  +     * if it is still around
  +     */
  +    if (obj) {
  +        apr_hash_set(sconf->cacheht, obj->key, strlen(obj->key), NULL);
  +        cleanup_cache_object(obj);
  +        h->cache_obj = NULL;
  +    }
       if (sconf->lock) {
           apr_thread_mutex_unlock(sconf->lock);
  -    }
  -
  -    cleanup_cache_object(obj);
  +    }    
       
       return OK;
   }
  
  
  

Re: cvs commit: httpd-2.0/modules/experimental mod_mem_cache.c

Posted by Ian Holsman <ia...@apache.org>.
oops.. used the wrong email from address to reply.

there is a mutex around this code.
what I changed was to add the cleanup_cache_object into the mutex lock

Ryan Bloom wrote:
> Doesn't the race condition still exist?   For example, I can delete the
> object after the if but before the hash_set.  You need a mutex, don't
> you?
> 
> Ryan
> 
> 
>>  -    apr_hash_set(sconf->cacheht, obj->key, strlen(obj->key), NULL);
>>  +    /*
>>  +     * RACE .. some one might have just deleted this object .. so
>>
> test
> 
>>  +     * if it is still around
>>  +     */
>>  +    if (obj) {
>>  +        apr_hash_set(sconf->cacheht, obj->key, strlen(obj->key),
>>
> NULL);
> 
>>  +        cleanup_cache_object(obj);
>>  +        h->cache_obj = NULL;
>>  +    }
>>       if (sconf->lock) {
>>           apr_thread_mutex_unlock(sconf->lock);
>>  -    }
>>  -
>>  -    cleanup_cache_object(obj);
>>  +    }
>>
>>       return OK;
>>   }
>>
>>
>>
>>
> 




RE: cvs commit: httpd-2.0/modules/experimental mod_mem_cache.c

Posted by Ryan Bloom <rb...@covalent.net>.
Doesn't the race condition still exist?   For example, I can delete the
object after the if but before the hash_set.  You need a mutex, don't
you?

Ryan

>   -    apr_hash_set(sconf->cacheht, obj->key, strlen(obj->key), NULL);
>   +    /*
>   +     * RACE .. some one might have just deleted this object .. so
test
>   +     * if it is still around
>   +     */
>   +    if (obj) {
>   +        apr_hash_set(sconf->cacheht, obj->key, strlen(obj->key),
NULL);
>   +        cleanup_cache_object(obj);
>   +        h->cache_obj = NULL;
>   +    }
>        if (sconf->lock) {
>            apr_thread_mutex_unlock(sconf->lock);
>   -    }
>   -
>   -    cleanup_cache_object(obj);
>   +    }
> 
>        return OK;
>    }
> 
> 
> 


RE: cvs commit: httpd-2.0/modules/experimental mod_mem_cache.c

Posted by Ryan Bloom <rb...@covalent.net>.
Doesn't the race condition still exist?   For example, I can delete the
object after the if but before the hash_set.  You need a mutex, don't
you?

Ryan

>   -    apr_hash_set(sconf->cacheht, obj->key, strlen(obj->key), NULL);
>   +    /*
>   +     * RACE .. some one might have just deleted this object .. so
test
>   +     * if it is still around
>   +     */
>   +    if (obj) {
>   +        apr_hash_set(sconf->cacheht, obj->key, strlen(obj->key),
NULL);
>   +        cleanup_cache_object(obj);
>   +        h->cache_obj = NULL;
>   +    }
>        if (sconf->lock) {
>            apr_thread_mutex_unlock(sconf->lock);
>   -    }
>   -
>   -    cleanup_cache_object(obj);
>   +    }
> 
>        return OK;
>    }
> 
> 
>