KNOWN_BUGS #17 fixed. A DNS cache entry may not remain locked between two

curl_easy_perform() invokes. It was previously unlocked at disconnect, which
could mean that it remained locked between multiple transfers. The DNS cache
may not live as long as the connection cache does, as they are separate.

To deal with the lack of DNS (host address) data availability in re-used
connections, libcurl now keeps a copy of the IP adress as a string, to be able
to show it even on subsequent requests on the same connection.

4 files changed, 54 insertions, 24 deletions

diff --git a/CHANGES b/CHANGES
index 5aa5a84eb..059f9aa53 100644
--- a/CHANGES
+++ b/CHANGES
@@ -7,6 +7,26 @@
                                   Changelog
 
 Daniel (28 January 2005)
+- KNOWN_BUGS #17 fixed. A DNS cache entry may not remain locked between two
+  curl_easy_perform() invokes. It was previously unlocked at disconnect, which
+  could mean that it remained locked between multiple transfers. The DNS cache
+  may not live as long as the connection cache does, as they are separate.
+
+  To deal with the lack of DNS (host address) data availability in re-used
+  connections, libcurl now keeps a copy of the IP adress as a string, to be
+  able to show it even on subsequent requests on the same connection.
+
+  The problem could be made to appear with this stunt:
+
+  1. create a multi handle
+  2. add an easy handle
+  3. fetch a URL that is persistent (leaves the connection alive)
+  4. remove the easy handle from the multi
+  5. kill the multi handle
+  6. create a multi handle
+  7. add the same easy handle to the new multi handle
+  8. fetch a URL from the same server as before (re-using the connection)
+
 - Stephen More pointed out that CURLOPT_FTPPORT and the -P option didn't work
   when built ipv6-enabled. I've now made a fix for it. Writing test cases for
   custom port hosts turned too tricky so unfortunately there's none.
diff --git a/docs/KNOWN_BUGS b/docs/KNOWN_BUGS
index 48c589294..bfa9aec18 100644
--- a/docs/KNOWN_BUGS
+++ b/docs/KNOWN_BUGS
@@ -6,19 +6,6 @@ may have been fixed since this was written!
 18. test case 57 has </test> that should be </client> but when corrected, the
   test case fails!
 
-17. Memory badness:
-  1. create a multi handle
-  2. add an easy handle
-  3. fetch a URL that is persistent (leaves the connection alive)
-  4. remove the easy handle from the multi
-  5. kill the multi handle
-  6. create a multi handle
-  7. add the same easy handle to the new multi handle
-  8. fetch a URL from the same server as before (re-using the connection)
-
-  Use valgrind to see the memory problems when libcurl assumes that the DNS
-  data lives as long as the connection
-
 16. FTP URLs passed to curl may contain NUL (0x00) in the RFC 1738 <user>,
   <password>, and <fpath> components, encoded as "%00".  The problem is that
   curl_unescape does not detect this, but instead returns a shortened C
diff --git a/lib/url.c b/lib/url.c
index f5fc82be4..c8c4360be 100644
--- a/lib/url.c
+++ b/lib/url.c
@@ -1417,12 +1417,6 @@ CURLcode Curl_disconnect(struct connectdata *conn)
 
   data = conn->data;
 
-  if(conn->dns_entry && data->hostcache)
-    /* if the DNS entry is still around, and the host cache is not blanked
-       (which it is for example when a shared one is killed by
-       curl_multi_cleanup() and similar stuff) */
-    Curl_resolv_unlock(data, conn->dns_entry); /* done with this */
-
 #if defined(CURLDEBUG) && defined(AGGRESIVE_TEST)
   /* scan for DNS cache entries still marked as in use */
   Curl_hash_apply(data->hostcache,
@@ -1503,6 +1497,7 @@ CURLcode Curl_disconnect(struct connectdata *conn)
   Curl_safefree(conn->allocptr.ref);
   Curl_safefree(conn->allocptr.host);
   Curl_safefree(conn->allocptr.cookiehost);
+  Curl_safefree(conn->ip_addr_str);
 
 #if defined(USE_ARES) || defined(USE_THREADING_GETHOSTBYNAME) || \
     defined(USE_THREADING_GETADDRINFO)
@@ -1991,10 +1986,22 @@ static void verboseconnect(struct connectdata *conn)
   char addrbuf[256];
 
   /* Get a printable version of the network address. */
-  Curl_printable_address(conn->ip_addr, addrbuf, sizeof(addrbuf));
+  if(!conn->bits.reuse) {
+    Curl_printable_address(conn->ip_addr, addrbuf, sizeof(addrbuf));
+
+    /* save the string */
+    if(conn->ip_addr_str)
+      free(conn->ip_addr_str);
+    conn->ip_addr_str = strdup(addrbuf);
+    if(!conn->ip_addr_str)
+      return; /* FAIL */
+  }
+  /* else,
+     Re-used, ip_addr is not safe to access. */
+
   infof(data, "Connected to %s (%s) port %d\n",
         conn->bits.httpproxy ? conn->proxy.dispname : conn->host.dispname,
-        addrbuf[0] ? addrbuf : "??", conn->port);
+        conn->ip_addr_str, conn->port);
 }
 
 /*
@@ -3533,12 +3540,16 @@ CURLcode Curl_done(struct connectdata **connp,
   struct SessionHandle *data=conn->data;
 
   /* cleanups done even if the connection is re-used */
-
   if(conn->bits.rangestringalloc) {
     free(conn->range);
     conn->bits.rangestringalloc = FALSE;
   }
 
+  if(conn->dns_entry) {
+    Curl_resolv_unlock(data, conn->dns_entry); /* done with this */
+    conn->dns_entry = NULL;
+  }
+
   /* Cleanup possible redirect junk */
   if(conn->newurl) {
     free(conn->newurl);
diff --git a/lib/urldata.h b/lib/urldata.h
index 76de45757..c51ccf303 100644
--- a/lib/urldata.h
+++ b/lib/urldata.h
@@ -450,9 +450,21 @@ struct connectdata {
 #define PROT_FTPS    (1<<9)
 #define PROT_SSL     (1<<10) /* protocol requires SSL */
 
-  /* the particular host we use, in two different ways */
+  /* 'dns_entry' is the particular host we use. This points to an entry in the
+     DNS cache and it will not get pruned while locked. It gets unlocked in
+     Curl_done() */
   struct Curl_dns_entry *dns_entry;
-  Curl_addrinfo *ip_addr; /* the particular IP we connected to */
+
+  /* 'ip_addr' is the particular IP we connected to. It points to a struct
+     within the DNS cache, so this pointer is only valid as long as the DNS
+     cache entry remains locked. It gets unlocked in Curl_done() */
+  Curl_addrinfo *ip_addr;
+
+  /* 'ip_addr_str' is the ip_addr data as a human readable malloc()ed string.
+     It remains available as long as the connection does, which is longer than
+     the ip_addr itself. Currently, this is only set (and used) in
+     url.c:verboseconnect(). */
+  char *ip_addr_str;
 
   char protostr[16];  /* store the protocol string in this buffer */