From 46cf55acc8a9f6b59fbac845339e87b8a9501956 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Wed, 15 May 2024 10:01:19 +1200 Subject: [PATCH v2] Add kludge to make FD_CLOSE level-triggered. Winsock only signals an FD_CLOSE event once if the other end of the socket shuts down gracefully. Because each WaitLatchOrSocket() call constructs and destroys a new event handle every time, we can miss the FD_CLOSE notification that is signaled after we've stopped waiting. The next WaitLatchOrSocket()'s event handle will never see it. Fix that race with some extra polling. It's not a beautiful code change, but it seems to work, and is in the same spirit as the kludge installed by commit f7819baa6. We wouldn't need this if we had exactly one long-lived event handle per socket, which has been proposed and tested and shown to work, but that's too complex to back-patch. It's plausible that commits 6051857fc and ed52c3707, later reverted by 29992a6a5, could be re-instated, with this fix in place. Back-patch to all supported releases. This should hopefully clear up build farm animal hamerkop's recent failures. Reported-by: Tom Lane Tested-by: Alexander Lakhin Discussion: https://postgr.es/m/176008.1715492071%40sss.pgh.pa.us --- src/backend/storage/ipc/latch.c | 37 +++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/backend/storage/ipc/latch.c b/src/backend/storage/ipc/latch.c index a7d88ebb048..4e28410e4c8 100644 --- a/src/backend/storage/ipc/latch.c +++ b/src/backend/storage/ipc/latch.c @@ -1999,6 +1999,43 @@ WaitEventSetWaitBlock(WaitEventSet *set, int cur_timeout, cur_event->reset = false; } + /* + * We associate the socket with a new event handle for each + * WaitEventSet. FD_CLOSE is only generated once if the other end + * closes gracefully. Therefore we might miss the FD_CLOSE + * notification, if it was delivered to another event after we stopped + * waiting for it. Close that race by polling for EOF after setting + * up this handle to receive notifications, and before entering the + * sleep. + * + * XXX If we had one event handle for the lifetime of a socket, we + * wouldn't need this. + */ + if (cur_event->events & WL_SOCKET_READABLE) + { + char c; + WSABUF buf; + DWORD received; + DWORD flags; + + /* + * Peek to see if EOF has been reached. Don't worry about error + * handling or pending data here, as those will cause a wakeup + * below and be discovered by a later non-peek recv() call. + */ + buf.buf = &c; + buf.len = 1; + flags = MSG_PEEK; + if (WSARecv(cur_event->fd, &buf, 1, &received, &flags, NULL, NULL) == 0) + { + occurred_events->pos = cur_event->pos; + occurred_events->user_data = cur_event->user_data; + occurred_events->events = WL_SOCKET_READABLE; + occurred_events->fd = cur_event->fd; + return 1; + } + } + /* * Windows does not guarantee to log an FD_WRITE network event * indicating that more data can be sent unless the previous send() -- 2.44.0