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

@@ -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 @@ int Curl_resolv_timeout(struct connectdata *conn,
  */
 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);

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

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