Re: Some 9.5beta2 backend processes not terminating properly?

On 2016-01-03 10:03:41 +0530, Amit Kapila wrote:
> On Sun, Jan 3, 2016 at 3:01 AM, Andres Freund <andres(at)anarazel(dot)de> wrote:
> > Indeed it does use shutdown(). If I read the npgsql code that'll even be
> > done in the exception handling path. So fixing the 0 byte case might
> > already do the trick.
> >
>
> I think this true for a TCP socket, but this code-path is used for UDP
> (SOCK_DGRAM) sockets as well and there is a comment below in
> that function which seems to be indicating why originally 0 byte case
> has not been handled at the place suggested by you (now it seems to
> be much less relevant).

I'm not sure what the origin of that comment is, it's been there all the
way since a4c40f14. But it doesn't really have much real effect: If
WSARecv in the retry loop returns 0 bytes, we'll not retry again as r !=
SOCKET_ERROR but actually return 0.

Note that the whole retry loop in pgwin32_recv(), which kinda mitigates
the problem explained above, isn't entered anymore as the FE/BE socket
is now always operated in non-blocking mode. I.e.
if (pgwin32_noblock)
{
/*
* No data received, and we are in "emulated non-blocking mode", so
* return indicating that we'd block if we were to continue.
*/
errno = EWOULDBLOCK;
return -1;
}
will always be taken. Which exactly explains the problem, together with
the edge-triggered behaviour of
WaitForMultipleObjects()/WSAEventSelect().

I really think we have a host of buggy code around the event handling -
but most of it has been used for a long while. So I think fixing the 0
byte case for 9.5 is good enough.

Regards,

Andres