Bugs item #2413067, was opened at 2008-12-09 19:59
Message generated for change (Comment added) made by plisiecki
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=2413067&group_id=976

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: libcurl
Group: bad behaviour
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: Phil Lisiecki (plisiecki)
Assigned to: Daniel Stenberg (bagder)
Summary: dns cache memory leak and TTL failure after failed conn

Initial Comment:
After a failed connection attempt, the DNS cache entry appears never to be unlocked. This results in a memory leak and an ever-increasing inuse count. And, since the cache entry can only ever be pruned when both the TTL (DNS_CACHE_TIMEOUT) is expired and inuse=0, the hostname will never be reresolved for the lifetime of that DNS cache.

I have attached a simple program that demonstrates the problem. It takes a URL to download on the command line. Make sure the hostname resolves to an IP that just times out. If you change the hostname's resolution, libcurl continues trying to connect to the original IP forever (as observed by netstat, for example). Also, adding debug print to url.c shows the increasing "inuse" count.

My patch to solve this issue is included below, though you may prefer to solve the problem a different way. (I have only lightly tested the patch at this point.)

--- curl-7.19.2.orig/lib/url.c Tue Dec 9 23:10:34 2008
+++ curl-7.19.2/lib/url.c Wed Dec 10 03:16:48 2008
@@ -2699,6 +2699,7 @@ ConnectionStore(struct SessionHandle *da
 static CURLcode ConnectPlease(struct SessionHandle *data,
                               struct connectdata *conn,
                               struct Curl_dns_entry *hostaddr,
+ bool *hostaddr_used,
                               bool *connected)
 {
   CURLcode result;
@@ -2721,6 +2722,12 @@ static CURLcode ConnectPlease(struct Ses
                            connected);
   if(CURLE_OK == result) {
     /* All is cool, then we store the current information */
+ if (conn->dns_entry != hostaddr) {
+ if (conn->dns_entry && !*hostaddr_used)
+ Curl_resolv_unlock(data, conn->dns_entry);
+ if (hostaddr)
+ *hostaddr_used = TRUE;
+ }
     conn->dns_entry = hostaddr;
     conn->ip_addr = addr;
 
@@ -4354,6 +4361,7 @@ static CURLcode create_conn(struct Sessi
 
 static CURLcode setup_conn(struct connectdata *conn,
                            struct Curl_dns_entry *hostaddr,
+ bool *hostaddr_used,
                            bool *protocol_done)
 {
   CURLcode result=CURLE_OK;
@@ -4406,7 +4414,8 @@ static CURLcode setup_conn(struct connec
        * only for the case where we re-use an existing connection and thus
        * this code section will not be reached with hostaddr == NULL.
        */
- result = ConnectPlease(data, conn, hostaddr, &connected);
+ result = ConnectPlease(data, conn, hostaddr, hostaddr_used, &connected);
+ free_hostaddr = (result != CURLE_OK);
 
       if(connected) {
         result = Curl_protocol_connect(conn, protocol_done);
@@ -4468,6 +4477,7 @@ CURLcode Curl_connect(struct SessionHand
 {
   CURLcode code;
   struct Curl_dns_entry *dns;
+ bool dns_used = FALSE;
 
   *asyncp = FALSE; /* assume synchronous resolves by default */
 
@@ -4485,12 +4495,16 @@ CURLcode Curl_connect(struct SessionHand
         /* If an address is available it means that we already have the name
            resolved, OR it isn't async. if this is a re-used connection 'dns'
            will be NULL here. Continue connecting from here */
- code = setup_conn(*in_connect, dns, protocol_done);
+ code = setup_conn(*in_connect, dns, &dns_used, protocol_done);
       /* else
          response will be received and treated async wise */
     }
   }
 
+ /* Free dns entry if it was not used */
+ if (dns && !dns_used)
+ Curl_resolv_unlock(data, dns);
+
   if(CURLE_OK != code && *in_connect) {
     /* We're not allowed to return failure with memory left allocated
        in the connectdata struct, free those here */
@@ -4511,7 +4525,11 @@ CURLcode Curl_async_resolved(struct conn
 {
 #if defined(USE_ARES) || defined(USE_THREADING_GETHOSTBYNAME) || \
     defined(USE_THREADING_GETADDRINFO)
- CURLcode code = setup_conn(conn, conn->async.dns, protocol_done);
+ bool dns_used = FALSE;
+ CURLcode code = setup_conn(conn, conn->async.dns, &dns_used, protocol_done);
+
+ if (conn->async.dns && !dns_used)
+ Curl_resolv_unlock(data, conn->async.dns);
 
   if(code)
     /* We're not allowed to return failure with memory left allocated

----------------------------------------------------------------------

>Comment By: Phil Lisiecki (plisiecki)
Date: 2008-12-09 20:04

Message:
Revised patch (one line from a previous solution attempt slipped into the
previous patch accidentally):

--- curl-7.19.2.orig/lib/url.c Tue Dec 9 23:10:34 2008
+++ curl-7.19.2/lib/url.c Wed Dec 10 03:16:48 2008
@@ -2699,6 +2699,7 @@ ConnectionStore(struct SessionHandle *da
 static CURLcode ConnectPlease(struct SessionHandle *data,
                               struct connectdata *conn,
                               struct Curl_dns_entry *hostaddr,
+ bool *hostaddr_used,
                               bool *connected)
 {
   CURLcode result;
@@ -2721,6 +2722,12 @@ static CURLcode ConnectPlease(struct Ses
                            connected);
   if(CURLE_OK == result) {
     /* All is cool, then we store the current information */
+ if (conn->dns_entry != hostaddr) {
+ if (conn->dns_entry && !*hostaddr_used)
+ Curl_resolv_unlock(data, conn->dns_entry);
+ if (hostaddr)
+ *hostaddr_used = TRUE;
+ }
     conn->dns_entry = hostaddr;
     conn->ip_addr = addr;
 
@@ -4354,6 +4361,7 @@ static CURLcode create_conn(struct Sessi
 
 static CURLcode setup_conn(struct connectdata *conn,
                            struct Curl_dns_entry *hostaddr,
+ bool *hostaddr_used,
                            bool *protocol_done)
 {
   CURLcode result=CURLE_OK;
@@ -4406,7 +4414,7 @@ static CURLcode setup_conn(struct connec
        * only for the case where we re-use an existing connection and
thus
        * this code section will not be reached with hostaddr == NULL.
        */
- result = ConnectPlease(data, conn, hostaddr, &connected);
+ result = ConnectPlease(data, conn, hostaddr, hostaddr_used,
&connected);
 
       if(connected) {
         result = Curl_protocol_connect(conn, protocol_done);
@@ -4468,6 +4476,7 @@ CURLcode Curl_connect(struct SessionHand
 {
   CURLcode code;
   struct Curl_dns_entry *dns;
+ bool dns_used = FALSE;
 
   *asyncp = FALSE; /* assume synchronous resolves by default */
 
@@ -4485,12 +4494,16 @@ CURLcode Curl_connect(struct SessionHand
         /* If an address is available it means that we already have the
name
            resolved, OR it isn't async. if this is a re-used connection
'dns'
            will be NULL here. Continue connecting from here */
- code = setup_conn(*in_connect, dns, protocol_done);
+ code = setup_conn(*in_connect, dns, &dns_used, protocol_done);
       /* else
          response will be received and treated async wise */
     }
   }
 
+ /* Free dns entry if it was not used */
+ if (dns && !dns_used)
+ Curl_resolv_unlock(data, dns);
+
   if(CURLE_OK != code && *in_connect) {
     /* We're not allowed to return failure with memory left allocated
        in the connectdata struct, free those here */
@@ -4511,7 +4524,11 @@ CURLcode Curl_async_resolved(struct conn
 {
 #if defined(USE_ARES) || defined(USE_THREADING_GETHOSTBYNAME) || \
     defined(USE_THREADING_GETADDRINFO)
- CURLcode code = setup_conn(conn, conn->async.dns, protocol_done);
+ bool dns_used = FALSE;
+ CURLcode code = setup_conn(conn, conn->async.dns, &dns_used,
protocol_done);
+
+ if (conn->async.dns && !dns_used)
+ Curl_resolv_unlock(data, conn->async.dns);
 
   if(code)
     /* We're not allowed to return failure with memory left allocated

----------------------------------------------------------------------

You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=2413067&group_id=976
Received on 2008-12-10