database/postgres
database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-05-02 11:44:50 +03:00
Code

Fix treatment of *lpNumberOfBytesRecvd == 0: that's a completion condition.

Browse Source 
pgwin32_recv() has treated a non-error return of zero bytes from WSARecv()
as being a reason to block ever since the current implementation was
introduced in commit a4c40f140d23cefb.  However, so far as one can tell
from Microsoft's documentation, that is just wrong: what it means is
graceful connection closure (in stream protocols) or receipt of a
zero-length message (in message protocols), and neither case should result
in blocking here.  The only reason the code worked at all was that control
then fell into the retry loop, which did *not* treat zero bytes specially,
so we'd get out after only wasting some cycles.  But as of 9.5 we do not
normally reach the retry loop and so the bug is exposed, as reported by
Shay Rojansky and diagnosed by Andres Freund.

Remove the unnecessary test on the byte count, and rearrange the code
in the retry loop so that it looks identical to the initial sequence.

Back-patch of commit 90e61df8130dc7051a108ada1219fb0680cb3eb6.  The
original plan was to apply this only to 9.5 and up, but after discussion
and buildfarm testing, it seems better to back-patch.  The noblock code
path has been at risk of this problem since it was introduced (in 9.0);
if it did happen in pre-9.5 branches, the symptom would be that a walsender
would wait indefinitely rather than noticing a loss of connection.  While
we lack proof that the case has been seen in the field, it seems possible
that it's happened without being reported.
This commit is contained in:
Tom Lane 2016-01-04 17:41:33 -05:00
parent aab4b73bdd
commit add6d821b7
1 changed files with 16 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
src/backend/port/win32/socket.c
View File

@ -323,12 +323,10 @@ pgwin32_recv(SOCKET s, char *buf, int len, int f)
wbuf.buf = buf; wbuf.buf = buf;
r = WSARecv(s, &wbuf, 1, &b, &flags, NULL, NULL); r = WSARecv(s, &wbuf, 1, &b, &flags, NULL, NULL);
if (r != SOCKET_ERROR && b > 0) if (r != SOCKET_ERROR)
/* Read succeeded right away */ return b; /* success */
return b;
if (r == SOCKET_ERROR && if (WSAGetLastError() != WSAEWOULDBLOCK)
WSAGetLastError() != WSAEWOULDBLOCK)
{ {
TranslateSocketError(); TranslateSocketError();
return -1; return -1;
@ -344,7 +342,7 @@ pgwin32_recv(SOCKET s, char *buf, int len, int f)
return -1; return -1;
} }
/* No error, zero bytes (win2000+) or error+WSAEWOULDBLOCK (<=nt4) */ /* We're in blocking mode, so wait for data */
for (n = 0; n < 5; n++) for (n = 0; n < 5; n++)
{ {
@ -353,25 +351,22 @@ pgwin32_recv(SOCKET s, char *buf, int len, int f)
return -1; /* errno already set */ return -1; /* errno already set */
r = WSARecv(s, &wbuf, 1, &b, &flags, NULL, NULL); r = WSARecv(s, &wbuf, 1, &b, &flags, NULL, NULL);
if (r == SOCKET_ERROR) if (r != SOCKET_ERROR)
return b; /* success */
if (WSAGetLastError() != WSAEWOULDBLOCK)
{ {
if (WSAGetLastError() == WSAEWOULDBLOCK)
{
/*
* There seem to be cases on win2k (at least) where WSARecv
* can return WSAEWOULDBLOCK even when
* pgwin32_waitforsinglesocket claims the socket is readable.
* In this case, just sleep for a moment and try again. We try
* up to 5 times - if it fails more than that it's not likely
* to ever come back.
*/
pg_usleep(10000);
continue;
}
TranslateSocketError(); TranslateSocketError();
return -1; return -1;
} }
return b;
/*
* There seem to be cases on win2k (at least) where WSARecv can return
* WSAEWOULDBLOCK even when pgwin32_waitforsinglesocket claims the
* socket is readable. In this case, just sleep for a moment and try
* again. We try up to 5 times - if it fails more than that it's not
* likely to ever come back.
*/
pg_usleep(10000);
} }
ereport(NOTICE, ereport(NOTICE,
(errmsg_internal("could not read from ready socket (after retries)"))); (errmsg_internal("could not read from ready socket (after retries)")));