multi_socket: re-use of same socket without notifying app

When a hostname resolves to multiple IP addresses and the first one
tried doesn't work, the socket for the second attempt may get dropped on
the floor, causing the request to eventually time out. The issue is that
when using kqueue (as on mac and bsd platforms) instead of select, the
kernel removes the first fd from kqueue when it is closed (in trynextip,
connect.c:503). Trynextip() then goes on to open a new socket, which
gets assigned the same number as the one it just closed. Later in
multi.c, socket_cb is not called because the fd is already in
multi->sockhash, so the new socket is never added to kqueue.

The correct fix is to ensure that socket_cb is called to remove the fd
when trynextip() closes the socket, and again to re-add it after
singleipsocket(). I'm not sure how to cleanly do that, but the attached
patch works around the problem in an admittedly kludgy way by delaying
the close to ensure that the newly-opened socket gets a different fd.

Daniel's added comment: I didn't spot a way to easily do a nicer fix so
I've proceeded with Ben's patch.

Bug: http://curl.haxx.se/bug/view.cgi?id=3017819
Patch by: Ben Darnell

diff --git a/lib/connect.c b/lib/connect.c
index f917ed59586f8dac79a04a52929b92ea0baffe98..12e71278e29194667f1b51c282294cdefdd91f41 100644 (file)
--- a/lib/connect.c
+++ b/lib/connect.c
@@ -499,13 +499,18 @@ static bool trynextip(struct connectdata *conn,
   curl_socket_t sockfd;
   Curl_addrinfo *ai;
 
-  /* first close the failed socket */
-  sclose(conn->sock[sockindex]);
+  /* First clean up after the failed socket.
+     Don't close it yet to ensure that the next IP's socket gets a different
+     file descriptor, which can prevent bugs when the curl_multi_socket_action
+     interface is used with certain select() replacements such as kqueue. */
+  curl_socket_t fd_to_close = conn->sock[sockindex];
   conn->sock[sockindex] = CURL_SOCKET_BAD;
   *connected = FALSE;
 
-  if(sockindex != FIRSTSOCKET)
+  if(sockindex != FIRSTSOCKET) {
+    sclose(fd_to_close);
     return TRUE; /* no next */
+  }
 
   /* try the next address */
   ai = conn->ip_addr->ai_next;
@@ -516,10 +521,12 @@ static bool trynextip(struct connectdata *conn,
       /* store the new socket descriptor */
       conn->sock[sockindex] = sockfd;
       conn->ip_addr = ai;
+      sclose(fd_to_close);
       return FALSE;
     }
     ai = ai->ai_next;
   }
+  sclose(fd_to_close);
   return TRUE;
 }