Re: [PATCHES] [patch] fe-connect.c doesn't handle EINTR correctly

David, sorry you patch didn't make it into 7.2.X. That whole EINTR
discussion was quite complicated so I am not surprised we missed it.

The attached patch implements your ENITR test in cases that seems to
need it. I have followed the method we used for ENITRY in fe-misc.c.

---------------------------------------------------------------------------

David Ford wrote:
> 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)

>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/users-lounge/docs/faq.html

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026