From 29692bdbb1dffed0eda38ad8268655425a7dcdf5 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 3 Jan 2016 13:56:29 -0500 Subject: [PATCH] Fix treatment of *lpNumberOfBytesRecvd == 0: that's a completion condition. 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 to 9.5. The code is wrong all the way back, AFAICS, but since it's relatively harmless in earlier branches we'll leave it alone. --- src/backend/port/win32/socket.c | 37 ++++++++++++++------------------- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/src/backend/port/win32/socket.c b/src/backend/port/win32/socket.c index 957a421b44..cfbfa2d6e9 100644 --- a/src/backend/port/win32/socket.c +++ b/src/backend/port/win32/socket.c @@ -323,12 +323,10 @@ pgwin32_recv(SOCKET s, char *buf, int len, int f) wbuf.buf = buf; r = WSARecv(s, &wbuf, 1, &b, &flags, NULL, NULL); - if (r != SOCKET_ERROR && b > 0) - /* Read succeeded right away */ - return b; + if (r != SOCKET_ERROR) + return b; /* success */ - if (r == SOCKET_ERROR && - WSAGetLastError() != WSAEWOULDBLOCK) + if (WSAGetLastError() != WSAEWOULDBLOCK) { TranslateSocketError(); return -1; @@ -344,7 +342,7 @@ pgwin32_recv(SOCKET s, char *buf, int len, int f) 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++) { @@ -353,25 +351,22 @@ pgwin32_recv(SOCKET s, char *buf, int len, int f) return -1; /* errno already set */ 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(); 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, (errmsg_internal("could not read from ready socket (after retries)")));