From c24fc3bc7309a22f6e07e6375abe584700df4552 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Wed, 15 May 2024 10:01:19 +1200 Subject: [PATCH v3] Fix FD_CLOSE socket event hangs on Windows. 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 | 43 +++++++++++++++++++++++++++++++++ src/include/storage/latch.h | 1 + 2 files changed, 44 insertions(+) diff --git a/src/backend/storage/ipc/latch.c b/src/backend/storage/ipc/latch.c index a7d88ebb048..cad360c1ada 100644 --- a/src/backend/storage/ipc/latch.c +++ b/src/backend/storage/ipc/latch.c @@ -1000,6 +1000,7 @@ AddWaitEventToSet(WaitEventSet *set, uint32 events, pgsocket fd, Latch *latch, event->user_data = user_data; #ifdef WIN32 event->reset = false; + event->peek = true; #endif if (events == WL_LATCH_SET) @@ -1999,6 +2000,48 @@ 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 a readable socket + * after setting up this handle to receive notifications, and before + * entering the sleep, on the first time we wait for this socket. + */ + if ((cur_event->events & WL_SOCKET_READABLE) != 0 && cur_event->peek) + { + char c; + WSABUF buf; + DWORD received; + DWORD flags; + + /* + * Don't do this next time, it's only needed once to smooth over + * the transition from one WaitEventSet's event handle to + * another's. + */ + cur_event->peek = false; + + /* + * Peek to see if EOF has been reached. Don't worry about errors, + * 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; + cur_event->reset = true; + return 1; + } + } + /* * Windows does not guarantee to log an FD_WRITE network event * indicating that more data can be sent unless the previous send() diff --git a/src/include/storage/latch.h b/src/include/storage/latch.h index 7e194d536f0..fb5cb2864db 100644 --- a/src/include/storage/latch.h +++ b/src/include/storage/latch.h @@ -157,6 +157,7 @@ typedef struct WaitEvent void *user_data; /* pointer provided in AddWaitEventToSet */ #ifdef WIN32 bool reset; /* Is reset of the event required? */ + bool peek; /* Is peek required? */ #endif } WaitEvent; -- 2.44.0