- Constantine Sapuntzakis posted bug #2891595

  (http://curl.haxx.se/bug/view.cgi?id=2891595) which identified how an entry
  in the DNS cache would linger too long if the request that added it was in
  use that long. He also provided the patch that now makes libcurl capable of
  still doing a request while the DNS hash entry may get timed out.

diff --git a/CHANGES b/CHANGES
index 3bd88093101b6e07ec73959e2186f499d453c16b..20ed3f2345de38927b39083693dd988e73dcde55 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -7,6 +7,12 @@
                                   Changelog
 
 Daniel Stenberg (11 Nov 2009)
+- Constantine Sapuntzakis posted bug #2891595
+  (http://curl.haxx.se/bug/view.cgi?id=2891595) which identified how an entry
+  in the DNS cache would linger too long if the request that added it was in
+  use that long. He also provided the patch that now makes libcurl capable of
+  still doing a request while the DNS hash entry may get timed out.
+  
 - Christian Schmitz noticed that the progress meter/callback was not properly
   used during the FTP connection phase (after the actual TCP connect), while
   it of course should be. I also made the speed check get called correctly so

diff --git a/RELEASE-NOTES b/RELEASE-NOTES
index 08d7215d81d559caca59c43d39535ff744d05720..89549d72e6bff5902aba721c0be5d955864c4078 100644 (file)
--- a/RELEASE-NOTES
+++ b/RELEASE-NOTES
@@ -16,6 +16,7 @@ This release includes the following bugfixes:
  o progress meter percentage and transfer time estimates fixes
  o portability enhancement for OS's without orthogonal directory tree structure
  o progress meter/callback during FTP connection
+ o DNS cache timeout while transfer in progress
 
 This release includes the following known bugs:
 
@@ -24,6 +25,6 @@ This release includes the following known bugs:
 This release would not have looked like this without help, code, reports and
 advice from friends like these:
 
- Yang Tse, Kamil Dudka, Christian Schmitz
+ Yang Tse, Kamil Dudka, Christian Schmitz, Constantine Sapuntzakis
 
         Thanks! (and sorry if I forgot to mention someone)

diff --git a/lib/hash.c b/lib/hash.c
index 6ca7431ce7541e9a02919a6b07bb2e05617233bf..bcd144a1126031bfa26fdf18a6e344b20ceaa366 100644 (file)
--- a/lib/hash.c
+++ b/lib/hash.c
@@ -140,8 +140,8 @@ mk_hash_element(const void *key, size_t key_len, const void *p)
 
 #define FETCH_LIST(x,y,z) x->table[x->hash_func(y, z, x->slots)]
 
-/* Return the data in the hash. If there already was a match in the hash,
-   that data is returned. */
+/* Insert the data in the hash. If there already was a match in the hash,
+   that data is replaced. */
 void *
 Curl_hash_add(struct curl_hash *h, void *key, size_t key_len, void *p)
 {
@@ -152,8 +152,9 @@ Curl_hash_add(struct curl_hash *h, void *key, size_t key_len, void *p)
   for (le = l->head; le; le = le->next) {
     he = (struct curl_hash_element *) le->ptr;
     if(h->comp_func(he->key, he->key_len, key, key_len)) {
-      h->dtor(p);     /* remove the NEW entry */
-      return he->ptr; /* return the EXISTING entry */
+      Curl_llist_remove(l, le, (void *)h);
+      --h->size;
+      break;
     }
   }
 

diff --git a/lib/hostip.c b/lib/hostip.c
index 2ccb201c8336583aa2da68fa7fcd407b0b9e026c..e40f1f11b2617347c5206eccd290354bab4d286e 100644 (file)
--- a/lib/hostip.c
+++ b/lib/hostip.c
@@ -123,6 +123,10 @@
 static struct curl_hash hostname_cache;
 static int host_cache_initialized;
 
+#ifdef CURLDEBUG
+static int ndns = 0;
+#endif
+
 static void freednsentry(void *freethis);
 
 /*
@@ -232,14 +236,7 @@ hostcache_timestamp_remove(void *datap, void *hc)
     (struct hostcache_prune_data *) datap;
   struct Curl_dns_entry *c = (struct Curl_dns_entry *) hc;
 
-  if((data->now - c->timestamp < data->cache_timeout) ||
-      c->inuse) {
-    /* please don't remove */
-    return 0;
-  }
-
-  /* fine, remove */
-  return 1;
+  return (data->now - c->timestamp >= data->cache_timeout);
 }
 
 /*
@@ -339,7 +336,6 @@ Curl_cache_addr(struct SessionHandle *data,
   size_t entry_len;
   struct Curl_dns_entry *dns;
   struct Curl_dns_entry *dns2;
-  time_t now;
 
   /* Create an entry id, based upon the hostname and port */
   entry_id = create_hostcache_id(hostname, port);
@@ -357,23 +353,22 @@ Curl_cache_addr(struct SessionHandle *data,
 
   dns->inuse = 0;   /* init to not used */
   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 */
 
-  /* Store the resolved data in our DNS cache. This function may return a
-     pointer to an existing struct already present in the hash, and it may
-     return the same argument we pass in. Make no assumptions. */
+  /* Store the resolved data in our DNS cache. */
   dns2 = Curl_hash_add(data->dns.hostcache, entry_id, entry_len+1,
                        (void *)dns);
   if(!dns2) {
-    /* Major badness, run away. */
     free(dns);
     free(entry_id);
     return NULL;
   }
-  time(&now);
-  dns = dns2;
 
-  dns->timestamp = now; /* used now */
+  dns = dns2;
   dns->inuse++;         /* mark entry as in-use */
+  DEBUGF(ndns++);
 
   /* free the allocated entry_id again */
   free(entry_id);
@@ -436,6 +431,7 @@ int Curl_resolv(struct connectdata *conn,
 
   if(dns) {
     dns->inuse++; /* we use it! */
+    DEBUGF(ndns++);
     rc = CURLRESOLV_RESOLVED;
   }
 
@@ -688,6 +684,13 @@ void Curl_resolv_unlock(struct SessionHandle *data, struct Curl_dns_entry *dns)
     Curl_share_lock(data, CURL_LOCK_DATA_DNS, CURL_LOCK_ACCESS_SINGLE);
 
   dns->inuse--;
+  DEBUGF(ndns--);
+  /* 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);
+  }
 
   if(data->share)
     Curl_share_unlock(data, CURL_LOCK_DATA_DNS);
@@ -700,12 +703,21 @@ static void freednsentry(void *freethis)
 {
   struct Curl_dns_entry *p = (struct Curl_dns_entry *) freethis;
 
-  if(p) {
+  /* mark the entry as not in hostcache */
+  p->timestamp = 0;
+  if (p->inuse == 0) {
     Curl_freeaddrinfo(p->addr);
     free(p);
   }
 }
 
+#ifdef CURLDEBUG
+int curl_get_ndns(void)
+{
+  return ndns;
+}
+#endif
+
 /*
  * Curl_mk_dnscache() creates a new DNS cache and returns the handle for it.
  */

diff --git a/lib/hostip.h b/lib/hostip.h
index 8b6bb76fbce09a08e3fb27f2272e1ac6d5c8f752..b6aef623a5b25f44e2e7ec65b7d7fe893053972b 100644 (file)
--- a/lib/hostip.h
+++ b/lib/hostip.h
@@ -121,6 +121,8 @@ 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 */
   time_t timestamp;
   long inuse;      /* use-counter, make very sure you decrease this
                       when you're done using the address you received */