From 0db831976e4b2d2553c8cd9aaae492970d830f57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20B=C3=BChler?= Date: Tue, 17 Mar 2015 09:09:43 +0100 Subject: [PATCH] fix refreshing of obsolete dns cache entries - cache entries must be also refreshed when they are in use - have the cache count as inuse reference too, freeing timestamp == 0 special value - use timestamp == 0 for CURLOPT_RESOLVE entries which don't get refreshed - remove CURLOPT_RESOLVE special inuse reference (timestamp == 0 will prevent refresh) - fix Curl_hostcache_clean - CURLOPT_RESOLVE entries don't have a special reference anymore, and it would also release non CURLOPT_RESOLVE references - fix locking in Curl_hostcache_clean - fix unit1305.c: hash now keeps a reference, need to set inuse = 1 --- lib/hostip.c | 70 ++++++++++++++++++++----------------------- lib/hostip.h | 7 ++--- tests/unit/unit1305.c | 1 + 3 files changed, 36 insertions(+), 42 deletions(-) diff --git a/lib/hostip.c b/lib/hostip.c index dc06e89f7..05f3ed0b5 100644 --- a/lib/hostip.c +++ b/lib/hostip.c @@ -137,10 +137,6 @@ struct curl_hash *Curl_global_host_cache_init(void) void Curl_global_host_cache_dtor(void) { if(host_cache_initialized) { - /* first make sure that any custom "CURLOPT_RESOLVE" names are - cleared off */ - Curl_hostcache_clean(NULL, &hostname_cache); - /* then free the remaining hash completely */ Curl_hash_clean(&hostname_cache); host_cache_initialized = 0; } @@ -234,7 +230,8 @@ hostcache_timestamp_remove(void *datap, void *hc) (struct hostcache_prune_data *) datap; struct Curl_dns_entry *c = (struct Curl_dns_entry *) hc; - return !c->inuse && (data->now - c->timestamp >= data->cache_timeout); + return (0 != c->timestamp) + && (data->now - c->timestamp >= data->cache_timeout); } /* @@ -288,10 +285,9 @@ remove_entry_if_stale(struct SessionHandle *data, struct Curl_dns_entry *dns) { struct hostcache_prune_data user; - if(!dns || (data->set.dns_cache_timeout == -1) || !data->dns.hostcache || - dns->inuse) + if(!dns || (data->set.dns_cache_timeout == -1) || !data->dns.hostcache) /* cache forever means never prune, and NULL hostcache means we can't do - it, if it still is in use then we leave it */ + it */ return 0; time(&user.now); @@ -395,11 +391,11 @@ Curl_cache_addr(struct SessionHandle *data, return NULL; } - dns->inuse = 0; /* init to not used */ + dns->inuse = 1; /* the cache has the first reference */ dns->addr = addr; /* this is the address(es) */ time(&dns->timestamp); if(dns->timestamp == 0) - dns->timestamp = 1; /* zero indicates that entry isn't in hash table */ + dns->timestamp = 1; /* zero indicates CURLOPT_RESOLVE entry */ /* Store the resolved data in our DNS cache. */ dns2 = Curl_hash_add(data->dns.hostcache, entry_id, entry_len+1, @@ -717,35 +713,27 @@ clean_up: */ void Curl_resolv_unlock(struct SessionHandle *data, struct Curl_dns_entry *dns) { - DEBUGASSERT(dns && (dns->inuse>0)); - if(data && data->share) Curl_share_lock(data, CURL_LOCK_DATA_DNS, CURL_LOCK_ACCESS_SINGLE); - dns->inuse--; - /* only free if nobody is using AND it is not in hostcache (timestamp == - 0) */ - if(dns->inuse == 0 && dns->timestamp == 0) { - Curl_freeaddrinfo(dns->addr); - free(dns); - } + freednsentry(dns); if(data && data->share) Curl_share_unlock(data, CURL_LOCK_DATA_DNS); } /* - * File-internal: free a cache dns entry. + * File-internal: release cache dns entry reference, free if inuse drops to 0 */ static void freednsentry(void *freethis) { - struct Curl_dns_entry *p = (struct Curl_dns_entry *) freethis; + struct Curl_dns_entry *dns = (struct Curl_dns_entry *) freethis; + DEBUGASSERT(dns && (dns->inuse>0)); - /* mark the entry as not in hostcache */ - p->timestamp = 0; - if(p->inuse == 0) { - Curl_freeaddrinfo(p->addr); - free(p); + dns->inuse--; + if(dns->inuse == 0) { + Curl_freeaddrinfo(dns->addr); + free(dns); } } @@ -757,13 +745,10 @@ struct curl_hash *Curl_mk_dnscache(void) return Curl_hash_alloc(7, Curl_hash_str, Curl_str_key_compare, freednsentry); } -static int hostcache_inuse(void *data, void *hc) +static int free_all_entries(void *data, void *hc) { - struct Curl_dns_entry *c = (struct Curl_dns_entry *) hc; - - if(c->inuse == 1) - Curl_resolv_unlock(data, c); - + (void)data; + (void)hc; return 1; /* free all entries */ } @@ -777,11 +762,13 @@ static int hostcache_inuse(void *data, void *hc) void Curl_hostcache_clean(struct SessionHandle *data, struct curl_hash *hash) { - /* Entries added to the hostcache with the CURLOPT_RESOLVE function are - * still present in the cache with the inuse counter set to 1. Detect them - * and cleanup! - */ - Curl_hash_clean_with_criterium(hash, data, hostcache_inuse); + if(data && data->share) + Curl_share_lock(data, CURL_LOCK_DATA_DNS, CURL_LOCK_ACCESS_SINGLE); + + Curl_hash_clean_with_criterium(hash, NULL, free_all_entries); + + if(data && data->share) + Curl_share_unlock(data, CURL_LOCK_DATA_DNS); } @@ -830,9 +817,16 @@ CURLcode Curl_loadhostpairs(struct SessionHandle *data) /* free the allocated entry_id again */ free(entry_id); - if(!dns) + if(!dns) { /* if not in the cache already, put this host in the cache */ dns = Curl_cache_addr(data, addr, hostname, port); + if(dns) { + dns->timestamp = 0; /* mark as added by CURLOPT_RESOLVE */ + /* release the returned reference; the cache itself will keep the + * entry alive: */ + dns->inuse--; + } + } else /* this is a duplicate, free it again */ Curl_freeaddrinfo(addr); diff --git a/lib/hostip.h b/lib/hostip.h index e1e880eab..0e2e2cf7f 100644 --- a/lib/hostip.h +++ b/lib/hostip.h @@ -65,11 +65,10 @@ void Curl_global_host_cache_dtor(void); struct Curl_dns_entry { Curl_addrinfo *addr; - /* timestamp == 0 -- entry not in hostcache - timestamp != 0 -- entry is in hostcache */ + /* timestamp == 0 -- CURLOPT_RESOLVE entry, doesn't timeout */ time_t timestamp; - long inuse; /* use-counter, make very sure you decrease this - when you're done using the address you received */ + /* use-counter, use Curl_resolv_unlock to release reference */ + long inuse; }; /* diff --git a/tests/unit/unit1305.c b/tests/unit/unit1305.c index 93815e5f8..4f9c609b0 100644 --- a/tests/unit/unit1305.c +++ b/tests/unit/unit1305.c @@ -128,6 +128,7 @@ UNITTEST_START abort_unless(rc == CURLE_OK, "data node creation failed"); key_len = strlen(data_key); + data_node->inuse = 1; /* hash will hold the reference */ nodep = Curl_hash_add(hp, data_key, key_len+1, data_node); abort_unless(nodep, "insertion into hash failed"); /* Freeing will now be done by Curl_hash_destroy */