On Tue, May 14, 2024 at 9:00 PM Alexander Lakhin <exclus...@gmail.com> wrote:
> 14.05.2024 03:38, Thomas Munro wrote:
> > I was beginning to suspect that lingering odour myself. I haven't
> > look at the GSS code but I was imagining that what we have here is
> > perhaps not unsent data dropped on the floor due to linger policy
> > (unclean socket close on process exist), but rather that the server
> > didn't see the socket as ready to read because it lost track of the
> > FD_CLOSE somewhere because the client closed it gracefully, and our
> > server-side FD_CLOSE handling has always been a bit suspect. I wonder
> > if the GSS code is somehow more prone to brokenness. One thing we
> > learned in earlier problems was that abortive/error disconnections
> > generate FD_CLOSE repeatedly, while graceful ones give you only one.
> > In other words, if the other end politely calls closesocket(), the
> > server had better not miss the FD_CLOSE event, because it won't come
> > again. That's what
> >
> > https://commitfest.postgresql.org/46/3523/
> >
> > is intended to fix. Does it help here? Unfortunately that's
> > unpleasantly complicated and unbackpatchable (keeping a side-table of
> > socket FDs and event handles, so we don't lose events between the
> > cracks).
>
> Yes, that cure helps here too. I've tested it on b282fa88d~1 (the last
> state when that patch set can be applied).
Thanks for checking, and generally for your infinite patience with all
these horrible Windows problems.
OK, so we know what the problem is here. Here is the simplest
solution I know of for that problem. I have proposed this in the past
and received negative feedback because it's a really gross hack. But
I don't personally know what else to do about the back-branches (or
even if that complex solution is the right way forward for master).
The attached kludge at least has the [de]merit of being a mirror image
of the kludge that follows it for the "opposite" event. Does this fix
it?
From cbe4680c3e561b26c0bb49fc39dc8a6f40e84134 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.mu...@gmail.com>
Date: Wed, 15 May 2024 10:01:19 +1200
Subject: [PATCH] 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 event WaitLatchOrSocket()
constructs and destroys a new event handle, we can miss the FD_CLOSE
event that is signaled just as we're destroying the handle. The next
WaitLatchOrSocket() will not see it.
Fix that race with some extra polling.
We wouldn't need this if we had long-lived event handles for sockets,
which has been proposed and tested and shown to work, but it's far too
complex to back-patch.
---
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..9a5273f17d6 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;
}
+ /*
+ * Because we associate the socket with different event handles at
+ * different times, and because FD_CLOSE is only generated once the
+ * other end closes gracefully, we might miss an FD_CLOSE event that
+ * was signaled already on a handle we've already closed. We close
+ * that race by synchronously polling for EOF, after adjusting the
+ * event above and before sleeping below.
+ *
+ * XXX If we arranged to have 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;
+ int r;
+
+ buf.buf = &c;
+ buf.len = 1;
+
+ /*
+ * Peek to see if EOF condition is true. Don't worry about error
+ * handling or pending data, just be careful not to consume it.
+ */
+ 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
