Commit fbe32758 authored by Bradley Nicholes's avatar Bradley Nicholes
Browse files

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

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.

Reviewed by: bnicholes, minfrin, jjclar


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/APACHE_2_0_BRANCH@104482 13f79535-47bb-0310-9956-ffa450edef68
parent bafd78c4

modules/experimental/util_ldap.c

+32 −1
Changes for modules/experimental/util_ldap.c: 32 added lines, 1 removed line.
Original line number Diff line number Diff line
@@ -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;

            

            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;



            /* 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;



        /* 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);

modules/experimental/util_ldap_cache.h

+1 −1
Changes for modules/experimental/util_ldap_cache.h: 1 added line, 1 removed line.
Original line number Diff line number Diff line
@@ -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);

modules/experimental/util_ldap_cache_mgr.c

+16 −13
Changes for modules/experimental/util_ldap_cache_mgr.c: 16 added lines, 13 removed lines.
Original line number Diff line number Diff line
@@ -212,7 +212,7 @@ 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;
@@ -240,18 +240,20 @@ util_url_node_t *util_ald_create_caches(util_ldap_state_t *st, const char *url) 
    /* 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)