From: Bill Stoddard Date: Sat, 4 May 2002 19:54:39 +0000 (+0000) Subject: Introduce a dedicated cache hash adt that does not rely on pools. This X-Git-Tag: 2.0.37~514 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=b0f3dfbce6634d3aa487a7937099c0607e8c7a6b;p=apache Introduce a dedicated cache hash adt that does not rely on pools. This cures a storage leak when garbage collecting expired entries out of the hash table. cache_hash is just apr_hash with some of the function removed and reimplemented to use malloc/free in place of apr_pool calls. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@94932 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/modules/experimental/cache_hash.c b/modules/experimental/cache_hash.c new file mode 100644 index 0000000000..7142c1bb3e --- /dev/null +++ b/modules/experimental/cache_hash.c @@ -0,0 +1,322 @@ +/* ==================================================================== + * The Apache Software License, Version 1.1 + * + * Copyright (c) 2000-2002 The Apache Software Foundation. All rights + * reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * + * 3. The end-user documentation included with the redistribution, + * if any, must include the following acknowledgment: + * "This product includes software developed by the + * Apache Software Foundation (http://www.apache.org/)." + * Alternately, this acknowledgment may appear in the software itself, + * if and wherever such third-party acknowledgments normally appear. + * + * 4. The names "Apache" and "Apache Software Foundation" must + * not be used to endorse or promote products derived from this + * software without prior written permission. For written + * permission, please contact apache@apache.org. + * + * 5. Products derived from this software may not be called "Apache", + * nor may "Apache" appear in their name, without prior written + * permission of the Apache Software Foundation. + * + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESSED OR IMPLIED + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + * DISCLAIMED. IN NO EVENT SHALL THE APACHE SOFTWARE FOUNDATION OR + * ITS CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, + * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT + * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * . + * + * Portions of this software are based upon public domain software + * originally written at the National Center for Supercomputing Applications, + * University of Illinois, Urbana-Champaign. + */ + +#include "apr_general.h" + +#include "mod_cache.h" +#include "cache_hash.h" + +#if APR_HAVE_STDLIB_H +#include +#endif +#if APR_HAVE_STRING_H +#include +#endif + + +/* + * The internal form of a hash table. + * + * The table is an array indexed by the hash of the key; collisions + * are resolved by hanging a linked list of hash entries off each + * element of the array. Although this is a really simple design it + * isn't too bad given that pools have a low allocation overhead. + */ + +typedef struct cache_hash_entry_t cache_hash_entry_t; + +struct cache_hash_entry_t { + cache_hash_entry_t *next; + int hash; + const void *key; + apr_ssize_t klen; + const void *val; +}; + +/* + * Data structure for iterating through a hash table. + * + * We keep a pointer to the next hash entry here to allow the current + * hash entry to be freed or otherwise mangled between calls to + * cache_hash_next(). + */ +struct cache_hash_index_t { + cache_hash_t *ht; + cache_hash_entry_t *this, *next; + int index; +}; + +/* + * The size of the array is always a power of two. We use the maximum + * index rather than the size so that we can use bitwise-AND for + * modular arithmetic. + * The count of hash entries may be greater depending on the chosen + * collision rate. + */ +struct cache_hash_t { + cache_hash_entry_t **array; + cache_hash_index_t iterator; /* For cache_hash_first(NULL, ...) */ + int count, max; +}; + +/* + * Hash creation functions. + */ +static cache_hash_entry_t **alloc_array(cache_hash_t *ht, int max) +{ + return calloc(1, sizeof(*ht->array) * (max + 1)); +} + +CACHE_DECLARE(cache_hash_t *) cache_hash_make(apr_size_t size) +{ + cache_hash_t *ht; + ht = malloc(sizeof(cache_hash_t)); + if (!ht) { + return NULL; + } + ht->count = 0; + ht->max = size; + ht->array = alloc_array(ht, ht->max); + if (!ht->array) { + free(ht); + return NULL; + } + return ht; +} + +/* + * Hash iteration functions. + */ + +CACHE_DECLARE(cache_hash_index_t *) cache_hash_next(cache_hash_index_t *hi) +{ + hi->this = hi->next; + while (!hi->this) { + if (hi->index > hi->ht->max) + return NULL; + hi->this = hi->ht->array[hi->index++]; + } + hi->next = hi->this->next; + return hi; +} + +CACHE_DECLARE(cache_hash_index_t *) cache_hash_first(cache_hash_t *ht) +{ + cache_hash_index_t *hi; + + hi = &ht->iterator; + hi->ht = ht; + hi->index = 0; + hi->this = NULL; + hi->next = NULL; + return cache_hash_next(hi); +} + +CACHE_DECLARE(void) cache_hash_this(cache_hash_index_t *hi, + const void **key, + apr_ssize_t *klen, + void **val) +{ + if (key) *key = hi->this->key; + if (klen) *klen = hi->this->klen; + if (val) *val = (void *)hi->this->val; +} + + +/* + * This is where we keep the details of the hash function and control + * the maximum collision rate. + * + * If val is non-NULL it creates and initializes a new hash entry if + * there isn't already one there; it returns an updatable pointer so + * that hash entries can be removed. + */ + +static cache_hash_entry_t **find_entry(cache_hash_t *ht, + const void *key, + apr_ssize_t klen, + const void *val) +{ + cache_hash_entry_t **hep, *he; + const unsigned char *p; + int hash; + apr_ssize_t i; + + /* + * This is the popular `times 33' hash algorithm which is used by + * perl and also appears in Berkeley DB. This is one of the best + * known hash functions for strings because it is both computed + * very fast and distributes very well. + * + * The originator may be Dan Bernstein but the code in Berkeley DB + * cites Chris Torek as the source. The best citation I have found + * is "Chris Torek, Hash function for text in C, Usenet message + * <27038@mimsy.umd.edu> in comp.lang.c , October, 1990." in Rich + * Salz's USENIX 1992 paper about INN which can be found at + * . + * + * The magic of number 33, i.e. why it works better than many other + * constants, prime or not, has never been adequately explained by + * anyone. So I try an explanation: if one experimentally tests all + * multipliers between 1 and 256 (as I did while writing a low-level + * data structure library some time ago) one detects that even + * numbers are not useable at all. The remaining 128 odd numbers + * (except for the number 1) work more or less all equally well. + * They all distribute in an acceptable way and this way fill a hash + * table with an average percent of approx. 86%. + * + * If one compares the chi^2 values of the variants (see + * Bob Jenkins ``Hashing Frequently Asked Questions'' at + * http://burtleburtle.net/bob/hash/hashfaq.html for a description + * of chi^2), the number 33 not even has the best value. But the + * number 33 and a few other equally good numbers like 17, 31, 63, + * 127 and 129 have nevertheless a great advantage to the remaining + * numbers in the large set of possible multipliers: their multiply + * operation can be replaced by a faster operation based on just one + * shift plus either a single addition or subtraction operation. And + * because a hash function has to both distribute good _and_ has to + * be very fast to compute, those few numbers should be preferred. + * + * -- Ralf S. Engelschall + */ + hash = 0; + if (klen == CACHE_HASH_KEY_STRING) { + for (p = key; *p; p++) { + hash = hash * 33 + *p; + } + klen = p - (const unsigned char *)key; + } + else { + for (p = key, i = klen; i; i--, p++) { + hash = hash * 33 + *p; + } + } + + /* scan linked list */ + for (hep = &ht->array[hash & ht->max], he = *hep; + he; + hep = &he->next, he = *hep) { + if (he->hash == hash && + he->klen == klen && + memcmp(he->key, key, klen) == 0) + break; + } + if (he || !val) + return hep; + /* add a new entry for non-NULL values */ + he = malloc(sizeof(*he)); + if (!he) { + return NULL; + } + he->next = NULL; + he->hash = hash; + he->key = key; + he->klen = klen; + he->val = val; + *hep = he; + ht->count++; + return hep; +} + +CACHE_DECLARE(void *) cache_hash_get(cache_hash_t *ht, + const void *key, + apr_ssize_t klen) +{ + cache_hash_entry_t *he; + he = *find_entry(ht, key, klen, NULL); + if (he) + return (void *)he->val; + else + return NULL; +} + +CACHE_DECLARE(void*) cache_hash_set(cache_hash_t *ht, + const void *key, + apr_ssize_t klen, + const void *val) +{ + cache_hash_entry_t **hep, *tmp; + const void *tval; + hep = find_entry(ht, key, klen, val); + if (*hep) { + if (!val) { + /* delete entry */ + tval = (*hep)->val; + tmp = *hep; + *hep = (*hep)->next; + free(tmp); + --ht->count; + } + else { + /* replace entry */ + tval = (*hep)->val; + (*hep)->val = val; + } + /* Return the object just removed from the cache + * to let the caller clean it up + */ + return tval; + } + /* else key not present and val==NULL */ + return NULL; +} + +CACHE_DECLARE(int) cache_hash_count(cache_hash_t *ht) +{ + return ht->count; +} diff --git a/modules/experimental/cache_hash.h b/modules/experimental/cache_hash.h new file mode 100644 index 0000000000..b73181b6d6 --- /dev/null +++ b/modules/experimental/cache_hash.h @@ -0,0 +1,194 @@ +/* ==================================================================== + * The Apache Software License, Version 1.1 + * + * Copyright (c) 2000-2002 The Apache Software Foundation. All rights + * reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * + * 3. The end-user documentation included with the redistribution, + * if any, must include the following acknowledgment: + * "This product includes software developed by the + * Apache Software Foundation (http://www.apache.org/)." + * Alternately, this acknowledgment may appear in the software itself, + * if and wherever such third-party acknowledgments normally appear. + * + * 4. The names "Apache" and "Apache Software Foundation" must + * not be used to endorse or promote products derived from this + * software without prior written permission. For written + * permission, please contact apache@apache.org. + * + * 5. Products derived from this software may not be called "Apache", + * nor may "Apache" appear in their name, without prior written + * permission of the Apache Software Foundation. + * + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESSED OR IMPLIED + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + * DISCLAIMED. IN NO EVENT SHALL THE APACHE SOFTWARE FOUNDATION OR + * ITS CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, + * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT + * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * . + * + * Portions of this software are based upon public domain software + * originally written at the National Center for Supercomputing Applications, + * University of Illinois, Urbana-Champaign. + */ + +#ifndef CACHE_HASH_H +#define CACHE_HASH_H + +#ifdef __cplusplus +extern "C" { +#endif + +#include "mod_cache.h" + +/** + * @file cache_hash.h + * @brief Cache Hash Tables + */ + +/** + * @defgroup Cache_Hash Hash Tables + * @ingroup CACHE + * @{ + */ + +/** + * When passing a key to cache_hash_set or cache_hash_get, this value can be + * passed to indicate a string-valued key, and have cache_hash compute the + * length automatically. + * + * @remark cache_hash will use strlen(key) for the length. The null-terminator + * is not included in the hash value (why throw a constant in?). + * Since the hash table merely references the provided key (rather + * than copying it), cache_hash_this() will return the null-term'd key. + */ +#define CACHE_HASH_KEY_STRING (-1) + +/** + * Abstract type for hash tables. + */ +typedef struct cache_hash_t cache_hash_t; + +/** + * Abstract type for scanning hash tables. + */ +typedef struct cache_hash_index_t cache_hash_index_t; + +/** + * Create a hash table. + * @param size + * @return The hash table just created + */ +CACHE_DECLARE(cache_hash_t *) cache_hash_make(apr_size_t size); + + +/** + * Associate a value with a key in a hash table. + * @param ht The hash table + * @param key Pointer to the key + * @param klen Length of the key. Can be CACHE_HASH_KEY_STRING to use the string length. + * @param val Value to associate with the key + * @remark If the value is NULL the hash entry is deleted. + * @return The value of the deleted cache entry (so the caller can clean it up). + */ +CACHE_DECLARE(void *) cache_hash_set(cache_hash_t *ht, const void *key, + apr_ssize_t klen, const void *val); + +/** + * Look up the value associated with a key in a hash table. + * @param ht The hash table + * @param key Pointer to the key + * @param klen Length of the key. Can be CACHE_HASH_KEY_STRING to use the string length. + * @return Returns NULL if the key is not present. + */ +CACHE_DECLARE(void *) cache_hash_get(cache_hash_t *ht, const void *key, + apr_ssize_t klen); + +/** + * Start iterating over the entries in a hash table. + * @param p The pool to allocate the cache_hash_index_t iterator. If this + * pool is NULL, then an internal, non-thread-safe iterator is used. + * @param ht The hash table + * @example + */ +/** + * 
+ * 
+ *     int sum_values(apr_pool_t *p, cache_hash_t *ht)
+ *     {
+ *         cache_hash_index_t *hi;
+ * 	   void *val;
+ * 	   int sum = 0;
+ * 	   for (hi = cache_hash_first(p, ht); hi; hi = cache_hash_next(hi)) {
+ * 	       cache_hash_this(hi, NULL, NULL, &val);
+ * 	       sum += *(int *)val;
+ * 	   }
+ * 	   return sum;
+ *     }
+ * 
+ * There is no restriction on adding or deleting hash entries during an
+ * iteration (although the results may be unpredictable unless all you do
+ * is delete the current entry) and multiple iterations can be in
+ * progress at the same time.
+ *
+ */ +CACHE_DECLARE(cache_hash_index_t *) cache_hash_first(cache_hash_t *ht); + +/** + * Continue iterating over the entries in a hash table. + * @param hi The iteration state + * @return a pointer to the updated iteration state. NULL if there are no more + * entries. + */ +CACHE_DECLARE(cache_hash_index_t *) cache_hash_next(cache_hash_index_t *hi); + +/** + * Get the current entry's details from the iteration state. + * @param hi The iteration state + * @param key Return pointer for the pointer to the key. + * @param klen Return pointer for the key length. + * @param val Return pointer for the associated value. + * @remark The return pointers should point to a variable that will be set to the + * corresponding data, or they may be NULL if the data isn't interesting. + */ +CACHE_DECLARE(void) cache_hash_this(cache_hash_index_t *hi, const void **key, + apr_ssize_t *klen, void **val); + +/** + * Get the number of key/value pairs in the hash table. + * @param ht The hash table + * @return The number of key/value pairs in the hash table. + */ +CACHE_DECLARE(int) cache_hash_count(cache_hash_t *ht); + + +/** @} */ +#ifdef __cplusplus +} +#endif + +#endif /* !CACHE_HASH_H */ diff --git a/modules/experimental/mod_cache.dsp b/modules/experimental/mod_cache.dsp index b9a3263685..38cd907885 100644 --- a/modules/experimental/mod_cache.dsp +++ b/modules/experimental/mod_cache.dsp @@ -52,8 +52,8 @@ BSC32=bscmake.exe # ADD BASE BSC32 /nologo # ADD BSC32 /nologo LINK32=link.exe -# ADD BASE LINK32 kernel32.lib /nologo /subsystem:windows /dll /incremental:no /map /machine:I386 -# ADD LINK32 kernel32.lib /nologo /subsystem:windows /dll /incremental:no /map /machine:I386 /out:"Release/mod_cache.so" /base:@..\..\os\win32\BaseAddr.ref,mod_cache +# ADD BASE LINK32 kernel32.lib /nologo /subsystem:windows /dll /map /machine:I386 +# ADD LINK32 kernel32.lib /nologo /subsystem:windows /dll /map /machine:I386 /out:"Release/mod_cache.so" /base:@..\..\os\win32\BaseAddr.ref,mod_cache !ELSEIF "$(CFG)" == "mod_cache - Win32 Debug" @@ -92,6 +92,10 @@ LINK32=link.exe # PROP Default_Filter "cpp;c;cxx;rc;def;r;odl;idl;hpj;bat" # Begin Source File +SOURCE=.\cache_hash.c +# End Source File +# Begin Source File + SOURCE=.\cache_storage.c # End Source File # Begin Source File @@ -108,6 +112,10 @@ SOURCE=.\mod_cache.c # PROP Default_Filter "h;hpp;hxx;hm;inl" # Begin Source File +SOURCE=.\cache_hash.h +# End Source File +# Begin Source File + SOURCE=.\mod_cache.h # End Source File # End Group diff --git a/modules/experimental/mod_mem_cache.c b/modules/experimental/mod_mem_cache.c index fdc0cb5fe5..4429d95495 100644 --- a/modules/experimental/mod_mem_cache.c +++ b/modules/experimental/mod_mem_cache.c @@ -58,6 +58,7 @@ #define CORE_PRIVATE #include "mod_cache.h" +#include "cache_hash.h" #include "ap_mpm.h" #include "apr_thread_mutex.h" #if APR_HAVE_UNISTD_H @@ -70,13 +71,6 @@ module AP_MODULE_DECLARE_DATA mem_cache_module; -/* - * XXX - * This cache uses apr_hash functions which leak storage when something is removed - * from the cache. This can be fixed in the apr_hash functions by making them use - * malloc/free rather than pools to manage their storage requirements. - */ - typedef enum { CACHE_TYPE_FILE = 1, CACHE_TYPE_HEAP, @@ -103,7 +97,7 @@ typedef struct mem_cache_object { typedef struct { apr_thread_mutex_t *lock; - apr_hash_t *cacheht; + cache_hash_t *cacheht; apr_size_t cache_size; apr_size_t object_cnt; @@ -206,7 +200,7 @@ static apr_status_t decrement_refcount(void *arg) * removed the object from the cache (and set obj->cleanup) */ if (!obj->cleanup) { - apr_hash_set(sconf->cacheht, obj->key, strlen(obj->key), NULL); + cache_hash_set(sconf->cacheht, obj->key, strlen(obj->key), NULL); sconf->object_cnt--; sconf->cache_size -= mobj->m_len; obj->cleanup = 1; @@ -243,7 +237,7 @@ static apr_status_t decrement_refcount(void *arg) static apr_status_t cleanup_cache_mem(void *sconfv) { cache_object_t *obj; - apr_hash_index_t *hi; + cache_hash_index_t *hi; mem_cache_conf *co = (mem_cache_conf*) sconfv; if (!co) { @@ -254,12 +248,12 @@ static apr_status_t cleanup_cache_mem(void *sconfv) apr_thread_mutex_lock(sconf->lock); } /* Iterate over the cache and clean up each entry */ - while ((hi = apr_hash_first(NULL, co->cacheht)) != NULL) { + while ((hi = cache_hash_first(NULL, co->cacheht)) != NULL) { /* Fetch the object from the cache */ - apr_hash_this(hi, NULL, NULL, (void **)&obj); + cache_hash_this(hi, NULL, NULL, (void **)&obj); if (obj) { /* Remove the object from the cache */ - apr_hash_set(sconf->cacheht, obj->key, strlen(obj->key), NULL); + cache_hash_set(sconf->cacheht, obj->key, strlen(obj->key), NULL); /* Free the object if the recount == 0 */ #ifdef USE_ATOMICS apr_atomic_inc(&obj->refcount); @@ -288,12 +282,12 @@ static void *create_cache_config(apr_pool_t *p, server_rec *s) sconf = apr_pcalloc(p, sizeof(mem_cache_conf)); - ap_mpm_query(AP_MPMQ_IS_THREADED, &threaded_mpm); if (threaded_mpm) { apr_thread_mutex_create(&sconf->lock, APR_THREAD_MUTEX_DEFAULT, p); } - sconf->cacheht = apr_hash_make(p); + /* Todo: determine hash table size from max_cache_object_cnt */ + sconf->cacheht = cache_hash_make(512); sconf->min_cache_object_size = DEFAULT_MIN_CACHE_OBJECT_SIZE; sconf->max_cache_object_size = DEFAULT_MAX_CACHE_OBJECT_SIZE; @@ -399,11 +393,11 @@ static int create_entity(cache_handle_t *h, request_rec *r, if (sconf->lock) { apr_thread_mutex_lock(sconf->lock); } - tmp_obj = (cache_object_t *) apr_hash_get(sconf->cacheht, + tmp_obj = (cache_object_t *) cache_hash_get(sconf->cacheht, key, - APR_HASH_KEY_STRING); + CACHE_HASH_KEY_STRING); if (!tmp_obj) { - apr_hash_set(sconf->cacheht, obj->key, strlen(obj->key), obj); + cache_hash_set(sconf->cacheht, obj->key, strlen(obj->key), obj); sconf->object_cnt++; sconf->cache_size += len; } @@ -445,8 +439,8 @@ static int open_entity(cache_handle_t *h, request_rec *r, const char *type, cons if (sconf->lock) { apr_thread_mutex_lock(sconf->lock); } - obj = (cache_object_t *) apr_hash_get(sconf->cacheht, key, - APR_HASH_KEY_STRING); + obj = (cache_object_t *) cache_hash_get(sconf->cacheht, key, + CACHE_HASH_KEY_STRING); if (obj) { if (obj->complete) { request_rec *rmain=r, *rtmp; @@ -508,10 +502,11 @@ static int remove_entity(cache_handle_t *h) */ if (!obj->cleanup) { mem_cache_object_t *mobj = (mem_cache_object_t *) obj->vobj; - apr_hash_set(sconf->cacheht, obj->key, strlen(obj->key), NULL); + cache_hash_set(sconf->cacheht, obj->key, strlen(obj->key), NULL); sconf->object_cnt--; sconf->cache_size -= mobj->m_len; obj->cleanup = 1; + ap_log_error(APLOG_MARK, APLOG_INFO|APLOG_NOERRNO, 0, NULL, "gcing a cache entry"); } if (sconf->lock) { @@ -585,13 +580,6 @@ static int remove_url(const char *type, const char *key) if (strcasecmp(type, "mem") && strcasecmp(type, "fd")) { return DECLINED; } - - /* WIBNIF - * apr_hash_set(..,..,..,NULL) returned pointer to the object just removed. - * That way, we could free the object w/o doing another call to an - * apr_hash function. - */ - /* Order of the operations is important to avoid race conditions. * First, remove the object from the cache. Remember, all additions * deletions from the cache are protected by sconf->lock. @@ -604,11 +592,11 @@ static int remove_url(const char *type, const char *key) if (sconf->lock) { apr_thread_mutex_lock(sconf->lock); } - obj = (cache_object_t *) apr_hash_get(sconf->cacheht, key, - APR_HASH_KEY_STRING); + /* This call will return a pointer to the cache_object just removed */ + obj = cache_hash_set(sconf->cacheht, key, CACHE_HASH_KEY_STRING, NULL); if (obj) { + /* obj has been removed from the cache */ mem_cache_object_t *mobj = (mem_cache_object_t *) obj->vobj; - apr_hash_set(sconf->cacheht, key, APR_HASH_KEY_STRING, NULL); sconf->object_cnt--; sconf->cache_size -= mobj->m_len;