Re: Some 9.5beta2 backend processes not terminating properly? - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Some 9.5beta2 backend processes not terminating properly?
Date
Msg-id 25775.1451841800@sss.pgh.pa.us
Whole thread Raw
In response to Re: Some 9.5beta2 backend processes not terminating properly?  (Andres Freund <andres@anarazel.de>)
Responses Re: Some 9.5beta2 backend processes not terminating properly?  (Andres Freund <andres@anarazel.de>)
List pgsql-hackers
Andres Freund <andres@anarazel.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



pgsql-hackers by date:

Previous
From: Simon Riggs
Date:
Subject: Re: Avoiding pin scan during btree vacuum
Next
From: Tom Lane
Date:
Subject: Re: dynloader.h missing in prebuilt package for Windows?