From: Bradley Nicholes Date: Thu, 17 Jun 2004 17:19:01 +0000 (+0000) Subject: Get rid of the race conditions by first checking to make sure that a duplicate node... X-Git-Tag: pre_ajp_proxy~142 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=182d7122726eb9ac1ef4a700c98e4c65ffb517a9;p=apache Get rid of the race conditions by first checking to make sure that a duplicate node does not already exist before inserting nodes into the different caches. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@103978 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/modules/experimental/util_ldap.c b/modules/experimental/util_ldap.c index 761503d79f..b1dcf8462d 100644 --- a/modules/experimental/util_ldap.c +++ b/modules/experimental/util_ldap.c @@ -581,10 +581,16 @@ start_over: else { if (curl) { /* compare successful - add to the compare cache */ - LDAP_CACHE_RDLOCK(); + LDAP_CACHE_WRLOCK(); newnode.reqdn = (char *)reqdn; newnode.dn = (char *)dn; - util_ald_cache_insert(curl->dn_compare_cache, &newnode); + + node = util_ald_cache_fetch(curl->dn_compare_cache, &newnode); + if ((node == NULL) || + (strcmp(reqdn, node->reqdn) != 0) || (strcmp(dn, node->dn) != 0)) { + + util_ald_cache_insert(curl->dn_compare_cache, &newnode); + } LDAP_CACHE_UNLOCK(); } ldc->reason = "DN Comparison TRUE (checked on server)"; @@ -702,7 +708,21 @@ start_over: LDAP_CACHE_WRLOCK(); the_compare_node.lastcompare = curtime; the_compare_node.result = result; - util_ald_cache_insert(curl->compare_cache, &the_compare_node); + + /* If the node doesn't exist then insert it, otherwise just update it with + the last results */ + compare_nodep = util_ald_cache_fetch(curl->compare_cache, &the_compare_node); + if ((compare_nodep == NULL) || + (strcmp(the_compare_node.dn, compare_nodep->dn) != 0) || + (strcmp(the_compare_node.attrib, compare_nodep->attrib) != 0) || + (strcmp(the_compare_node.value, compare_nodep->value) != 0)) { + + util_ald_cache_insert(curl->compare_cache, &the_compare_node); + } + else { + compare_nodep->lastcompare = curtime; + compare_nodep->result = result; + } LDAP_CACHE_UNLOCK(); } if (LDAP_COMPARE_TRUE == result) { @@ -920,7 +940,18 @@ start_over: the_search_node.bindpw = bindpw; the_search_node.lastbind = apr_time_now(); the_search_node.vals = vals; - util_ald_cache_insert(curl->search_cache, &the_search_node); + + /* Search again to make sure that another thread didn't ready insert this node + into the cache before we got here. If it does exist then update the lastbind */ + search_nodep = util_ald_cache_fetch(curl->search_cache, &the_search_node); + if ((search_nodep == NULL) || + (strcmp(*binddn, search_nodep->dn) != 0) || (strcmp(bindpw, search_nodep->bindpw) != 0)) { + + util_ald_cache_insert(curl->search_cache, &the_search_node); + } + else { + search_nodep->lastbind = the_search_node.lastbind; + } LDAP_CACHE_UNLOCK(); } ldap_msgfree(res); diff --git a/modules/experimental/util_ldap_cache.h b/modules/experimental/util_ldap_cache.h index 017ed08972..df3a91b599 100644 --- a/modules/experimental/util_ldap_cache.h +++ b/modules/experimental/util_ldap_cache.h @@ -199,7 +199,7 @@ util_ald_cache_t *util_ald_create_cache(util_ldap_state_t *st, void util_ald_destroy_cache(util_ald_cache_t *cache); void *util_ald_cache_fetch(util_ald_cache_t *cache, void *payload); -void util_ald_cache_insert(util_ald_cache_t *cache, void *payload); +void *util_ald_cache_insert(util_ald_cache_t *cache, void *payload); void util_ald_cache_remove(util_ald_cache_t *cache, void *payload); char *util_ald_cache_display_stats(request_rec *r, util_ald_cache_t *cache, char *name, char *id); diff --git a/modules/experimental/util_ldap_cache_mgr.c b/modules/experimental/util_ldap_cache_mgr.c index d7bf34c409..9e684ecb4a 100644 --- a/modules/experimental/util_ldap_cache_mgr.c +++ b/modules/experimental/util_ldap_cache_mgr.c @@ -153,8 +153,8 @@ unsigned long util_ald_hash_string(int nstr, ...) for (p = str; *p; ++p) { h = ( h << 4 ) + *p; if ( ( g = h & 0xf0000000 ) ) { - h = h ^ (g >> 24); - h = h ^ g; + h = h ^ (g >> 24); + h = h ^ g; } } } @@ -187,15 +187,15 @@ void util_ald_cache_purge(util_ald_cache_t *cache) p = cache->nodes[i]; while (p != NULL) { if (p->add_time < cache->marktime) { - q = p->next; - (*cache->free)(cache, p->payload); - util_ald_free(cache, p); - cache->numentries--; - cache->npurged++; - p = q; + q = p->next; + (*cache->free)(cache, p->payload); + util_ald_free(cache, p); + cache->numentries--; + cache->npurged++; + p = q; } else { - p = p->next; + p = p->next; } } } @@ -212,46 +212,48 @@ void util_ald_cache_purge(util_ald_cache_t *cache) */ util_url_node_t *util_ald_create_caches(util_ldap_state_t *st, const char *url) { - util_url_node_t *curl = NULL; + util_url_node_t curl, *newcurl; util_ald_cache_t *search_cache; util_ald_cache_t *compare_cache; util_ald_cache_t *dn_compare_cache; /* create the three caches */ search_cache = util_ald_create_cache(st, - util_ldap_search_node_hash, - util_ldap_search_node_compare, - util_ldap_search_node_copy, - util_ldap_search_node_free, + util_ldap_search_node_hash, + util_ldap_search_node_compare, + util_ldap_search_node_copy, + util_ldap_search_node_free, util_ldap_search_node_display); compare_cache = util_ald_create_cache(st, - util_ldap_compare_node_hash, - util_ldap_compare_node_compare, - util_ldap_compare_node_copy, - util_ldap_compare_node_free, - util_ldap_compare_node_display); + util_ldap_compare_node_hash, + util_ldap_compare_node_compare, + util_ldap_compare_node_copy, + util_ldap_compare_node_free, + util_ldap_compare_node_display); dn_compare_cache = util_ald_create_cache(st, - util_ldap_dn_compare_node_hash, - util_ldap_dn_compare_node_compare, - util_ldap_dn_compare_node_copy, - util_ldap_dn_compare_node_free, - util_ldap_dn_compare_node_display); + util_ldap_dn_compare_node_hash, + util_ldap_dn_compare_node_compare, + util_ldap_dn_compare_node_copy, + util_ldap_dn_compare_node_free, + util_ldap_dn_compare_node_display); /* check that all the caches initialised successfully */ if (search_cache && compare_cache && dn_compare_cache) { -/*XXX This can be allocated on the stack since it will be copied anyway */ - curl = (util_url_node_t *)apr_pcalloc(st->pool, sizeof(util_url_node_t)); - curl->url = url; - curl->search_cache = search_cache; - curl->compare_cache = compare_cache; - curl->dn_compare_cache = dn_compare_cache; + /* The contents of this structure will be duplicated in shared + memory during the insert. So use stack memory rather than + pool memory to avoid a memory leak. */ + memset (&curl, 0, sizeof(util_url_node_t)); + curl.url = url; + curl.search_cache = search_cache; + curl.compare_cache = compare_cache; + curl.dn_compare_cache = dn_compare_cache; - util_ald_cache_insert(st->util_ldap_cache, curl); + newcurl = util_ald_cache_insert(st->util_ldap_cache, &curl); } - return curl; + return newcurl; } @@ -369,14 +371,14 @@ void *util_ald_cache_fetch(util_ald_cache_t *cache, void *payload) * Insert an item into the cache. * *** Does not catch duplicates!!! *** */ -void util_ald_cache_insert(util_ald_cache_t *cache, void *payload) +void *util_ald_cache_insert(util_ald_cache_t *cache, void *payload) { int hashval; util_cache_node_t *node; /* sanity check */ if (cache == NULL || payload == NULL) { - return; + return NULL; } /* check if we are full - if so, try purge */ @@ -384,13 +386,13 @@ void util_ald_cache_insert(util_ald_cache_t *cache, void *payload) util_ald_cache_purge(cache); if (cache->numentries >= cache->maxentries) { /* if the purge was not effective, we leave now to avoid an overflow */ - return; + return NULL; } } /* should be safe to add an entry */ if ((node = (util_cache_node_t *)util_ald_alloc(cache, sizeof(util_cache_node_t))) == NULL) { - return; + return NULL; } /* populate the entry */ @@ -408,6 +410,7 @@ void util_ald_cache_insert(util_ald_cache_t *cache, void *payload) cache->marktime=apr_time_now(); } + return node->payload; } void util_ald_cache_remove(util_ald_cache_t *cache, void *payload)