Rework handling of asynchronous connect(2) errors on Unix.

If connect() returns EINPROGRESS, then previously we would detect a
successful connection by the socket becoming selectable for writing,
and spot an unsuccessful one by an error code being returned on the
first attempt to read from it.

This isn't the right way to do it: the right way is to respond to the
initial writability notification by calling getsockopt(SO_ERROR) to
retrieve the error code (if any) from the completed connection
attempt. Doing it the old way had the problem that when the socket
became writable, we could sometimes already have written some of our
outgoing data to it before finding out that the connect attempt failed
- which meant we'd discard that data from the bufchain, and no longer
have it to send through a later successful connection to a different
candidate address.

diff --git a/unix/uxnet.c b/unix/uxnet.c
index 75f809d2b8d0015e1652859fde7486ff7c8ce8bb..903faeb096744e20e943fec174a2f0063166ad9a 100644 (file)
--- a/unix/uxnet.c
+++ b/unix/uxnet.c
@@ -1342,21 +1342,7 @@ static int net_select_result(int fd, int event)
            }
        }
        if (ret < 0) {
-            /*
-             * An error at this point _might_ be an error reported
-             * by a non-blocking connect(). So before we return a
-             * panic status to the user, let's just see whether
-             * that's the case.
-             */
-            int err = errno;
-           if (s->addr) {
-               plug_log(s->plug, 1, s->addr, s->port, strerror(err), err);
-               while (err && s->addr && sk_nextaddr(s->addr, &s->step)) {
-                   err = try_connect(s);
-               }
-           }
-            if (err != 0)
-                return plug_closing(s->plug, strerror(err), err, 0);
+            return plug_closing(s->plug, strerror(errno), errno, 0);
        } else if (0 == ret) {
             s->incomingeof = TRUE;     /* stop trying to read now */
             uxsel_tell(s);
@@ -1378,11 +1364,48 @@ static int net_select_result(int fd, int event)
        if (!s->connected) {
            /*
             * select() reports a socket as _writable_ when an
-            * asynchronous connection is completed.
+            * asynchronous connect() attempt either completes or
+            * fails. So first we must find out which.
             */
+            {
+                int err;
+                socklen_t errlen = sizeof(err);
+                char *errmsg = NULL;
+                if (getsockopt(s->s, SOL_SOCKET, SO_ERROR, &err, &errlen)<0) {
+                    errmsg = dupprintf("getsockopt(SO_ERROR): %s",
+                                       strerror(errno));
+                    err = errno;       /* got to put something in here */
+                } else if (err != 0) {
+                    errmsg = dupstr(strerror(err));
+                }
+                if (errmsg) {
+                    /*
+                     * The asynchronous connection attempt failed.
+                     * Report the problem via plug_log, and try again
+                     * with the next candidate address, if we have
+                     * more than one.
+                     */
+                    assert(s->addr);
+                    plug_log(s->plug, 1, s->addr, s->port, errmsg, err);
+                    while (err && s->addr && sk_nextaddr(s->addr, &s->step)) {
+                        err = try_connect(s);
+                    }
+                    if (err)
+                        return plug_closing(s->plug, strerror(err), err, 0);
+                    if (!s->connected)
+                        return 0;      /* another async attempt in progress */
+                }
+            }
+
+            /*
+             * If we get here, we've managed to make a connection.
+             */
+            if (s->addr) {
+                sk_addr_free(s->addr);
+                s->addr = NULL;
+            }
            s->connected = s->writable = 1;
            uxsel_tell(s);
-           break;
        } else {
            int bufsize_before, bufsize_after;
            s->writable = 1;