On 2016-01-03 10:03:41 +0530, Amit Kapila wrote:
> On Sun, Jan 3, 2016 at 3:01 AM, Andres Freund <andres@anarazel.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
"emulatednon-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
