pg_dump possible fix, need testers. (was: Re: [HACKERS] pg_dump disaster)

* Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> [000121 14:35] wrote:
> Alfred Perlstein <bright(at)wintelcom(dot)net> writes:
> > We both missed it, and yes it was my fault. All connections are
> > behaving as if PQsetnonblocking(conn, TRUE) have been called on them.
> > The original non-blocking patches did something weird, they seemed
> > to _always_ stick the socket into non-blocking mode. This would
> > activate my non-blocking stuff for all connections.
>
> Yes, the present state of the code seems to activate nonblocking socket
> mode all the time; possibly we could band-aid our way back to a working
> psql by turning off nonblock mode by default. But this doesn't address
> the fact that the API of these routines cannot support nonblock mode
> without being redesigned.

These patches revert the default setting of the non-block flag back
to the old way connections were done. Since i'm unable to reproduce
this bug I'm hoping people can _please_ give me feedback.

This is just a first shot at fixing the issue, I'll supply changes
to the docs if this all goes well (that you must explicitly set the
blocking status after a connect/disconnect)

I'm a bit concerned about busy looping because the connection is
left in a non-blocking state after the connect, however most of
the code performs select() and checks for EWOULDBLOCK/EAGAIN so it
might not be an issue.

Thanks for holding off on backing out the changes.

Summary:
don't set the nonblock flag during connections
PQsetnonblocking doesn't fiddle with socket flags anymore as the library
seems to be setup to deal with the socket being in non-block mode at
all times
turn off the nonblock flag if/when the connection is torn down to ensure
that a reconnect behaves like it used to.

Index: fe-connect.c
===================================================================
RCS file: /home/pgcvs/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.114
diff -u -c -I$Header: -r1.114 fe-connect.c
cvs diff: conflicting specifications of output style
*** fe-connect.c 2000/01/23 01:27:39 1.114
--- fe-connect.c 2000/01/23 08:56:17
***************
*** 391,397 ****
PGconn *conn;
char *tmp; /* An error message from some service we call. */
bool error = FALSE; /* We encountered an error. */
- int i;

conn = makeEmptyPGconn();
if (conn == NULL)
--- 391,396 ----
***************
*** 586,591 ****
--- 585,614 ----
}

/* ----------
+ * connectMakeNonblocking -
+ * Make a connection non-blocking.
+ * Returns 1 if successful, 0 if not.
+ * ----------
+ */
+ static int
+ connectMakeNonblocking(PGconn *conn)
+ {
+ #ifndef WIN32
+ if (fcntl(conn->sock, F_SETFL, O_NONBLOCK) < 0)
+ #else
+ if (ioctlsocket(conn->sock, FIONBIO, &on) != 0)
+ #endif
+ {
+ printfPQExpBuffer(&conn->errorMessage,
+ "connectMakeNonblocking -- fcntl() failed: errno=%d\n%s\n",
+ errno, strerror(errno));
+ return 0;
+ }
+
+ return 1;
+ }
+
+ /* ----------
* connectNoDelay -
* Sets the TCP_NODELAY socket option.
* Returns 1 if successful, 0 if not.
***************
*** 755,761 ****
* Ewan Mellor <eem21(at)cam(dot)ac(dot)uk>.
* ---------- */
#if (!defined(WIN32) || defined(WIN32_NON_BLOCKING_CONNECTIONS)) && !defined(USE_SSL)
! if (PQsetnonblocking(conn, TRUE) != 0)
goto connect_errReturn;
#endif

--- 778,784 ----
* Ewan Mellor <eem21(at)cam(dot)ac(dot)uk>.
* ---------- */
#if (!defined(WIN32) || defined(WIN32_NON_BLOCKING_CONNECTIONS)) && !defined(USE_SSL)
! if (connectMakeNonblocking(conn) == 0)
goto connect_errReturn;
#endif

***************
*** 868,874 ****
/* This makes the connection non-blocking, for all those cases which forced us
not to do it above. */
#if (defined(WIN32) && !defined(WIN32_NON_BLOCKING_CONNECTIONS)) || defined(USE_SSL)
! if (PQsetnonblocking(conn, TRUE) != 0)
goto connect_errReturn;
#endif

--- 891,897 ----
/* This makes the connection non-blocking, for all those cases which forced us
not to do it above. */
#if (defined(WIN32) && !defined(WIN32_NON_BLOCKING_CONNECTIONS)) || defined(USE_SSL)
! if (connectMakeNonblocking(conn) == 0)
goto connect_errReturn;
#endif

***************
*** 1785,1790 ****
--- 1808,1820 ----
(void) pqPuts("X", conn);
(void) pqFlush(conn);
}
+
+ /*
+ * must reset the blocking status so a possible reconnect will work
+ * don't call PQsetnonblocking() because it will fail if it's unable
+ * to flush the connection.
+ */
+ conn->nonblocking = FALSE;

/*
* Close the connection, reset all transient state, flush I/O buffers.
Index: fe-exec.c
===================================================================
RCS file: /home/pgcvs/pgsql/src/interfaces/libpq/fe-exec.c,v
retrieving revision 1.87
diff -u -c -I$Header: -r1.87 fe-exec.c
cvs diff: conflicting specifications of output style
*** fe-exec.c 2000/01/18 06:09:24 1.87
--- fe-exec.c 2000/01/23 08:56:29
***************
*** 2116,2122 ****
int
PQsetnonblocking(PGconn *conn, int arg)
{
- int fcntlarg;

arg = (arg == TRUE) ? 1 : 0;
/* early out if the socket is already in the state requested */
--- 2116,2121 ----
***************
*** 2131,2174 ****
* _from_ or _to_ blocking mode, either way we can block them.
*/
/* if we are going from blocking to non-blocking flush here */
! if (!pqIsnonblocking(conn) && pqFlush(conn))
! return (-1);
!
!
! #ifdef USE_SSL
! if (conn->ssl)
! {
! printfPQExpBuffer(&conn->errorMessage,
! "PQsetnonblocking() -- not supported when using SSL\n");
! return (-1);
! }
! #endif /* USE_SSL */
!
! #ifndef WIN32
! fcntlarg = fcntl(conn->sock, F_GETFL, 0);
! if (fcntlarg == -1)
! return (-1);
!
! if ((arg == TRUE &&
! fcntl(conn->sock, F_SETFL, fcntlarg | O_NONBLOCK) == -1) ||
! (arg == FALSE &&
! fcntl(conn->sock, F_SETFL, fcntlarg & ~O_NONBLOCK) == -1))
! #else
! fcntlarg = arg;
! if (ioctlsocket(conn->sock, FIONBIO, &fcntlarg) != 0)
! #endif
! {
! printfPQExpBuffer(&conn->errorMessage,
! "PQsetblocking() -- unable to set nonblocking status to %s\n",
! arg == TRUE ? "TRUE" : "FALSE");
return (-1);
- }

conn->nonblocking = arg;
-
- /* if we are going from non-blocking to blocking flush here */
- if (pqIsnonblocking(conn) && pqFlush(conn))
- return (-1);

return (0);
}
--- 2130,2139 ----
* _from_ or _to_ blocking mode, either way we can block them.
*/
/* if we are going from blocking to non-blocking flush here */
! if (pqFlush(conn))
return (-1);

conn->nonblocking = arg;

return (0);
}

--
-Alfred Perlstein - [bright(at)wintelcom(dot)net|alfred(at)freebsd(dot)org]