connect: Add connection delay to Happy Eyeballs.

This patch adds a 200ms delay between the first and second address
family socket connection attempts.

It also iterates over IP addresses in the order returned by the
system, meaning most dual-stack systems will try IPv6 first.

Additionally, it refactors the connect code, removing most code that
handled synchronous connects. Since all sockets are now non-blocking,
the logic can be made simpler.

5 files changed, 82 insertions, 209 deletions

diff --git a/lib/connect.c b/lib/connect.c
index 672eb697d..2fd9883b9 100644
--- a/lib/connect.c
+++ b/lib/connect.c
@@ -164,8 +164,7 @@ tcpkeepalive(struct SessionHandle *data,
 static CURLcode
 singleipconnect(struct connectdata *conn,
                 const Curl_addrinfo *ai, /* start connecting to this */
-                curl_socket_t *sock,
-                bool *connected);
+                curl_socket_t *sock);
 
 /*
  * Curl_timeleft() returns the amount of milliseconds left allowed for the
@@ -534,12 +533,9 @@ static bool verifyconnect(curl_socket_t sockfd, int *error)
    more address exists or error */
 static CURLcode trynextip(struct connectdata *conn,
                           int sockindex,
-                          int tempindex,
-                          bool *connected)
+                          int tempindex)
 {
-  curl_socket_t sockfd;
-  Curl_addrinfo *ai;
-  int family = tempindex ? AF_INET6 : AF_INET;
+  CURLcode rc = CURLE_COULDNT_CONNECT;
 
   /* First clean up after the failed socket.
      Don't close it yet to ensure that the next IP's socket gets a different
@@ -547,36 +543,35 @@ static CURLcode trynextip(struct connectdata *conn,
      interface is used with certain select() replacements such as kqueue. */
   curl_socket_t fd_to_close = conn->tempsock[tempindex];
   conn->tempsock[tempindex] = CURL_SOCKET_BAD;
-  *connected = FALSE;
 
-  if(sockindex != FIRSTSOCKET) {
-    Curl_closesocket(conn, fd_to_close);
-    return CURLE_COULDNT_CONNECT; /* no next */
-  }
+  if(sockindex == FIRSTSOCKET) {
+    Curl_addrinfo *ai;
+    int family;
 
-  /* try the next address with same family */
-  ai = conn->tempaddr[tempindex]->ai_next;
-  while(ai && ai->ai_family != family)
-    ai = ai->ai_next;
-
-  while(ai && ai->ai_family == family) {
-    CURLcode res = singleipconnect(conn, ai, &sockfd, connected);
-    if(res)
-      return res;
-    if(sockfd != CURL_SOCKET_BAD) {
-      /* store the new socket descriptor */
-      conn->tempsock[tempindex] = sockfd;
-      conn->tempaddr[tempindex] = ai;
-      Curl_closesocket(conn, fd_to_close);
-      return CURLE_OK;
+    if(conn->tempaddr[tempindex]) {
+      /* find next address in the same protocol family */
+      family = conn->tempaddr[tempindex]->ai_family;
+      ai = conn->tempaddr[tempindex]->ai_next;
+    }
+    else {
+      /* happy eyeballs - try the other protocol family */
+      int firstfamily = conn->tempaddr[0]->ai_family;
+      family = (firstfamily == AF_INET) ? AF_INET6 : AF_INET;
+      ai = conn->tempaddr[0]->ai_next;
     }
 
-    do {
+    while(ai && ai->ai_family != family)
       ai = ai->ai_next;
-    } while(ai && ai->ai_family != family);
+    if(ai) {
+      rc = singleipconnect(conn, ai, &conn->tempsock[tempindex]);
+      conn->tempaddr[tempindex] = ai;
+    }
   }
-  Curl_closesocket(conn, fd_to_close);
-  return CURLE_COULDNT_CONNECT;
+
+  if(fd_to_close != CURL_SOCKET_BAD)
+    Curl_closesocket(conn, fd_to_close);
+
+  return rc;
 }
 
 /* Copies connection info into the session handle to make it available
@@ -701,7 +696,7 @@ CURLcode Curl_is_connected(struct connectdata *conn,
 {
   struct SessionHandle *data = conn->data;
   CURLcode code = CURLE_OK;
-  long allow = DEFAULT_CONNECT_TIMEOUT;
+  long allow;
   int error = 0;
   struct timeval now;
   int result;
@@ -748,6 +743,12 @@ CURLcode Curl_is_connected(struct connectdata *conn,
               conn->timeoutms_per_addr);
         error = ETIMEDOUT;
       }
+
+      /* should we try another protocol family? */
+      if(i == 0 && conn->tempaddr[1] == NULL &&
+         curlx_tvdiff(now, conn->connecttime) >= HAPPY_EYEBALLS_TIMEOUT) {
+        trynextip(conn, sockindex, 1);
+      }
     }
     else if(result == CURL_CSELECT_OUT) {
       if(verifyconnect(conn->tempsock[i], &error)) {
@@ -759,13 +760,8 @@ CURLcode Curl_is_connected(struct connectdata *conn,
         conn->ip_addr = conn->tempaddr[i];
 
         /* close the other socket, if open */
-        if(conn->tempsock[other] != CURL_SOCKET_BAD) {
-          if(conn->fclosesocket)
-            conn->fclosesocket(conn->closesocket_client,
-                               conn->tempsock[other]);
-          else
-            sclose(conn->tempsock[other]);
-        }
+        if(conn->tempsock[other] != CURL_SOCKET_BAD)
+          Curl_closesocket(conn, conn->tempsock[other]);
 
         /* see if we need to do any proxy magic first once we connected */
         code = Curl_connected_proxy(conn, sockindex);
@@ -797,18 +793,28 @@ CURLcode Curl_is_connected(struct connectdata *conn,
       data->state.os_errno = error;
       SET_SOCKERRNO(error);
       Curl_printable_address(conn->tempaddr[i], ipaddress, MAX_IPADR_LEN);
-      infof(data, "connect to %s port %ld: %s\n",
+      infof(data, "connect to %s port %ld failed: %s\n",
             ipaddress, conn->port, Curl_strerror(conn, error));
 
       conn->timeoutms_per_addr = conn->tempaddr[i]->ai_next == NULL ?
                                  allow : allow / 2;
 
-      code = trynextip(conn, sockindex, i, connected);
+      code = trynextip(conn, sockindex, i);
     }
   }
 
   if(code) {
     /* no more addresses to try */
+
+    /* if the first address family runs out of addresses to try before
+       the happy eyeball timeout, go ahead and try the next family now */
+    if(conn->tempaddr[1] == NULL) {
+      int rc;
+      rc = trynextip(conn, sockindex, 1);
+      if(rc == CURLE_OK)
+        return CURLE_OK;
+    }
+
     failf(data, "Failed to connect to %s port %ld: %s",
           conn->host.name, conn->port, Curl_strerror(conn, error));
   }
@@ -927,8 +933,7 @@ void Curl_sndbufset(curl_socket_t sockfd)
 static CURLcode
 singleipconnect(struct connectdata *conn,
                 const Curl_addrinfo *ai,
-                curl_socket_t *sockp,
-                bool *connected)
+                curl_socket_t *sockp)
 {
   struct Curl_sockaddr_ex addr;
   int rc;
@@ -941,7 +946,6 @@ singleipconnect(struct connectdata *conn,
   long port;
 
   *sockp = CURL_SOCKET_BAD;
-  *connected = FALSE; /* default is not connected */
 
   res = Curl_socket(conn, ai, &addr, &sockfd);
   if(res)
@@ -1038,14 +1042,12 @@ singleipconnect(struct connectdata *conn,
 
     default:
       /* unknown error, fallthrough and try another address! */
-      failf(data, "Failed to connect to %s: %s",
-            conn->ip_addr_str, Curl_strerror(conn,error));
+      infof(data, "Immediate connect fail for %s: %s\n",
+            ipaddress, Curl_strerror(conn,error));
       data->state.os_errno = error;
 
       /* connect failed */
-      Curl_closesocket(conn, sockfd);
-
-      break;
+      return CURLE_COULDNT_CONNECT;
     }
   }
   else
@@ -1061,23 +1063,13 @@ singleipconnect(struct connectdata *conn,
  */
 
 CURLcode Curl_connecthost(struct connectdata *conn,  /* context */
-                          const struct Curl_dns_entry *remotehost,
-                          bool *connected)           /* really connected? */
+                          const struct Curl_dns_entry *remotehost)
 {
   struct SessionHandle *data = conn->data;
-  struct timeval after;
   struct timeval before = Curl_tvnow();
-  int i;
-
-  /*************************************************************
-   * Figure out what maximum time we have left
-   *************************************************************/
-  long timeout_ms;
-
-  *connected = FALSE; /* default to not connected */
+  CURLcode res;
 
-  /* get the timeout left */
-  timeout_ms = Curl_timeleft(data, &before, TRUE);
+  long timeout_ms = Curl_timeleft(data, &before, TRUE);
 
   if(timeout_ms < 0) {
     /* a precaution, no need to continue if time already is up */
@@ -1087,69 +1079,26 @@ CURLcode Curl_connecthost(struct connectdata *conn,  /* context */
 
   conn->num_addr = Curl_num_addresses(remotehost->addr);
   conn->tempaddr[0] = remotehost->addr;
-  conn->tempaddr[1] = remotehost->addr;
-
-  /* Below is the loop that attempts to connect to all IP-addresses we
-   * know for the given host.
-   * One by one, for each protocol, until one IP succeeds.
-   */
-
-  for(i=0; i<2; i++) {
-    curl_socket_t sockfd = CURL_SOCKET_BAD;
-    Curl_addrinfo *ai = conn->tempaddr[i];
-    int family = i ? AF_INET6 : AF_INET;
-
-    /* find first address for this address family, if any */
-    while(ai && ai->ai_family != family)
-      ai = ai->ai_next;
-
-    /*
-     * Connecting with a Curl_addrinfo chain
-     */
-    while(ai) {
-      CURLcode res;
-
-      /* Max time for the next connection attempt */
-      conn->timeoutms_per_addr = ai->ai_next == NULL ?
-                                 timeout_ms : timeout_ms / 2;
-
-      /* start connecting to the IP curr_addr points to */
-      res = singleipconnect(conn, ai, &sockfd, connected);
-      if(res)
-        return res;
-
-      if(sockfd != CURL_SOCKET_BAD)
-        break;
-
-      /* get a new timeout for next attempt */
-      after = Curl_tvnow();
-      timeout_ms -= Curl_tvdiff(after, before);
-      if(timeout_ms < 0) {
-        failf(data, "connect() timed out!");
-        return CURLE_OPERATION_TIMEDOUT;
-      }
-      before = after;
-
-      /* next addresses */
-      do {
-        ai = ai->ai_next;
-      } while(ai && ai->ai_family != family);
-    }  /* end of connect-to-each-address loop */
-
-    conn->tempsock[i] = sockfd;
-    conn->tempaddr[i] = ai;
-  }
-
-  if((conn->tempsock[0] == CURL_SOCKET_BAD) &&
-     (conn->tempsock[1] == CURL_SOCKET_BAD)) {
-    /* no good connect was made */
-    failf(data, "couldn't connect to %s at %s:%ld",
-          conn->bits.proxy?"proxy":"host",
-          conn->bits.proxy?conn->proxy.name:conn->host.name, conn->port);
-    return CURLE_COULDNT_CONNECT;
-  }
-
-  /* leave the socket in non-blocking mode */
+  conn->tempaddr[1] = NULL;
+  conn->tempsock[0] = CURL_SOCKET_BAD;
+  conn->tempsock[1] = CURL_SOCKET_BAD;
+  Curl_expire(conn->data,
+              HAPPY_EYEBALLS_TIMEOUT + (MULTI_TIMEOUT_INACCURACY/1000));
+
+  /* Max time for the next connection attempt */
+  conn->timeoutms_per_addr =
+    conn->tempaddr[0]->ai_next == NULL ? timeout_ms : timeout_ms / 2;
+
+  /* start connecting to first IP */
+  res = singleipconnect(conn, conn->tempaddr[0], &(conn->tempsock[0]));
+  while(res != CURLE_OK &&
+        conn->tempaddr[0] &&
+        conn->tempaddr[0]->ai_next &&
+        conn->tempsock[0] == CURL_SOCKET_BAD)
+    res = trynextip(conn, FIRSTSOCKET, 0);
+
+  if(conn->tempsock[0] == CURL_SOCKET_BAD)
+    return res;
 
   data->info.numconnects++; /* to track the number of connections made */
 
diff --git a/lib/connect.h b/lib/connect.h
index 526ce37a0..925163941 100644
--- a/lib/connect.h
+++ b/lib/connect.h
@@ -31,9 +31,7 @@ CURLcode Curl_is_connected(struct connectdata *conn,
                            bool *connected);
 
 CURLcode Curl_connecthost(struct connectdata *conn,
-                          const struct Curl_dns_entry *host, /* connect to
-                                                                this */
-                          bool *connected); /* truly connected? */
+                          const struct Curl_dns_entry *host);
 
 /* generic function that returns how much time there's left to run, according
    to the timeouts set */
@@ -42,6 +40,8 @@ long Curl_timeleft(struct SessionHandle *data,
                    bool duringconnect);
 
 #define DEFAULT_CONNECT_TIMEOUT 300000 /* milliseconds == five minutes */
+#define HAPPY_EYEBALLS_TIMEOUT     200 /* milliseconds to wait between
+                                          ipv4/ipv6 connection attempts */
 
 /*
  * Used to extract socket and connectdata struct for the most recent
diff --git a/lib/ftp.c b/lib/ftp.c
index 46ec8a6f6..8879ff1d9 100644
--- a/lib/ftp.c
+++ b/lib/ftp.c
@@ -1884,7 +1884,6 @@ static CURLcode ftp_state_pasv_resp(struct connectdata *conn,
   struct Curl_dns_entry *addr=NULL;
   int rc;
   unsigned short connectport; /* the local port connect() should use! */
-  bool connected;
   char *str=&data->state.buffer[4];  /* start on the first letter */
 
   if((ftpc->count1 == 0) &&
@@ -2038,9 +2037,8 @@ static CURLcode ftp_state_pasv_resp(struct connectdata *conn,
     }
   }
 
-  result = Curl_connecthost(conn,
-                            addr,
-                            &connected);
+  conn->bits.tcpconnect[SECONDARYSOCKET] = FALSE;
+  result = Curl_connecthost(conn, addr);
 
   Curl_resolv_unlock(data, addr); /* we're done using this address */
 
@@ -2051,7 +2049,6 @@ static CURLcode ftp_state_pasv_resp(struct connectdata *conn,
     return result;
   }
 
-  conn->bits.tcpconnect[SECONDARYSOCKET] = connected;
 
   /*
    * When this is used from the multi interface, this might've returned with
@@ -2063,15 +2060,6 @@ static CURLcode ftp_state_pasv_resp(struct connectdata *conn,
     /* this just dumps information about this second connection */
     ftp_pasv_verbose(conn, conn->ip_addr, ftpc->newhost, connectport);
 
-  if(connected) {
-    /* Only do the proxy connection magic if we're actually connected.  We do
-       this little trick and send in the same 'connected' variable here again
-       and it will be set FALSE by proxy_magic() for when for example the
-       CONNECT procedure doesn't complete */
-    infof(data, "Connection to proxy confirmed almost instantly\n");
-    result = proxy_magic(conn, ftpc->newhost, ftpc->newport, &connected);
-  }
-  conn->bits.tcpconnect[SECONDARYSOCKET] = connected;
   conn->bits.do_more = TRUE;
   state(conn, FTP_STOP); /* this phase is completed */
 
diff --git a/lib/multi.c b/lib/multi.c
index e6c2934a6..722cd8625 100644
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -874,6 +874,7 @@ CURLMcode curl_multi_wait(CURLM *multi_handle,
 
   if(nfds) {
     /* wait... */
+    infof(data, "Curl_poll(%d ds, %d ms)\n", nfds, timeout_ms);
     i = Curl_poll(ufds, nfds, timeout_ms);
 
     if(i) {
diff --git a/lib/url.c b/lib/url.c
index 03c76078b..b069375a7 100644
--- a/lib/url.c
+++ b/lib/url.c
@@ -3256,43 +3256,6 @@ CURLcode Curl_connected_proxy(struct connectdata *conn,
   return CURLE_OK;
 }
 
-static CURLcode ConnectPlease(struct SessionHandle *data,
-                              struct connectdata *conn,
-                              bool *connected)
-{
-  CURLcode result;
-#ifndef CURL_DISABLE_VERBOSE_STRINGS
-  char *hostname = conn->bits.proxy?conn->proxy.name:conn->host.name;
-
-  infof(data, "About to connect() to %s%s port %ld (#%ld)\n",
-        conn->bits.proxy?"proxy ":"",
-        hostname, conn->port, conn->connection_id);
-#else
-  (void)data;
-#endif
-
-  /*************************************************************
-   * Connect to server/proxy
-   *************************************************************/
-  result= Curl_connecthost(conn,
-                           conn->dns_entry,
-                           connected);
-  if(CURLE_OK == result) {
-    if(*connected) {
-      result = Curl_connected_proxy(conn, FIRSTSOCKET);
-      if(!result) {
-        conn->bits.tcpconnect[FIRSTSOCKET] = TRUE;
-        Curl_pgrsTime(data, TIMER_CONNECT); /* connect done */
-      }
-    }
-  }
-
-  if(result)
-    *connected = FALSE; /* mark it as not connected */
-
-  return result;
-}
-
 /*
  * verboseconnect() displays verbose information after a connect
  */
@@ -5600,36 +5563,8 @@ CURLcode Curl_setup_conn(struct connectdata *conn,
     /* loop for CURL_SERVER_CLOSED_CONNECTION */
 
     if(CURL_SOCKET_BAD == conn->sock[FIRSTSOCKET]) {
-      /* Try to connect only if not already connected */
-      bool connected = FALSE;
-
-      result = ConnectPlease(data, conn, &connected);
-
-      if(result && !conn->ip_addr) {
-        /* transport connection failure not related with authentication */
-        conn->bits.tcpconnect[FIRSTSOCKET] = FALSE;
-        return result;
-      }
-
-      if(connected) {
-        result = Curl_protocol_connect(conn, protocol_done);
-        if(CURLE_OK == result)
-          conn->bits.tcpconnect[FIRSTSOCKET] = TRUE;
-      }
-      else
-        conn->bits.tcpconnect[FIRSTSOCKET] = FALSE;
-
-      /* if the connection was closed by the server while exchanging
-         authentication informations, retry with the new set
-         authentication information */
-      if(conn->bits.proxy_connect_closed) {
-        /* reset the error buffer */
-        if(data->set.errorbuffer)
-          data->set.errorbuffer[0] = '\0';
-        data->state.errorbuf = FALSE;
-        continue;
-      }
-
+      conn->bits.tcpconnect[FIRSTSOCKET] = FALSE;
+      result = Curl_connecthost(conn, conn->dns_entry);
       if(CURLE_OK != result)
         return result;
     }