async_req: fix non-blocking connect()
authorRalph Boehme <slow@samba.org>
Sun, 18 Oct 2015 20:21:10 +0000 (22:21 +0200)
committerJeremy Allison <jra@samba.org>
Tue, 20 Oct 2015 18:22:22 +0000 (20:22 +0200)
According to Stevens UNIX Network Programming and various other sources,
the correct handling for non-blocking connect() is:

- when the initial connect() return -1/EINPROGRESS polling the socket
  for *writeability*

- in the poll handler call getsocktopt() with SO_ERROR to get the
  finished connect() return value

Simply calling connect() a second time without error checking is
probably wrong and not portable. For a successfull connect() Linux
returns 0, but Solaris will return EISCONN:

24254:   0.0336  0.0002 connect(4, 0xFEFFECAC, 16, SOV_DEFAULT) Err#150 EINPROGRESS
24254:          AF_INET  name = 10.10.10.143  port = 1024
24254:   0.0349  0.0001 port_associate(3, 4, 0x00000004, 0x0000001D,0x080648A8) = 0
24254:   0.0495  0.0146 port_getn(3, 0xFEFFEB50, 1, 1, 0xFEFFEB60) = 1 [0]
24254:   0.0497  0.0002 connect(4, 0x080646E4, 16, SOV_DEFAULT) Err#133 EISCONN
24254:          AF_INET  name = 10.10.10.143  port = 1024

Bug: https://bugzilla.samba.org/show_bug.cgi?id=11564

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
lib/async_req/async_sock.c

index bc3780c9199e8bb0b2b40ff1d4bfae6c1d97b398..c0ad8f303b8fcd7b8b1f6e2f5f54ca80f405be3a 100644 (file)
@@ -127,24 +127,17 @@ struct tevent_req *async_connect_send(
                return tevent_req_post(req, ev);
        }
 
-       /**
-        * A number of error messages show that something good is progressing
-        * and that we have to wait for readability.
-        *
-        * If none of them are present, bail out.
+       /*
+        * The only errno indicating that the connect is still in
+        * flight is EINPROGRESS, everything else is an error
         */
 
-       if (!(errno == EINPROGRESS || errno == EALREADY ||
-#ifdef EISCONN
-             errno == EISCONN ||
-#endif
-             errno == EAGAIN || errno == EINTR)) {
+       if (errno != EINPROGRESS) {
                tevent_req_error(req, errno);
                return tevent_req_post(req, ev);
        }
 
-       state->fde = tevent_add_fd(ev, state, fd,
-                                  TEVENT_FD_READ | TEVENT_FD_WRITE,
+       state->fde = tevent_add_fd(ev, state, fd, TEVENT_FD_WRITE,
                                   async_connect_connected, req);
        if (state->fde == NULL) {
                tevent_req_error(req, ENOMEM);
@@ -189,27 +182,32 @@ static void async_connect_connected(struct tevent_context *ev,
        struct async_connect_state *state =
                tevent_req_data(req, struct async_connect_state);
        int ret;
-
-       if (state->before_connect != NULL) {
-               state->before_connect(state->private_data);
-       }
-
-       ret = connect(state->fd, (struct sockaddr *)(void *)&state->address,
-                     state->address_len);
-
-       if (state->after_connect != NULL) {
-               state->after_connect(state->private_data);
-       }
-
-       if (ret == 0) {
-               tevent_req_done(req);
+       int socket_error = 0;
+       socklen_t slen = sizeof(socket_error);
+
+       ret = getsockopt(state->fd, SOL_SOCKET, SO_ERROR,
+                        &socket_error, &slen);
+
+       if (ret != 0) {
+               /*
+                * According to Stevens this is the Solaris behaviour
+                * in case the connection encountered an error:
+                * getsockopt() fails, error is in errno
+                */
+               tevent_req_error(req, errno);
                return;
        }
-       if (errno == EINPROGRESS) {
-               /* Try again later, leave the fde around */
+
+       if (socket_error != 0) {
+               /*
+                * Berkeley derived implementations (including) Linux
+                * return the pending error via socket_error.
+                */
+               tevent_req_error(req, socket_error);
                return;
        }
-       tevent_req_error(req, errno);
+
+       tevent_req_done(req);
        return;
 }