>
>
>
>No, it should *not* look like that.  The fe-connect.c code is designed
>to move on as soon as it's convinced that the kernel has accepted the
>connection request.  We use a non-blocking connect() call and later
>wait for connection complete by probing the select() status.  Looping
>on the connect() itself would be a busy-wait, which would be antisocial.
>

The fe-connect.c code moves on regardless of the completion of the 
connect() if it has been interrupted.

To simplify, in a program without SIGALRM events, PQconnect* won't be 
interrupted.  The connect() call will complete properly.

In a program with SIGALRM events, the call is interrupted inside 
connect().  If SA_RESTART was disabled for connect() in POSIX semantics, 
the  program would automatically jump right back into the connect() 
call.  However by default POSIX code enables SA_RESTART which for 
SIGALRM means -don't- automatically restart the system call.  This means 
the programmer needs to check for -1/errno=EINTR and jump back into 
connect() himself.  There isn't a concern for busy wait/anti social code 
behavior, your program was in the middle of connect() when it was 
interrupted, you're simply jumping back to where you left off.

It doesn't matter if it is a blocking connect or non-blocking connect, 
handling EINTR must be done if SIGALRM events are employed.  A fast 
enough event timer with a non-blocking connect will also be susceptible 
to EINTR.

EINTR is distinctly different from EINPROGRESS.  If they were the same 
then there would be a problem.  EINTR should be handled by jumping back 
into the connect() call, it is re-entrant and designed for this.

Regardless, you don't wait for the connection to complete, the code 
following the connect() call returns failure for every -1 result from 
connect() unless it is EINPROGRESS or EWOULDBLOCK.  select() is -not- 
used in fe-connect.c.  It is possible with the current code for the 
connection to fail in non-blocking mode.  Reason: you call connect() in 
non-blocking mode, break out of the section on EINPROGRESS, and continue 
assuming that the connection will be successful.

       EINPROGRESS
              The  socket is non-blocking and the connection can
              not be completed immediately.  It  is  possible  to
              select(2)  or  poll(2)  for completion by selecting
              the socket  for  writing.  After  select  indicates
              writability, use getsockopt(2) to read the SO_ERROR
              option at level  SOL_SOCKET  to  determine  whether
              connect  completed  successfully (SO_ERROR is zero)
              or unsuccessfully (SO_ERROR is  one  of  the  usual
              error  codes listed here, explaining the reason for
              the failure).

The socket is not checked any further after the connect().  The code 
should not continue on into the SSL handling until you're sure that the 
socket is ready for operation.

The reason why I am getting EINTR from a non-blocking connect is because 
my event timer happens to fire in the middle of the connect() call. 
 Just because you set the socket to FIONBIO doesn't mean that connect() 
can't be interrupted.

David



Fri, 26 Oct 2001 20:15:30 -0400
David Ford <david@blue-labs.org>
Re: [patch] helps fe-connect.c handle -EINTR more gracefully
David Ford <david@blue-labs.org> writes:
> [ much ]

I think you are missing the point.  I am not saying that we shouldn't
deal with EINTR; rather I am raising what I think is a legitimate
question: *what* is the most appropriate response?  My reading of
HP's gloss suggests that we could treat EINTR the same as EINPROGRESS,
ie, consider the connect() to have succeeded and move on to the
wait-for-connection-complete-or-failure-using-select() phase.
If that works I would prefer it to repeating the connect() call,
primarily because it avoids any possibility of introducing an
infinite loop.

For PQrequestCancel we clearly do need to retry the connect(), since
that use of connect() isn't nonblocking.  But I'm not convinced that
we should do so in the main-line connection code.

> It is possible with the current code for the 
> connection to fail in non-blocking mode.  Reason: you call connect() in 
> non-blocking mode, break out of the section on EINPROGRESS, and continue 
> assuming that the connection will be successful.

No, we don't.  If you think that, then you haven't studied the code
sufficiently to be submitting patches for it.  What we actually do
is exactly what is recommended by the manpage you're quoting at me.
It's just split across multiple routines.

			regards, tom lane

Fri, 26 Oct 2001 21:11:49 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: [patch] helps fe-connect.c handle -EINTR more gracefully 
Actually, now that I look at this another time, there's an interesting
question to ask: have you compiled with USE_SSL?

The USE_SSL case definitely is broken, since it invokes the connect()
in blocking mode, but fails to retry on EINTR, which it clearly should
do in that mode.  (What's even worse is that you cannot suppress
the problem by setting allow_ssl_try false.  If you compiled with SSL
support enabled then you lose anyway.)

I think we can fix this by moving the SSL startup code to a saner place,
namely in PQconnectPoll just before it's about to send the startup
packet.  There's no reason why we shouldn't *always* do the connect()
in nonblock mode.  We could switch the socket back to blocking mode
while invoking the SSL negotiation phase (which'd be skipped if not
allow_ssl_try, so that a library compiled with USE_SSL isn't ipso
facto broken for people who want non-SSL nonblocking connect).

If you are testing with USE_SSL then that explains why you are seeing
EINTR failures.  If not, then we still have to ask whether EINTR really
needs to be handled differently from EINPROGRESS.

			regards, tom lane

Fri, 26 Oct 2001 21:43:32 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: [patch] helps fe-connect.c handle -EINTR more gracefully 