unix: report bind error in uv_tcp_connect()

Fix a bug where libuv forgets about EADDRINUSE errors reported earlier:
uv_tcp_bind() + uv_tcp_connect() seemingly succeed but the socket isn't
actually bound to the requested address.

This bug goes back to at least 2011 if indeed it ever worked at all.

PR-URL: https://github.com/libuv/libuv/pull/2218
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Jameson Nash <vtjnash@gmail.com>

src/unix/tcp.c

@@ -214,14 +214,15 @@ int uv__tcp_connect(uv_connect_t* req,
   if (handle->connect_req != NULL)
     return UV_EALREADY;  /* FIXME(bnoordhuis) UV_EINVAL or maybe UV_EBUSY. */
 
+  if (handle->delayed_error != 0)
+    goto out;
+
   err = maybe_new_socket(handle,
                          addr->sa_family,
                          UV_HANDLE_READABLE | UV_HANDLE_WRITABLE);
   if (err)
     return err;
 
-  handle->delayed_error = 0;
-
   do {
     errno = 0;
     r = connect(uv__stream_fd(handle), addr, addrlen);
@@ -249,6 +250,8 @@ int uv__tcp_connect(uv_connect_t* req,
       return UV__ERR(errno);
   }
 
+out:
+
   uv__req_init(handle->loop, req, UV_CONNECT);
   req->cb = cb;
   req->handle = (uv_stream_t*) handle;

src/win/tcp.c

@@ -800,9 +800,8 @@ static int uv_tcp_try_connect(uv_connect_t* req,
   if (err)
     return err;
 
-  if (handle->delayed_error) {
-    return handle->delayed_error;
-  }
+  if (handle->delayed_error != 0)
+    goto out;
 
   if (!(handle->flags & UV_HANDLE_BOUND)) {
     if (addrlen == sizeof(uv_addr_ip4_any_)) {
@@ -815,8 +814,8 @@ static int uv_tcp_try_connect(uv_connect_t* req,
     err = uv_tcp_try_bind(handle, bind_addr, addrlen, 0);
     if (err)
       return err;
-    if (handle->delayed_error)
-      return handle->delayed_error;
+    if (handle->delayed_error != 0)
+      goto out;
   }
 
   if (!handle->tcp.conn.func_connectex) {
@@ -844,11 +843,21 @@ static int uv_tcp_try_connect(uv_connect_t* req,
              NULL);
   }
 
+out:
+
   UV_REQ_INIT(req, UV_CONNECT);
   req->handle = (uv_stream_t*) handle;
   req->cb = cb;
   memset(&req->u.io.overlapped, 0, sizeof(req->u.io.overlapped));
 
+  if (handle->delayed_error != 0) {
+    /* Process the req without IOCP. */
+    handle->reqs_pending++;
+    REGISTER_HANDLE_REQ(loop, handle, req);
+    uv_insert_pending_req(loop, (uv_req_t*)req);
+    return 0;
+  }
+
   success = handle->tcp.conn.func_connectex(handle->socket,
                                             (const struct sockaddr*) &converted,
                                             addrlen,
@@ -1215,7 +1224,14 @@ void uv_process_tcp_connect_req(uv_loop_t* loop, uv_tcp_t* handle,
   UNREGISTER_HANDLE_REQ(loop, handle, req);
 
   err = 0;
-  if (REQ_SUCCESS(req)) {
+  if (handle->delayed_error) {
+    /* To smooth over the differences between unixes errors that
+     * were reported synchronously on the first connect can be delayed
+     * until the next tick--which is now.
+     */
+    err = handle->delayed_error;
+    handle->delayed_error = 0;
+  } else if (REQ_SUCCESS(req)) {
     if (handle->flags & UV_HANDLE_CLOSING) {
       /* use UV_ECANCELED for consistency with Unix */
       err = ERROR_OPERATION_ABORTED;

test/echo-server.c

@@ -228,7 +228,7 @@ static int tcp4_echo_start(int port) {
   struct sockaddr_in addr;
   int r;
 
-  ASSERT(0 == uv_ip4_addr("0.0.0.0", port, &addr));
+  ASSERT(0 == uv_ip4_addr("127.0.0.1", port, &addr));
 
   server = (uv_handle_t*)&tcpServer;
   serverType = TCP;

test/test-list.h

@@ -116,7 +116,8 @@ TEST_DECLARE   (tcp_open_bound)
 TEST_DECLARE   (tcp_open_connected)
 TEST_DECLARE   (tcp_connect_error_after_write)
 TEST_DECLARE   (tcp_shutdown_after_write)
-TEST_DECLARE   (tcp_bind_error_addrinuse)
+TEST_DECLARE   (tcp_bind_error_addrinuse_connect)
+TEST_DECLARE   (tcp_bind_error_addrinuse_listen)
 TEST_DECLARE   (tcp_bind_error_addrnotavail_1)
 TEST_DECLARE   (tcp_bind_error_addrnotavail_2)
 TEST_DECLARE   (tcp_bind_error_fault)
@@ -671,7 +672,13 @@ TASK_LIST_START
   TEST_HELPER (tcp_shutdown_after_write, tcp4_echo_server)
 
   TEST_ENTRY  (tcp_connect_error_after_write)
-  TEST_ENTRY  (tcp_bind_error_addrinuse)
+  TEST_ENTRY  (tcp_bind_error_addrinuse_connect)
+  /* tcp4_echo_server steals the port. It needs to be a separate process
+   * because libuv sets setsockopt(SO_REUSEADDR) that lets you steal an
+   * existing bind if it originates from the same process.
+   */
+  TEST_HELPER (tcp_bind_error_addrinuse_connect, tcp4_echo_server)
+  TEST_ENTRY  (tcp_bind_error_addrinuse_listen)
   TEST_ENTRY  (tcp_bind_error_addrnotavail_1)
   TEST_ENTRY  (tcp_bind_error_addrnotavail_2)
   TEST_ENTRY  (tcp_bind_error_fault)

test/test-tcp-bind-error.c

@@ -25,6 +25,7 @@
 #include <stdlib.h>
 
 
+static int connect_cb_called = 0;
 static int close_cb_called = 0;
 
 
@@ -34,7 +35,49 @@ static void close_cb(uv_handle_t* handle) {
 }
 
 
-TEST_IMPL(tcp_bind_error_addrinuse) {
+static void connect_cb(uv_connect_t* req, int status) {
+  ASSERT(status == UV_EADDRINUSE);
+  uv_close((uv_handle_t*) req->handle, close_cb);
+  connect_cb_called++;
+}
+
+
+TEST_IMPL(tcp_bind_error_addrinuse_connect) {
+  struct sockaddr_in addr;
+  int addrlen;
+  uv_connect_t req;
+  uv_tcp_t conn;
+
+  /* 127.0.0.1:<TEST_PORT> is already taken by tcp4_echo_server running in
+   * another process. uv_tcp_bind() and uv_tcp_connect() should still succeed
+   * (greatest common denominator across platforms) but the connect callback
+   * should receive an UV_EADDRINUSE error.
+   */
+  ASSERT(0 == uv_tcp_init(uv_default_loop(), &conn));
+  ASSERT(0 == uv_ip4_addr("127.0.0.1", TEST_PORT, &addr));
+  ASSERT(0 == uv_tcp_bind(&conn, (const struct sockaddr*) &addr, 0));
+
+  ASSERT(0 == uv_ip4_addr("127.0.0.1", TEST_PORT + 1, &addr));
+  ASSERT(0 == uv_tcp_connect(&req,
+                             &conn,
+                             (const struct sockaddr*) &addr,
+                             connect_cb));
+
+  addrlen = sizeof(addr);
+  ASSERT(UV_EADDRINUSE == uv_tcp_getsockname(&conn,
+                                             (struct sockaddr*) &addr,
+                                             &addrlen));
+
+  ASSERT(0 == uv_run(uv_default_loop(), UV_RUN_DEFAULT));
+  ASSERT(connect_cb_called == 1);
+  ASSERT(close_cb_called == 1);
+
+  MAKE_VALGRIND_HAPPY();
+  return 0;
+}
+
+
+TEST_IMPL(tcp_bind_error_addrinuse_listen) {
   struct sockaddr_in addr;
   uv_tcp_t server1, server2;
   int r;