Get rid of the race conditions by first checking to make sure that a duplicate node...

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 761503d79fcd8554983ca1fc124a9ff07739553e..b1dcf8462d9379af3fa2a4cc0332ab7d24cf533e 100644 (file)
--- 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 017ed0897279ac869938475eb16f51f9fc741836..df3a91b599aa070cfab193178b572e26dc0f0fe2 100644 (file)
--- 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 d7bf34c409cac9b6a666680185880b8b3877e2e8..9e684ecb4a7f76225bf15eaa7b392f394dceac73 100644 (file)
--- 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)