Re: Some 9.5beta2 backend processes not terminating properly?

Andres Freund <andres(at)anarazel(dot)de> writes:
> On 2016-01-03 10:03:41 +0530, Amit Kapila wrote:
>> 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.

That comment is a bit scary because it suggests that there might be some
cross-Windows-versions differences in the behavior of WSARecv. However,
I poked around on msdn.microsoft.com until I found a version of the
WSARecv man page that claimed to apply to Windows 2000, and it says the
same thing as the newer versions: b==0 implies graceful connection closure
(if stream protocol) or zero-size message (if message-oriented protocol)
and in neither case is it appropriate for this code to block.

So I think the exclusion of zero is an outright bug and always has been.

Actually, we could remove the test on b altogether and then simplify the
next line; I see no indication in Microsoft's docs that b<0 is a possible
case. That would make the code here more nearly match what is in the
retry loop --- and we now realize that it's only because we used to fall
through to the retry loop that the EOF case behaved sanely.

> 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.

Agreed. Let's do it and ship this puppy.

regards, tom lane