[patch] fe-connect.c doesn't handle EINTR correctly - Mailing list pgsql-hackers

Last year we had a drawn out discussion about this and I created a patch
for it.  I never noticed that the patch didn't go in until I installed
7.2 the other day and realised that fe-connect.c never was fixed.

Here is the patch again.  It is against CVS 3/16/2002.  This time I only
rewrote the connect procedure at line 912, I leave it up to the regular
hackers to copy it's functionality to the SSL procedure just below it.

In summary, if a software writer implements timer events or other events
which generate a signal with a timing fast enough to occur while libpq
is inside connect(), then connect returns -EINTR.  The code following
the connect call does not handle this and generates an error message.
 The sum result is that the pg_connect() fails.  If the timer or other
event is right on the window of the connect() completion time, the
pg_connect() may appear to work sporadically.  If the event is too slow,
pg_connect() will appear to always work and if the event is too fast,
pg_connect() will always fail.

David

Index: src/interfaces/libpq/fe-connect.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.181
diff -u -r1.181 fe-connect.c
--- src/interfaces/libpq/fe-connect.c    2001/11/11 02:09:05    1.181
+++ src/interfaces/libpq/fe-connect.c    2002/03/16 05:17:47
@@ -909,29 +909,48 @@
      * Thus, we have to make arrangements for all eventualities.
      * ----------
      */
-    if (connect(conn->sock, &conn->raddr.sa, conn->raddr_len) < 0)
-    {
-        if (SOCK_ERRNO == EINPROGRESS || SOCK_ERRNO == EWOULDBLOCK || SOCK_ERRNO == 0)
-        {
-            /*
-             * This is fine - we're in non-blocking mode, and the
-             * connection is in progress.
-             */
-            conn->status = CONNECTION_STARTED;
-        }
-        else
-        {
-            /* Something's gone wrong */
-            connectFailureMessage(conn, SOCK_ERRNO);
-            goto connect_errReturn;
+    do {
+        int e;
+        e=connect(conn->sock, &conn->raddr.sa, conn->raddr_len)
+
+        if(e < 0) {
+            switch (e) {
+                case EINTR:
+                    /*
+                     * Interrupted by a signal, keep trying.  This handling is
+                     * required because the user may have turned on signals in
+                     * his program.  Previously, libpq would erronously fail to
+                     * connect if the user's timer event fired and interrupted
+                     * this syscall.  It is important that we don't try to sleep
+                     * here because this may cause havoc with the user program.
+                     */
+                    continue;
+                    break;
+                case 0:
+                case EINPROGRESS:
+                case EWOULDBLOCK:
+                    /*
+                     * This is fine - we're in non-blocking mode, and the
+                     * connection is in progress.
+                     */
+                    conn->status = CONNECTION_STARTED;
+                    break;
+                default:
+                    /* Something's gone wrong */
+                    connectFailureMessage(conn, SOCK_ERRNO);
+                    goto connect_errReturn;
+                    break;
+            }
+        } else {
+            /* We're connected now */
+            conn->status = CONNECTION_MADE;
         }
-    }
-    else
-    {
-        /* We're connected already */
-        conn->status = CONNECTION_MADE;
-    }
+
+        if(conn->status == CONNECTION_STARTED || conn->status == CONNECTION_MADE)
+            break;

+    } while(1);
+
 #ifdef USE_SSL
     /* Attempt to negotiate SSL usage */
     if (conn->allow_ssl_try)