Subject: Re: Why is src/test/modules/committs/t/002_standby.pl flaky?

From: Thomas Munro <[email-masked]>
Date: Tue, 22 Mar 2022 16:24:23 +1300

On Tue, Mar 22, 2022 at 4:13 PM Tom Lane <[email-masked]> wrote:
> Thomas Munro <[email-masked]> writes:
> > I have a new socket abstraction patch that should address the known
> > Windows socket/event bugs, but it's a little bigger than I thought it
> > would be, not quite ready, and now too late to expect people to review
> > for 15, so I think it should go into the next cycle.  I've bounced
> > https://commitfest.postgresql.org/37/3523/ into the next CF.  We'll
> > need to do something like 75674c7e for master.
>
> OK.  You want me to push 75674c7e to HEAD?

Thanks, yes, please do.

---

From: Tom Lane <[email-masked]>
Date: Tue, 22 Mar 2022 10:19:43 -0400

> Thanks, yes, please do.

Done.

			regards, tom lane

---

From: Thomas Munro <[email-masked]>
Date: Fri, 10 Nov 2023 16:31:27 +1300

Here is a new attempt to fix this mess.  Disclaimer: this based
entirely on reading the manual and vicariously hacking a computer I
don't have via CI.

The two basic ideas are:

 * keep per-socket event handles in a hash table
 * add our own level-triggered event memory

The socket table entries are reference counted, and exist as long as
the socket is currently in at least one WaitEventSet.  When creating a
new entry, extra polling logic re-checks the initial level-triggered
state (an overhead that we had in an ad-hoc way already, and that can
be avoided by more widespread use of long lived WaitEventSet).  You
are not allowed to close a socket while it's in a WaitEventSet,
because then a new socket could be allocated with the same number and
chaos would ensue.  For example, if we revive the idea of hooking
libpq connections up to long-lived WaitEventSets, we'll probably need
to invent a libpq event callback that says 'I am going to close socket
X!', so you have a chance to remove the socket from any WaitEventSet
*before* it's closed, to maintain that invariant.  Other lazier ideas
are possible, but probably become impossible in a hypothetical
multi-threaded future.

With these changes, AFAIK it should be safe to reinstate graceful
socket shutdowns, to fix the field complaints about FATAL error
messages being eaten by a grue and the annoying random CI/BF failures.

Here are some other ideas that I considered but rejected for now:

1.  We could throw the WAIT_USE_WIN32 code away, and hack
WAIT_USE_POLL to use WSAPoll() on Windows; we could create a
'self-pipe' using a pair of connected AF_UNIX sockets to implement
latches and fake signals.  It seems like a lot of work, and makes
latches a bit worse (instead of "everything is an event!" we have
"everything is a socket!" with a helper thread, and we don't even have
socketpair() on this OS).  Blah.

2.  We could figure out how to do fancy asynchronous sockets and IOCP.
That's how NT really wants to talk to the world, it doesn't really
want to pretend to be Unix.  I expect that is where we'll get to
eventually but it's a much bigger cross-platform R&D job.

3.  Maybe there is a kind of partial step towards idea 2 that Andres
mentioned on another thread somewhere: one could use an IOCP, and then
use event callbacks that run on system threads to post IOCP messages
(a bit like we do for our fake waitpid()).

What I have here is the simplest way I could see to patch up what we
already have, with the idea that in the fullness of time we'll
eventually get around to idea 2, once someone is ready to do the
press-ups.

Review/poking-with-a-stick/trying-to-break-it most welcome.
From 4614df55bd0e98b70b942543482e6a9eb767f718 Mon Sep 17 00:00:00 2001
From: Thomas Munro <[email-masked]>
Date: Wed, 1 Nov 2023 05:53:12 +1300
Subject: [PATCH v2 1/6] simplehash: Allow raw memory to be freed.

Commit 48995040d5e introduced SH_RAW_ALLOCATOR, but assumed that memory
allocated that way could be freed with pfree().  Allow SH_RAW_FREE to be
defined too, for cases where that isn't true.
---
 src/include/lib/simplehash.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/include/lib/simplehash.h b/src/include/lib/simplehash.h
index b7adc16b80..cd354e2f11 100644
--- a/src/include/lib/simplehash.h
+++ b/src/include/lib/simplehash.h
@@ -42,6 +42,7 @@
  *		declarations reside
  *	  - SH_RAW_ALLOCATOR - if defined, memory contexts are not used; instead,
  *	    use this to allocate bytes. The allocator must zero the returned space.
+ *	  - SH_RAW_FREE - free operation corresponding to SH_RAW_ALLOCATOR
  *	  - SH_USE_NONDEFAULT_ALLOCATOR - if defined no element allocator functions
  *		are defined, so you can supply your own
  *	  The following parameters are only relevant when SH_DEFINE is defined:
@@ -410,7 +411,11 @@ SH_ALLOCATE(SH_TYPE * type, Size size)
 static inline void
 SH_FREE(SH_TYPE * type, void *pointer)
 {
+#ifdef SH_RAW_FREE
+	SH_RAW_FREE(pointer);
+#else
 	pfree(pointer);
+#endif
 }
 
 #endif
@@ -458,7 +463,11 @@ SH_SCOPE void
 SH_DESTROY(SH_TYPE * tb)
 {
 	SH_FREE(tb, tb->data);
+#ifdef SH_RAW_FREE
+	SH_RAW_FREE(tb);
+#else
 	pfree(tb);
+#endif
 }
 
 /* reset the contents of a previously created hash table */
-- 
2.42.0

From 3ee479b182ac53e4839da1156ef30cd9f2523de7 Mon Sep 17 00:00:00 2001
From: Thomas Munro <[email-masked]>
Date: Wed, 1 Nov 2023 06:51:38 +1300
Subject: [PATCH v2 2/6] simplehash: Allow raw allocation to fail.

Commit 48995040d5e allowed for raw allocators to be used instead of the
MemoryContext API, but didn't contemplate allocation failure.  Teach the
grow and insert operations to report failure to the caller.
---
 src/include/lib/simplehash.h | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/src/include/lib/simplehash.h b/src/include/lib/simplehash.h
index cd354e2f11..d1034baf67 100644
--- a/src/include/lib/simplehash.h
+++ b/src/include/lib/simplehash.h
@@ -205,7 +205,7 @@ SH_SCOPE void SH_DESTROY(SH_TYPE * tb);
 SH_SCOPE void SH_RESET(SH_TYPE * tb);
 
 /* void <prefix>_grow(<prefix>_hash *tb, uint64 newsize) */
-SH_SCOPE void SH_GROW(SH_TYPE * tb, uint64 newsize);
+SH_SCOPE bool SH_GROW(SH_TYPE * tb, uint64 newsize);
 
 /* <element> *<prefix>_insert(<prefix>_hash *tb, <key> key, bool *found) */
 SH_SCOPE	SH_ELEMENT_TYPE *SH_INSERT(SH_TYPE * tb, SH_KEY_TYPE key, bool *found);
@@ -442,6 +442,8 @@ SH_CREATE(MemoryContext ctx, uint32 nelements, void *private_data)
 
 #ifdef SH_RAW_ALLOCATOR
 	tb = (SH_TYPE *) SH_RAW_ALLOCATOR(sizeof(SH_TYPE));
+	if (!tb)
+		return NULL;
 #else
 	tb = (SH_TYPE *) MemoryContextAllocZero(ctx, sizeof(SH_TYPE));
 	tb->ctx = ctx;
@@ -454,6 +456,17 @@ SH_CREATE(MemoryContext ctx, uint32 nelements, void *private_data)
 	SH_COMPUTE_PARAMETERS(tb, size);
 
 	tb->data = (SH_ELEMENT_TYPE *) SH_ALLOCATE(tb, sizeof(SH_ELEMENT_TYPE) * tb->size);
+#ifdef SH_RAW_ALLOCATOR
+	if (!tb->data)
+	{
+#ifdef SH_RAW_FREE
+		SH_RAW_FREE(tb);
+#else
+		pfree(tb);
+#endif
+		return NULL;
+	}
+#endif
 
 	return tb;
 }
@@ -485,7 +498,7 @@ SH_RESET(SH_TYPE * tb)
  * necessary. But resizing to the exact input size can be advantageous
  * performance-wise, when known at some point.
  */
-SH_SCOPE void
+SH_SCOPE bool
 SH_GROW(SH_TYPE * tb, uint64 newsize)
 {
 	uint64		oldsize = tb->size;
@@ -502,9 +515,13 @@ SH_GROW(SH_TYPE * tb, uint64 newsize)
 	/* compute parameters for new table */
 	SH_COMPUTE_PARAMETERS(tb, newsize);
 
-	tb->data = (SH_ELEMENT_TYPE *) SH_ALLOCATE(tb, sizeof(SH_ELEMENT_TYPE) * tb->size);
+	newdata = (SH_ELEMENT_TYPE *) SH_ALLOCATE(tb, sizeof(SH_ELEMENT_TYPE) * tb->size);
+#ifdef SH_RAW_ALLOCATOR
+	if (!newdata)
+		return false;
+#endif
 
-	newdata = tb->data;
+	tb->data = newdata;
 
 	/*
 	 * Copy entries from the old data to newdata. We theoretically could use
@@ -589,6 +606,8 @@ SH_GROW(SH_TYPE * tb, uint64 newsize)
 	}
 
 	SH_FREE(tb, olddata);
+
+	return true;
 }
 
 /*
@@ -623,7 +642,11 @@ restart:
 		 * When optimizing, it can be very useful to print these out.
 		 */
 		/* SH_STAT(tb); */
-		SH_GROW(tb, tb->size * 2);
+		if (!SH_GROW(tb, tb->size * 2))
+		{
+			*found = false;
+			return NULL;
+		}
 		/* SH_STAT(tb); */
 	}
 
-- 
2.42.0

From 03a5bd46e5cdf1e551a64b16bb8915aead67ad85 Mon Sep 17 00:00:00 2001
From: Thomas Munro <[email-masked]>
Date: Sun, 19 Mar 2023 16:07:20 +1300
Subject: [PATCH v2 3/6] Redesign Windows socket event management.

Previously, we created a Winsock event handle for each socket in each
WaitEventSet, and then we translated an FD_CLOSE event directly to
WL_SOCKET_READABLE.  Since FD_CLOSE is reported only once when the
remote end shuts down gracefully, we could hang in rare scenarios where
backend code relies on WL_SOCKET_READABLE being level-triggered.

We got away with this in the past when the thing on the other end of the
socket was another PostgreSQL server (ie via postgres_fdw, replication
etc), because the remote server would exit without shutting down or
closing its socket, and that produces a repeating 'abortive' FD_CLOSE.
We'd like to change that as it also eats error messages, producing user
complaints and random CI failures, but that's a sepaarate issue and
we'll need to fix this first.

New design:

* for each socket, we now create just one event handle to be used by
  all WaitEventSet objects that are interested in the socket

* for each socket, we now track a set of sticky events that are reported
  as poll() would until they are cleared by either the send()/recv()
  wrappers, or failing that by an explicit re-check

The lifetime management of event handles and associated state is done
by reference counting.
---
 src/backend/port/win32/socket.c  | 364 +++++++++++++++++++++++++++++++
 src/backend/storage/ipc/latch.c  | 212 ++++++------------
 src/include/port/win32_port.h    |   6 +
 src/include/storage/latch.h      |   3 -
 src/tools/pgindent/typedefs.list |   1 +
 5 files changed, 441 insertions(+), 145 deletions(-)

diff --git a/src/backend/port/win32/socket.c b/src/backend/port/win32/socket.c
index 9c339397d1..a7fa98cb1d 100644
--- a/src/backend/port/win32/socket.c
+++ b/src/backend/port/win32/socket.c
@@ -13,6 +13,8 @@
 
 #include "postgres.h"
 
+#include "common/hashfn.h"
+
 /*
  * Indicate if pgwin32_recv() and pgwin32_send() should operate
  * in non-blocking mode.
@@ -37,6 +39,77 @@ int			pgwin32_noblock = 0;
 #undef recv
 #undef send
 
+/*
+ * An entry in our socket table.
+ */
+typedef struct SocketTableEntry
+{
+	SOCKET		sock;
+	char		status;
+
+	/*
+	 * The reference count for the event handle.  Client code that wants to
+	 * use the event functions must acquire a reference and release it when
+	 * finished.
+	 */
+	int			reference_count;
+
+	/*
+	 * The FD_XXX events that were most recently selected for this socket
+	 * number with WSAEventSelect().
+	 */
+	int			selected_events;
+
+	/*
+	 * The FD_XXX events already reported by Winsock, that we'll continue to
+	 * report as long as they are true.  They are cleared by our send/recv
+	 * wrappers, because those are 're-enabling' functions that will cause
+	 * Winsock to report them again.  The are also cleared by an explicit
+	 * check we perform for the benefit of hypothetical code that might be
+	 * reach Winsock send/recv wrappers without going via our wrappers.
+	 */
+	int			level_triggered_events;
+
+	/*
+	 * Windows kernel event most recently associated with the socket number.
+	 */
+	HANDLE		event_handle;
+} SocketTableEntry;
+
+static inline void *
+malloc0(size_t size)
+{
+	void	   *result;
+
+	result = malloc(size);
+	if (result)
+		memset(result, 0, size);
+
+	return result;
+}
+
+/*
+ * It almost seems feasible to use an array to store our per-socket state,
+ * based on the observation that Windows socket descriptors seem to be small
+ * integers as on Unix, but the manual warns against making that assumption.
+ * So we use a hash table.
+ */
+
+#define SH_PREFIX socket_table
+#define SH_ELEMENT_TYPE SocketTableEntry
+#define SH_RAW_ALLOCATOR malloc0
+#define SH_RAW_FREE free
+#define SH_SCOPE static inline
+#define SH_KEY_TYPE SOCKET
+#define SH_KEY sock
+#define SH_HASH_KEY(tb, key) murmurhash32(key)
+#define SH_EQUAL(tb, a, b) (a) == (b)
+#define SH_DECLARE
+#define SH_DEFINE
+#include "lib/simplehash.h"
+
+static socket_table_hash * socket_table;
+
 /*
  * Blocking socket functions implemented so they listen on both
  * the socket and the signal event, required for signal handling.
@@ -310,6 +383,265 @@ pgwin32_socket(int af, int type, int protocol)
 	return s;
 }
 
+/*
+ * Check if any of FD_READ, FD_WRITE or FD_CLOSE is still true.  Used to
+ * re-check level-triggered events.
+ */
+static int
+pgwin32_socket_poll(SOCKET s, int events)
+{
+	int			revents = 0;
+
+	if (events & (FD_READ | FD_CLOSE))
+	{
+		ssize_t		rc;
+		char		c;
+
+		rc = recv(s, &c, 1, MSG_PEEK);
+		if (rc == 1)
+		{
+			/* At least one byte to read. */
+			if (events & FD_READ)
+				revents |= FD_READ;
+		}
+		else if (rc == 0 || WSAGetLastError() != WSAEWOULDBLOCK)
+		{
+			/* EOF due to graceful shutdown, or error. */
+			if (events & FD_CLOSE)
+				revents |= FD_CLOSE;
+		}
+	}
+
+	if (events & FD_WRITE)
+	{
+		char		c;
+
+		/* If it looks like we could write or get an error, report that. */
+		if (send(s, &c, 0, 0) == 0 || WSAGetLastError() != WSAEWOULDBLOCK)
+			revents |= FD_WRITE;
+	}
+
+	return revents;
+}
+
+/*
+ * Adjust the set of FD_XXX events this socket's event handle should wake up
+ * for.  Returns 0 on success, otherwise -1 and sets errno.
+ */
+int
+pgwin32_socket_select_events(SOCKET s, int selected_events)
+{
+	SocketTableEntry *entry;
+
+	Assert(socket_table);
+	entry = socket_table_lookup(socket_table, s);
+
+	Assert(entry);
+	Assert(entry->reference_count > 0);
+	Assert(entry->event_handle != WSA_INVALID_EVENT);
+
+	/* Do nothing if no change. */
+	if (selected_events == entry->selected_events)
+		return 0;
+
+	/*
+	 * Tell Winsock to link the socket to the event handle, and which events
+	 * we're interested in.
+	 */
+	if (WSAEventSelect(s, entry->event_handle, selected_events) == SOCKET_ERROR)
+	{
+		TranslateSocketError();
+		return -1;
+	}
+
+	entry->selected_events = selected_events;
+
+	/*
+	 * The manual tells us: "Issuing a WSAEventSelect for a socket cancels any
+	 * previous WSAAsyncSelect or WSAEventSelect for the same socket and
+	 * clears the internal network event record."  If that is true, we might
+	 * have wiped an internal flag we're interested in.  Close that race by
+	 * triggering an explicit poll before we sleep, by pretending we have seen
+	 * all of these events.
+	 */
+	if (selected_events & (FD_READ | FD_WRITE))
+		entry->level_triggered_events = selected_events & (FD_READ | FD_WRITE | FD_CLOSE);
+	else
+		entry->level_triggered_events = 0;
+
+	return 0;
+}
+
+/*
+ * Before waiting on the event handle, check if we have pending
+ * level-triggered events that are still true, and if so take measures to
+ * prevent the sleep.
+ */
+void
+pgwin32_socket_prepare_to_wait(SOCKET s)
+{
+	SocketTableEntry *entry;
+
+	Assert(socket_table);
+	entry = socket_table_lookup(socket_table, s);
+
+	Assert(entry);
+	Assert(entry->reference_count > 0);
+	Assert(entry->event_handle != WSA_INVALID_EVENT);
+
+	/*
+	 * If we're not waiting for FD_READ or FD_WRITE, don't try to poll the
+	 * socket.  Server sockets and client sockets that haven't connected yet
+	 * can't be polled by that technique.
+	 */
+	if ((entry->selected_events & (FD_READ | FD_WRITE)) &&
+		entry->level_triggered_events != 0)
+	{
+		/*
+		 * Re-check the level-triggered events we have recorded.  This is
+		 * necessary because someone might access WSASend()/WSARecv() directly
+		 * without going via our wrapper functions, so they might never be
+		 * cleared otherwise.
+		 */
+		entry->level_triggered_events =
+			pgwin32_socket_poll(s,
+								entry->level_triggered_events & entry->selected_events);
+		if (entry->level_triggered_events)
+		{
+			/*
+			 * At least one readiness condition is still true.  Prevent
+			 * sleeping, and let pgwin32_socket_enumerate_events() report
+			 * these level-triggered events.
+			 */
+			WSASetEvent(entry->event_handle);
+		}
+	}
+}
+
+/*
+ * After the Windows event handle has been signaled, this function can be
+ * called to find out which socket events occurred, and atomically reset the
+ * event handle for the next sleep.
+ *
+ * The events returned are also remembered in our level-triggered event mask,
+ * so they'll prevent sleeping and be reported again as long as they remain
+ * true.
+ */
+int
+pgwin32_socket_enumerate_events(SOCKET s)
+{
+	WSANETWORKEVENTS new_events = {0};
+	SocketTableEntry *entry;
+	int			result;
+
+	Assert(socket_table);
+	entry = socket_table_lookup(socket_table, s);
+
+	Assert(entry);
+	Assert(entry->reference_count > 0);
+	Assert(entry->event_handle != WSA_INVALID_EVENT);
+
+	/*
+	 * Atomically consume the internal network event record and reset the
+	 * associated event handle.  This guarantees that we can't miss future
+	 * wakeups.
+	 */
+	if (WSAEnumNetworkEvents(s, entry->event_handle, &new_events) != 0)
+	{
+		TranslateSocketError();
+		return -1;
+	}
+
+	/* Add any events pgwin32_socket_prepare_to_wait() decided to feed us. */
+	result = entry->level_triggered_events | new_events.lNetworkEvents;
+
+	/* Remember certain events for next time around. */
+	if (entry->selected_events & (FD_READ | FD_WRITE))
+		entry->level_triggered_events = result & (FD_READ | FD_WRITE | FD_CLOSE);
+	else
+		entry->level_triggered_events = 0;
+
+	return result;
+}
+
+/*
+ * Acquire a reference-counted Windows event handle for this socket.  This can
+ * be used for waiting for socket events.  Returns NULL and sets errno on
+ * failure.
+ */
+HANDLE
+pgwin32_socket_acquire_event_handle(SOCKET s)
+{
+	SocketTableEntry *entry;
+	bool		found;
+
+	/* First-time initialization. */
+	if (unlikely(socket_table == NULL))
+	{
+		socket_table = socket_table_create(16, NULL);
+		if (socket_table == NULL)
+		{
+			errno = ENOMEM;
+			return NULL;
+		}
+	}
+
+	/* If we already have it, just bump the count. */
+	entry = socket_table_insert(socket_table, s, &found);
+	if (likely(found))
+	{
+		Assert(entry->event_handle != WSA_INVALID_EVENT);
+		entry->reference_count++;
+		return entry->event_handle;
+	}
+
+	/* Did we run out of memory? */
+	if (entry == NULL)
+	{
+		errno = ENOMEM;
+		return NULL;
+	}
+
+	/* Allocate a new event handle. */
+	entry->event_handle = WSACreateEvent();
+	if (entry->event_handle == WSA_INVALID_EVENT)
+	{
+		socket_table_delete_item(socket_table, entry);
+		errno = ENOMEM;
+		return NULL;
+	}
+
+	entry->selected_events = 0;
+	entry->level_triggered_events = 0;
+	entry->reference_count = 1;
+
+	return entry->event_handle;
+}
+
+/*
+ * Release a reference-counted event handle.
+ */
+void
+pgwin32_socket_release_event_handle(SOCKET s)
+{
+	SocketTableEntry *entry;
+
+	Assert(socket_table);
+	entry = socket_table_lookup(socket_table, s);
+
+	Assert(entry);
+	Assert(entry->reference_count > 0);
+	Assert(entry->event_handle != WSA_INVALID_EVENT);
+
+	if (--entry->reference_count == 0)
+	{
+		WSACloseEvent(entry->event_handle);
+		socket_table_delete_item(socket_table, entry);
+
+		/* XXX Free socket_table if it is empty? */
+	}
+}
+
 int
 pgwin32_bind(SOCKET s, struct sockaddr *addr, int addrlen)
 {
@@ -402,6 +734,22 @@ pgwin32_recv(SOCKET s, char *buf, int len, int f)
 		return -1;
 	}
 
+	/*
+	 * WSARecv() is a re-enabling function for Winsock's FD_READ event, so it
+	 * is now safe to clear our level-triggered flag.  This is only an
+	 * optimization for a common case, and not required for correctness.  If
+	 * someone calls WSARecv() directly instead of going through this wrapper,
+	 * pgwin32_socket_prepare_to_wait() will figure that out and clear it
+	 * anyway.
+	 */
+	if (socket_table)
+	{
+		SocketTableEntry *entry = socket_table_lookup(socket_table, s);
+
+		if (entry)
+			entry->level_triggered_events &= ~FD_READ;
+	}
+
 	if (pgwin32_noblock)
 	{
 		/*
@@ -485,6 +833,22 @@ pgwin32_send(SOCKET s, const void *buf, int len, int flags)
 			return -1;
 		}
 
+		/*
+		 * WSASend() is a re-enabling function for Winsock's FD_WRITE event,
+		 * so it is now safe to clear our level-triggered flag.  This is only
+		 * an optimization for a common case, and not required for
+		 * correctness.  If someone calls WSASend() directly instead of going
+		 * through this wrapper, pgwin32_socket_prepare_to_wait() will figure
+		 * that out and clear it anyway.
+		 */
+		if (socket_table)
+		{
+			SocketTableEntry *entry = socket_table_lookup(socket_table, s);
+
+			if (entry)
+				entry->level_triggered_events &= ~FD_WRITE;
+		}
+
 		if (pgwin32_noblock)
 		{
 			/*
diff --git a/src/backend/storage/ipc/latch.c b/src/backend/storage/ipc/latch.c
index 2fd386a4ed..5bf03a3cd9 100644
--- a/src/backend/storage/ipc/latch.c
+++ b/src/backend/storage/ipc/latch.c
@@ -847,20 +847,9 @@ FreeWaitEventSet(WaitEventSet *set)
 		 cur_event < (set->events + set->nevents);
 		 cur_event++)
 	{
-		if (cur_event->events & WL_LATCH_SET)
-		{
-			/* uses the latch's HANDLE */
-		}
-		else if (cur_event->events & WL_POSTMASTER_DEATH)
-		{
-			/* uses PostmasterHandle */
-		}
-		else
-		{
-			/* Clean up the event object we created for the socket */
-			WSAEventSelect(cur_event->fd, NULL, 0);
-			WSACloseEvent(set->handles[cur_event->pos + 1]);
-		}
+		/* Release reference to socket's event handle. */
+		if (cur_event->events & WL_SOCKET_MASK)
+			pgwin32_socket_release_event_handle(cur_event->fd);
 	}
 #endif
 
@@ -955,9 +944,6 @@ AddWaitEventToSet(WaitEventSet *set, uint32 events, pgsocket fd, Latch *latch,
 	event->fd = fd;
 	event->events = events;
 	event->user_data = user_data;
-#ifdef WIN32
-	event->reset = false;
-#endif
 
 	if (events == WL_LATCH_SET)
 	{
@@ -976,10 +962,21 @@ AddWaitEventToSet(WaitEventSet *set, uint32 events, pgsocket fd, Latch *latch,
 	}
 	else if (events == WL_POSTMASTER_DEATH)
 	{
-#ifndef WIN32
+#if defined(WAIT_USE_WIN32)
+		set->handles[event->pos + 1] = PostmasterHandle;
+		event->fd = PGINVALID_SOCKET;
+#else
 		event->fd = postmaster_alive_fds[POSTMASTER_FD_WATCH];
 #endif
 	}
+	else if (events & WL_SOCKET_MASK)
+	{
+#if defined(WAIT_USE_WIN32)
+		set->handles[event->pos + 1] = pgwin32_socket_acquire_event_handle(fd);
+		if (!set->handles[event->pos + 1])
+			elog(ERROR, "could not acquire socket event handle: %m");
+#endif
+	}
 
 	/* perform wait primitive specific initialization, if needed */
 #if defined(WAIT_USE_EPOLL)
@@ -1322,45 +1319,52 @@ WaitEventAdjustKqueue(WaitEventSet *set, WaitEvent *event, int old_events)
 #endif
 
 #if defined(WAIT_USE_WIN32)
+static int
+ToWinsockEvents(int pg_events)
+{
+	int			winsock_events = 0;
+
+	if (pg_events & WL_SOCKET_READABLE)
+		winsock_events |= FD_CLOSE | FD_READ;
+	if (pg_events & WL_SOCKET_WRITEABLE)
+		winsock_events |= FD_CLOSE | FD_WRITE;
+	if (pg_events & WL_SOCKET_CONNECTED)
+		winsock_events |= FD_CLOSE | FD_CONNECT;
+	if (pg_events & WL_SOCKET_ACCEPT)
+		winsock_events |= FD_CLOSE | FD_ACCEPT;
+
+	return winsock_events;
+}
+
+static int
+FromWinsockEvents(int winsock_events)
+{
+	int			pg_events = 0;
+
+	if (winsock_events & (FD_CLOSE | FD_READ))
+		pg_events |= WL_SOCKET_READABLE;
+	if (winsock_events & (FD_CLOSE | FD_WRITE))
+		pg_events |= WL_SOCKET_WRITEABLE;
+	if (winsock_events & (FD_CLOSE | FD_CONNECT))
+		pg_events |= WL_SOCKET_CONNECTED;
+	if (winsock_events & (FD_CLOSE | FD_ACCEPT))
+		pg_events |= WL_SOCKET_ACCEPT;
+
+	return pg_events;
+}
+
 static void
 WaitEventAdjustWin32(WaitEventSet *set, WaitEvent *event)
 {
-	HANDLE	   *handle = &set->handles[event->pos + 1];
-
-	if (event->events == WL_LATCH_SET)
+	if (event->events & WL_LATCH_SET)
 	{
-		Assert(set->latch != NULL);
-		*handle = set->latch->event;
+		set->handles[event->pos + 1] = set->latch->event;
 	}
-	else if (event->events == WL_POSTMASTER_DEATH)
-	{
-		*handle = PostmasterHandle;
-	}
-	else
+	else if (event->events & WL_SOCKET_MASK)
 	{
-		int			flags = FD_CLOSE;	/* always check for errors/EOF */
-
-		if (event->events & WL_SOCKET_READABLE)
-			flags |= FD_READ;
-		if (event->events & WL_SOCKET_WRITEABLE)
-			flags |= FD_WRITE;
-		if (event->events & WL_SOCKET_CONNECTED)
-			flags |= FD_CONNECT;
-		if (event->events & WL_SOCKET_ACCEPT)
-			flags |= FD_ACCEPT;
-
-		if (*handle == WSA_INVALID_EVENT)
-		{
-			*handle = WSACreateEvent();
-			if (*handle == WSA_INVALID_EVENT)
-				elog(ERROR, "failed to create event for socket: error code %d",
-					 WSAGetLastError());
-		}
-		if (WSAEventSelect(event->fd, *handle, flags) != 0)
-			elog(ERROR, "failed to set up event for socket: error code %d",
-				 WSAGetLastError());
-
-		Assert(event->fd != PGINVALID_SOCKET);
+		if (pgwin32_socket_select_events(event->fd,
+										 ToWinsockEvents(event->events)) < 0)
+			elog(ERROR, "failed to set up event for socket: %m");
 	}
 }
 #endif
@@ -1945,48 +1949,16 @@ WaitEventSetWaitBlock(WaitEventSet *set, int cur_timeout,
 	DWORD		rc;
 	WaitEvent  *cur_event;
 
-	/* Reset any wait events that need it */
+	/*
+	 * Allow level-triggered events to be signaled, causing
+	 * WaitForMultipleObjects() to return immediately.
+	 */
 	for (cur_event = set->events;
 		 cur_event < (set->events + set->nevents);
 		 cur_event++)
 	{
-		if (cur_event->reset)
-		{
-			WaitEventAdjustWin32(set, cur_event);
-			cur_event->reset = false;
-		}
-
-		/*
-		 * Windows does not guarantee to log an FD_WRITE network event
-		 * indicating that more data can be sent unless the previous send()
-		 * failed with WSAEWOULDBLOCK.  While our caller might well have made
-		 * such a call, we cannot assume that here.  Therefore, if waiting for
-		 * write-ready, force the issue by doing a dummy send().  If the dummy
-		 * send() succeeds, assume that the socket is in fact write-ready, and
-		 * return immediately.  Also, if it fails with something other than
-		 * WSAEWOULDBLOCK, return a write-ready indication to let our caller
-		 * deal with the error condition.
-		 */
-		if (cur_event->events & WL_SOCKET_WRITEABLE)
-		{
-			char		c;
-			WSABUF		buf;
-			DWORD		sent;
-			int			r;
-
-			buf.buf = &c;
-			buf.len = 0;
-
-			r = WSASend(cur_event->fd, &buf, 1, &sent, 0, NULL, NULL);
-			if (r == 0 || WSAGetLastError() != WSAEWOULDBLOCK)
-			{
-				occurred_events->pos = cur_event->pos;
-				occurred_events->user_data = cur_event->user_data;
-				occurred_events->events = WL_SOCKET_WRITEABLE;
-				occurred_events->fd = cur_event->fd;
-				return 1;
-			}
-		}
+		if (cur_event->events & WL_SOCKET_MASK)
+			pgwin32_socket_prepare_to_wait(cur_event->fd);
 	}
 
 	/*
@@ -2067,64 +2039,20 @@ WaitEventSetWaitBlock(WaitEventSet *set, int cur_timeout,
 		}
 		else if (cur_event->events & WL_SOCKET_MASK)
 		{
-			WSANETWORKEVENTS resEvents;
-			HANDLE		handle = set->handles[cur_event->pos + 1];
+			int			winsock_events;
+			int			pg_events;
 
 			Assert(cur_event->fd);
 
-			occurred_events->fd = cur_event->fd;
+			winsock_events = pgwin32_socket_enumerate_events(cur_event->fd);
+			if (winsock_events < 0)
+				elog(ERROR, "could not enumerate socket events: %m");
 
-			ZeroMemory(&resEvents, sizeof(resEvents));
-			if (WSAEnumNetworkEvents(cur_event->fd, handle, &resEvents) != 0)
-				elog(ERROR, "failed to enumerate network events: error code %d",
-					 WSAGetLastError());
-			if ((cur_event->events & WL_SOCKET_READABLE) &&
-				(resEvents.lNetworkEvents & FD_READ))
-			{
-				/* data available in socket */
-				occurred_events->events |= WL_SOCKET_READABLE;
-
-				/*------
-				 * WaitForMultipleObjects doesn't guarantee that a read event
-				 * will be returned if the latch is set at the same time.  Even
-				 * if it did, the caller might drop that event expecting it to
-				 * reoccur on next call.  So, we must force the event to be
-				 * reset if this WaitEventSet is used again in order to avoid
-				 * an indefinite hang.
-				 *
-				 * Refer
-				 * https://msdn.microsoft.com/en-us/library/windows/desktop/ms741576(v=vs.85).aspx
-				 * for the behavior of socket events.
-				 *------
-				 */
-				cur_event->reset = true;
-			}
-			if ((cur_event->events & WL_SOCKET_WRITEABLE) &&
-				(resEvents.lNetworkEvents & FD_WRITE))
-			{
-				/* writeable */
-				occurred_events->events |= WL_SOCKET_WRITEABLE;
-			}
-			if ((cur_event->events & WL_SOCKET_CONNECTED) &&
-				(resEvents.lNetworkEvents & FD_CONNECT))
-			{
-				/* connected */
-				occurred_events->events |= WL_SOCKET_CONNECTED;
-			}
-			if ((cur_event->events & WL_SOCKET_ACCEPT) &&
-				(resEvents.lNetworkEvents & FD_ACCEPT))
-			{
-				/* incoming connection could be accepted */
-				occurred_events->events |= WL_SOCKET_ACCEPT;
-			}
-			if (resEvents.lNetworkEvents & FD_CLOSE)
-			{
-				/* EOF/error, so signal all caller-requested socket flags */
-				occurred_events->events |= (cur_event->events & WL_SOCKET_MASK);
-			}
-
-			if (occurred_events->events != 0)
+			pg_events = FromWinsockEvents(winsock_events) & cur_event->events;
+			if (pg_events)
 			{
+				occurred_events->fd = cur_event->fd;
+				occurred_events->events = pg_events;
 				occurred_events++;
 				returned_events++;
 			}
diff --git a/src/include/port/win32_port.h b/src/include/port/win32_port.h
index 27a11c7868..a0ed6aaeaa 100644
--- a/src/include/port/win32_port.h
+++ b/src/include/port/win32_port.h
@@ -506,6 +506,12 @@ extern int	pgwin32_recv(SOCKET s, char *buf, int len, int flags);
 extern int	pgwin32_send(SOCKET s, const void *buf, int len, int flags);
 extern int	pgwin32_waitforsinglesocket(SOCKET s, int what, int timeout);
 
+extern HANDLE pgwin32_socket_acquire_event_handle(SOCKET s);
+extern void pgwin32_socket_release_event_handle(SOCKET s);
+extern int	pgwin32_socket_select_events(SOCKET s, int events);
+extern void pgwin32_socket_prepare_to_wait(SOCKET s);
+extern int	pgwin32_socket_enumerate_events(SOCKET s);
+
 extern PGDLLIMPORT int pgwin32_noblock;
 
 #endif							/* FRONTEND */
diff --git a/src/include/storage/latch.h b/src/include/storage/latch.h
index 99cc47874a..cbcc5ef23f 100644
--- a/src/include/storage/latch.h
+++ b/src/include/storage/latch.h
@@ -153,9 +153,6 @@ typedef struct WaitEvent
 	uint32		events;			/* triggered events */
 	pgsocket	fd;				/* socket fd associated with event */
 	void	   *user_data;		/* pointer provided in AddWaitEventToSet */
-#ifdef WIN32
-	bool		reset;			/* Is reset of the event required? */
-#endif
 } WaitEvent;
 
 /* forward declaration to avoid exposing latch.c implementation details */
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index bf50a32119..15dd7fa2b8 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -2585,6 +2585,7 @@ Snapshot
 SnapshotData
 SnapshotType
 SockAddr
+SocketTableEntry
 Sort
 SortBy
 SortByDir
-- 
2.42.0

From c561d1f7091368c5e09735ab099eb4a656587dfb Mon Sep 17 00:00:00 2001
From: Thomas Munro <[email-masked]>
Date: Fri, 10 Nov 2023 08:41:28 +1300
Subject: [PATCH v2 4/6] Remove pgwin32_select().

pgwin32_select(), used to replace select() in backend code on Windows,
would need to be updated to use the new per-socket event handles.  Since
the last remaining user of select() in the backend is scheduled for
replacement with the WaitEventSet API, it seems better to demolish it
instead.

Any extension code that is relying on select() with fake signals will
still compile, but will no longer respond to signals.  Hypothetical code
like that is probably buggy anyway, because backend code should also be
handling interrupts, and should switch to the various WaitEventSet APIs.
---
 src/backend/port/win32/socket.c | 200 --------------------------------
 src/include/port/win32_port.h   |   2 -
 2 files changed, 202 deletions(-)

diff --git a/src/backend/port/win32/socket.c b/src/backend/port/win32/socket.c
index a7fa98cb1d..d0cf08392f 100644
--- a/src/backend/port/win32/socket.c
+++ b/src/backend/port/win32/socket.c
@@ -867,203 +867,3 @@ pgwin32_send(SOCKET s, const void *buf, int len, int flags)
 
 	return -1;
 }
-
-
-/*
- * Wait for activity on one or more sockets.
- * While waiting, allow signals to run
- *
- * NOTE! Currently does not implement exceptfds check,
- * since it is not used in postgresql!
- */
-int
-pgwin32_select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds, const struct timeval *timeout)
-{
-	WSAEVENT	events[FD_SETSIZE * 2]; /* worst case is readfds totally
-										 * different from writefds, so
-										 * 2*FD_SETSIZE sockets */
-	SOCKET		sockets[FD_SETSIZE * 2];
-	int			numevents = 0;
-	int			i;
-	int			r;
-	DWORD		timeoutval = WSA_INFINITE;
-	FD_SET		outreadfds;
-	FD_SET		outwritefds;
-	int			nummatches = 0;
-
-	Assert(exceptfds == NULL);
-
-	if (pgwin32_poll_signals())
-		return -1;
-
-	FD_ZERO(&outreadfds);
-	FD_ZERO(&outwritefds);
-
-	/*
-	 * Windows does not guarantee to log an FD_WRITE network event indicating
-	 * that more data can be sent unless the previous send() failed with
-	 * WSAEWOULDBLOCK.  While our caller might well have made such a call, we
-	 * cannot assume that here.  Therefore, if waiting for write-ready, force
-	 * the issue by doing a dummy send().  If the dummy send() succeeds,
-	 * assume that the socket is in fact write-ready, and return immediately.
-	 * Also, if it fails with something other than WSAEWOULDBLOCK, return a
-	 * write-ready indication to let our caller deal with the error condition.
-	 */
-	if (writefds != NULL)
-	{
-		for (i = 0; i < writefds->fd_count; i++)
-		{
-			char		c;
-			WSABUF		buf;
-			DWORD		sent;
-
-			buf.buf = &c;
-			buf.len = 0;
-
-			r = WSASend(writefds->fd_array[i], &buf, 1, &sent, 0, NULL, NULL);
-			if (r == 0 || WSAGetLastError() != WSAEWOULDBLOCK)
-				FD_SET(writefds->fd_array[i], &outwritefds);
-		}
-
-		/* If we found any write-ready sockets, just return them immediately */
-		if (outwritefds.fd_count > 0)
-		{
-			memcpy(writefds, &outwritefds, sizeof(fd_set));
-			if (readfds)
-				FD_ZERO(readfds);
-			return outwritefds.fd_count;
-		}
-	}
-
-
-	/* Now set up for an actual select */
-
-	if (timeout != NULL)
-	{
-		/* timeoutval is in milliseconds */
-		timeoutval = timeout->tv_sec * 1000 + timeout->tv_usec / 1000;
-	}
-
-	if (readfds != NULL)
-	{
-		for (i = 0; i < readfds->fd_count; i++)
-		{
-			events[numevents] = WSACreateEvent();
-			sockets[numevents] = readfds->fd_array[i];
-			numevents++;
-		}
-	}
-	if (writefds != NULL)
-	{
-		for (i = 0; i < writefds->fd_count; i++)
-		{
-			if (!readfds ||
-				!FD_ISSET(writefds->fd_array[i], readfds))
-			{
-				/* If the socket is not in the read list */
-				events[numevents] = WSACreateEvent();
-				sockets[numevents] = writefds->fd_array[i];
-				numevents++;
-			}
-		}
-	}
-
-	for (i = 0; i < numevents; i++)
-	{
-		int			flags = 0;
-
-		if (readfds && FD_ISSET(sockets[i], readfds))
-			flags |= FD_READ | FD_ACCEPT | FD_CLOSE;
-
-		if (writefds && FD_ISSET(sockets[i], writefds))
-			flags |= FD_WRITE | FD_CLOSE;
-
-		if (WSAEventSelect(sockets[i], events[i], flags) != 0)
-		{
-			TranslateSocketError();
-			/* release already-assigned event objects */
-			while (--i >= 0)
-				WSAEventSelect(sockets[i], NULL, 0);
-			for (i = 0; i < numevents; i++)
-				WSACloseEvent(events[i]);
-			return -1;
-		}
-	}
-
-	events[numevents] = pgwin32_signal_event;
-	r = WaitForMultipleObjectsEx(numevents + 1, events, FALSE, timeoutval, TRUE);
-	if (r != WAIT_TIMEOUT && r != WAIT_IO_COMPLETION && r != (WAIT_OBJECT_0 + numevents))
-	{
-		/*
-		 * We scan all events, even those not signaled, in case more than one
-		 * event has been tagged but Wait.. can only return one.
-		 */
-		WSANETWORKEVENTS resEvents;
-
-		for (i = 0; i < numevents; i++)
-		{
-			ZeroMemory(&resEvents, sizeof(resEvents));
-			if (WSAEnumNetworkEvents(sockets[i], events[i], &resEvents) != 0)
-				elog(ERROR, "failed to enumerate network events: error code %d",
-					 WSAGetLastError());
-			/* Read activity? */
-			if (readfds && FD_ISSET(sockets[i], readfds))
-			{
-				if ((resEvents.lNetworkEvents & FD_READ) ||
-					(resEvents.lNetworkEvents & FD_ACCEPT) ||
-					(resEvents.lNetworkEvents & FD_CLOSE))
-				{
-					FD_SET(sockets[i], &outreadfds);
-
-					nummatches++;
-				}
-			}
-			/* Write activity? */
-			if (writefds && FD_ISSET(sockets[i], writefds))
-			{
-				if ((resEvents.lNetworkEvents & FD_WRITE) ||
-					(resEvents.lNetworkEvents & FD_CLOSE))
-				{
-					FD_SET(sockets[i], &outwritefds);
-
-					nummatches++;
-				}
-			}
-		}
-	}
-
-	/* Clean up all the event objects */
-	for (i = 0; i < numevents; i++)
-	{
-		WSAEventSelect(sockets[i], NULL, 0);
-		WSACloseEvent(events[i]);
-	}
-
-	if (r == WSA_WAIT_TIMEOUT)
-	{
-		if (readfds)
-			FD_ZERO(readfds);
-		if (writefds)
-			FD_ZERO(writefds);
-		return 0;
-	}
-
-	/* Signal-like events. */
-	if (r == WAIT_OBJECT_0 + numevents || r == WAIT_IO_COMPLETION)
-	{
-		pgwin32_dispatch_queued_signals();
-		errno = EINTR;
-		if (readfds)
-			FD_ZERO(readfds);
-		if (writefds)
-			FD_ZERO(writefds);
-		return -1;
-	}
-
-	/* Overwrite socket sets with our resulting values */
-	if (readfds)
-		memcpy(readfds, &outreadfds, sizeof(fd_set));
-	if (writefds)
-		memcpy(writefds, &outwritefds, sizeof(fd_set));
-	return nummatches;
-}
diff --git a/src/include/port/win32_port.h b/src/include/port/win32_port.h
index a0ed6aaeaa..1b605d9403 100644
--- a/src/include/port/win32_port.h
+++ b/src/include/port/win32_port.h
@@ -492,7 +492,6 @@ extern int	pgkill(int pid, int sig);
 #define listen(s, backlog) pgwin32_listen(s, backlog)
 #define accept(s, addr, addrlen) pgwin32_accept(s, addr, addrlen)
 #define connect(s, name, namelen) pgwin32_connect(s, name, namelen)
-#define select(n, r, w, e, timeout) pgwin32_select(n, r, w, e, timeout)
 #define recv(s, buf, len, flags) pgwin32_recv(s, buf, len, flags)
 #define send(s, buf, len, flags) pgwin32_send(s, buf, len, flags)
 
@@ -501,7 +500,6 @@ extern int	pgwin32_bind(SOCKET s, struct sockaddr *addr, int addrlen);
 extern int	pgwin32_listen(SOCKET s, int backlog);
 extern SOCKET pgwin32_accept(SOCKET s, struct sockaddr *addr, int *addrlen);
 extern int	pgwin32_connect(SOCKET s, const struct sockaddr *name, int namelen);
-extern int	pgwin32_select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds, const struct timeval *timeout);
 extern int	pgwin32_recv(SOCKET s, char *buf, int len, int flags);
 extern int	pgwin32_send(SOCKET s, const void *buf, int len, int flags);
 extern int	pgwin32_waitforsinglesocket(SOCKET s, int what, int timeout);
-- 
2.42.0

From 27144fba3c17e3d15e973e250f3ce3151079aea0 Mon Sep 17 00:00:00 2001
From: Thomas Munro <[email-masked]>
Date: Fri, 10 Nov 2023 08:59:45 +1300
Subject: [PATCH v2 5/6] Refactor pgwin32_waitforsinglesocket() to share
 events.

This function is hardly used, since sockets in the backend are almost
always in non-blocking mode.  Ideally they would *always* be
non-blocking, and if we ever get to that situation by project policy we
could just delete this and related code, but in the meantime, we have to
adjust it to use the new per-socket event handle or it could lose
network events.

While here, delete the code paths for UDP which are probably dead code
since we retired the UDP-powered stats collector.  There is another user
of UDP in auth.c, but it's using sendto() and thus not reaching this
code.  Any other user of UDP that I might have missed probably isn't
generating 'high load' like the old stats system that presumably
motivated that sleeping logic.
---
 src/backend/port/win32/socket.c | 103 ++++++--------------------------
 1 file changed, 17 insertions(+), 86 deletions(-)

diff --git a/src/backend/port/win32/socket.c b/src/backend/port/win32/socket.c
index d0cf08392f..797a1f503e 100644
--- a/src/backend/port/win32/socket.c
+++ b/src/backend/port/win32/socket.c
@@ -238,120 +238,51 @@ pgwin32_poll_signals(void)
 	return 0;
 }
 
-static int
-isDataGram(SOCKET s)
-{
-	int			type;
-	int			typelen = sizeof(type);
-
-	if (getsockopt(s, SOL_SOCKET, SO_TYPE, (char *) &type, &typelen))
-		return 1;
-
-	return (type == SOCK_DGRAM) ? 1 : 0;
-}
-
 int
 pgwin32_waitforsinglesocket(SOCKET s, int what, int timeout)
 {
-	static HANDLE waitevent = INVALID_HANDLE_VALUE;
-	static SOCKET current_socket = INVALID_SOCKET;
-	static int	isUDP = 0;
 	HANDLE		events[2];
 	int			r;
 
-	/* Create an event object just once and use it on all future calls */
-	if (waitevent == INVALID_HANDLE_VALUE)
-	{
-		waitevent = CreateEvent(NULL, TRUE, FALSE, NULL);
-
-		if (waitevent == INVALID_HANDLE_VALUE)
-			ereport(ERROR,
-					(errmsg_internal("could not create socket waiting event: error code %lu", GetLastError())));
-	}
-	else if (!ResetEvent(waitevent))
-		ereport(ERROR,
-				(errmsg_internal("could not reset socket waiting event: error code %lu", GetLastError())));
-
-	/*
-	 * Track whether socket is UDP or not.  (NB: most likely, this is both
-	 * useless and wrong; there is no reason to think that the behavior of
-	 * WSAEventSelect is different for TCP and UDP.)
-	 */
-	if (current_socket != s)
-		isUDP = isDataGram(s);
-	current_socket = s;
+	events[0] = pgwin32_signal_event;
+	events[1] = pgwin32_socket_acquire_event_handle(s);
 
-	/*
-	 * Attach event to socket.  NOTE: we must detach it again before
-	 * returning, since other bits of code may try to attach other events to
-	 * the socket.
-	 */
-	if (WSAEventSelect(s, waitevent, what) != 0)
+	if (events[1] == NULL)
 	{
-		TranslateSocketError();
+		/* errno is set */
 		return 0;
 	}
 
-	events[0] = pgwin32_signal_event;
-	events[1] = waitevent;
-
-	/*
-	 * Just a workaround of unknown locking problem with writing in UDP socket
-	 * under high load: Client's pgsql backend sleeps infinitely in
-	 * WaitForMultipleObjectsEx, pgstat process sleeps in pgwin32_select().
-	 * So, we will wait with small timeout(0.1 sec) and if socket is still
-	 * blocked, try WSASend (see comments in pgwin32_select) and wait again.
-	 */
-	if ((what & FD_WRITE) && isUDP)
+	if (pgwin32_socket_select_events(s, what) < 0)
 	{
-		for (;;)
-		{
-			r = WaitForMultipleObjectsEx(2, events, FALSE, 100, TRUE);
-
-			if (r == WAIT_TIMEOUT)
-			{
-				char		c;
-				WSABUF		buf;
-				DWORD		sent;
-
-				buf.buf = &c;
-				buf.len = 0;
-
-				r = WSASend(s, &buf, 1, &sent, 0, NULL, NULL);
-				if (r == 0)		/* Completed - means things are fine! */
-				{
-					WSAEventSelect(s, NULL, 0);
-					return 1;
-				}
-				else if (WSAGetLastError() != WSAEWOULDBLOCK)
-				{
-					TranslateSocketError();
-					WSAEventSelect(s, NULL, 0);
-					return 0;
-				}
-			}
-			else
-				break;
-		}
+		pgwin32_socket_release_event_handle(s);
+		return 0;
 	}
-	else
-		r = WaitForMultipleObjectsEx(2, events, FALSE, timeout, TRUE);
 
-	WSAEventSelect(s, NULL, 0);
+	pgwin32_socket_prepare_to_wait(s);
+
+	r = WaitForMultipleObjectsEx(2, events, FALSE, timeout, TRUE);
 
 	if (r == WAIT_OBJECT_0 || r == WAIT_IO_COMPLETION)
 	{
+		pgwin32_socket_release_event_handle(s);
 		pgwin32_dispatch_queued_signals();
 		errno = EINTR;
 		return 0;
 	}
 	if (r == WAIT_OBJECT_0 + 1)
+	{
+		pgwin32_socket_enumerate_events(s);
+		pgwin32_socket_release_event_handle(s);
 		return 1;
+	}
 	if (r == WAIT_TIMEOUT)
 	{
+		pgwin32_socket_release_event_handle(s);
 		errno = EWOULDBLOCK;
 		return 0;
 	}
+	pgwin32_socket_release_event_handle(s);
 	ereport(ERROR,
 			(errmsg_internal("unrecognized return value from WaitForMultipleObjects: %d (error code %lu)", r, GetLastError())));
 	return 0;
-- 
2.42.0

From 86d3a318af19d8fa476e36aae1c6b754912d7c4c Mon Sep 17 00:00:00 2001
From: Thomas Munro <[email-masked]>
Date: Fri, 10 Nov 2023 10:24:32 +1300
Subject: [PATCH v2 6/6] Reinstate "graceful shutdown" changes for Windows.

This reverts commit 29992a6a509b256efc4ac560a1586b51a64b2637.

See the commit messages for 6051857fc and ed52c3707.
---
 src/backend/libpq/pqcomm.c | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c
index 522584e597..0ca93fefc8 100644
--- a/src/backend/libpq/pqcomm.c
+++ b/src/backend/libpq/pqcomm.c
@@ -280,15 +280,30 @@ socket_close(int code, Datum arg)
 		secure_close(MyProcPort);
 
 		/*
-		 * Formerly we did an explicit close() here, but it seems better to
-		 * leave the socket open until the process dies.  This allows clients
-		 * to perform a "synchronous close" if they care --- wait till the
-		 * transport layer reports connection closure, and you can be sure the
-		 * backend has exited.
+		 * On most platforms, we leave the socket open until the process dies.
+		 * This allows clients to perform a "synchronous close" if they care
+		 * --- wait till the transport layer reports connection closure, and
+		 * you can be sure the backend has exited.  Saves a kernel call, too.
 		 *
-		 * We do set sock to PGINVALID_SOCKET to prevent any further I/O,
-		 * though.
+		 * However, that does not work on Windows: if the kernel closes the
+		 * socket it will invoke an "abortive shutdown" that discards any data
+		 * not yet sent to the client.  (This is a flat-out violation of the
+		 * TCP RFCs, but count on Microsoft not to care about that.)  To get
+		 * the spec-compliant "graceful shutdown" behavior, we must invoke
+		 * closesocket() explicitly.  When using OpenSSL, it seems that clean
+		 * shutdown also requires an explicit shutdown() call.
+		 *
+		 * This code runs late enough during process shutdown that we should
+		 * have finished all externally-visible shutdown activities, so that
+		 * in principle it's good enough to act as a synchronous close on
+		 * Windows too.  But it's a lot more fragile than the other way.
 		 */
+#ifdef WIN32
+		shutdown(MyProcPort->sock, SD_SEND);
+		closesocket(MyProcPort->sock);
+#endif
+
+		/* In any case, set sock to PGINVALID_SOCKET to prevent further I/O */
 		MyProcPort->sock = PGINVALID_SOCKET;
 	}
 }
-- 
2.42.0

---

From: Alexander Lakhin <[email-masked]>
Date: Tue, 21 Nov 2023 14:00:00 +0300

Hello Thomas,

> entirely on reading the manual and vicariously hacking a computer I

As it also might (and I would like it to) be the final attempt, I decided
to gather information and all the cases that we had on this topic.
At least, for the last 5 years we've seen:

[1] 2019-01-18: Re: BUG #15598: PostgreSQL Error Code is not reported when connection terminated due to 
idle-in-transaction timeout
     test 099_case_15598 made (in attachment)
no commit

[2] 2019-01-22: Rare SSL failures on eelpout
     references [1]
     test 099_rare_ssl_failures made (in attachment)
commit 2019-03-19 1f39a1c06: Restructure libpq's hqandling of send failures.

[3] 2019-11-28: pgsql: Add tests for '-f' option in dropdb utility.
     test 051_dropdb_force was proposed (included in the attachment)
commit 2019-11-30 98a9b37ba: Revert commits 290acac92b and 8a7e9e9dad.

[4] 2019-12-06: closesocket behavior in different platforms
     references [1], [2], [3]; a documentation change proposed
no commit

[5] 2020-06-03: libpq copy error handling busted
     test 099_pgbench_with_server_off made (in attachment)
commit 2020-06-07 7247e243a: Try to read data from the socket in pqSendSome's write_failed paths. (a fix for 1f39a1c06)

[6] 2020-10-19: BUG #16678: The ecpg connect/test5 test sometimes fails on Windows
no commit

[7] 2021-11-17: Windows: Wrong error message at connection termination
    references [6]
commit: 2021-12-02 6051857fc: On Windows, close the client socket explicitly during backend shutdown.

[8] 2021-12-05 17:03:18: MSVC SSL test failure
commit: 2021-12-07 ed52c3707: On Windows, also call shutdown() while closing the client socket.

[9] 2021-12-30: Why is src/test/modules/committs/t/002_standby.pl flaky?
    additional test 099_postgres_fdw_disconnect made (in attachment)
commit 2022-01-26 75674c7ec: Revert "graceful shutdown" changes for Windows, in back branches only. (REL_14_STABLE)
commit 2022-03-22 29992a6a5: Revert "graceful shutdown" changes for Windows. (master)

[10] 2022-02-02 19:19:22: BUG #17391: While using --with-ssl=openssl and PG_TEST_EXTRA='ssl' options, SSL tests fail on 
OpenBSD 7.0
commit 2022-02-12 335fa5a26: Fix thinko in PQisBusy(). (a fix for 1f39a1c06)
commit 2022-02-12 faa189c93: Move libpq's write_failed mechanism down to pqsecure_raw_write(). (a fix for 1f39a1c06)

As it becomes difficult to test/check all those cases scattered around, I
decided to retell the whole story by means of tests. Please look at the
script win-sock-tests-01.cmd attached, which can be executed both on
Windows (as regular cmd) and on Linux (bash win-sock-tests-01.cmd).

At the end of the script we can see several things.
First, the last patchset you posted, applied to b282fa88d~1, fixes the
issue discussed in this thread (it eliminates failures of
commit_ts_002_standby (and also 099_postgres_fdw_disconnect)).

Second, with the sleep added (see [6]), I had the same results of
`meson test` on Windows and on Linux.
Namely, there are some tests failing (see win-sock-tests-01.cmd) due to
walsender preventing server stop.
I describe this issue separately (see details in walsender-cannot-exit.txt;
maybe it's worth to discuss it in a separate thread) as it's kind of
off-topic. With the supplementary sleep() added to WalSndLoop(), the
complete `meson test` passes successfully both on Windows and on Linux.

Third, cases [1] and [3] are still broken, due to a Windows peculiarity.
Please see server.c and client.c attached, which demonstrate:
Case "no shutdown/closesocket" on Windows:
C:\src>server
Listening for incoming connections...
                         C:\src>client
Client connected: 127.0.0.1:64395
                         Connection to server established. Enter message: msg
Client message: msg
Sending message...
                         Sleeping...
Exiting...
C:\src>
                         Calling recv()...
                         recv() failed

Case "no shutdown/closesocket" on Linux:
$ server
Listening for incoming connections...
                         $ client
Client connected: 127.0.0.1:33044
                         Connection to server established. Enter message: msg
Client message: msg
Sending message...
                         Sleeping...
Exiting...
$
                         Calling recv()...
                         Server message: MESSAGE

Case "shutdown/closesocket" on Windows:
C:\src>server shutdown closesocket
Listening for incoming connections...
                         C:\src>client
Client connected: 127.0.0.1:64396
                         Connection to server established. Enter message: msg
Client message: msg
Sending message...
                         Sleeping...
Calling shutdown()...
Calling closesocket()...
Exiting...
C:\src>
                         Calling recv()...
                         Server message: MESSAGE

That's okay so far, but what makes cases [1]/[3] different from all cases
in the whole existing test suite, which now performed successfully, is
that psql calls send() before recv() on a socket closed and abandoned by
the server.
Those programs show on Windows:
C:\src>server shutdown closesocket
Listening for incoming connections...
                         C:\src>client send_before_recv
Client connected: 127.0.0.1:64397
                         Connection to server established. Enter message: msg
Client message: msg
Sending message...
                         Sleeping...
Calling shutdown()...
Calling closesocket()...
Exiting...
C:\src>
                         send() returned 4
                         Calling recv()...
                         recv() failed

As known, on Linux the same scenario works just fine.

Fourth, tests 099_rare_ssl_failures (and 001_ssl_tests, though more rarely)
fail for me with the latest patches (only on Windows again):
...
  8/10 postgresql:ssl_1 / ssl_1/099_rare_ssl_failures ERROR           141.34s   exit status 3
...
  9/10 postgresql:ssl_7 / ssl_7/099_rare_ssl_failures OK              142.52s   2000 subtests passed
10/10 postgresql:ssl_6 / ssl_6/099_rare_ssl_failures OK              143.00s   2000 subtests passed

Ok:                 2
Expected Fail:      0
Fail:               8

ssl_1\099_rare_ssl_failures\log\regress_log_099_rare_ssl_failures.txt:
...
iteration 354
[20:57:06.984](0.106s) ok 707 - certificate authorization fails with revoked client cert with server-side CRL directory
[20:57:06.984](0.000s) ok 708 - certificate authorization fails with revoked client cert with server-side CRL directory: 
matches
iteration 355
[20:57:07.156](0.172s) ok 709 - certificate authorization fails with revoked client cert with server-side CRL directory
[20:57:07.156](0.001s) not ok 710 - certificate authorization fails with revoked client cert with server-side CRL 
directory: matches
[20:57:07.159](0.003s) #   Failed test 'certificate authorization fails with revoked client cert with server-side CRL 
directory: matches'
#   at .../src/test/ssl_1/t/099_rare_ssl_failures.pl line 88.
[20:57:07.159](0.000s) #                   'psql: error: connection to server at "127.0.0.1", port 59843 failed: could 
not receive data from server: Software caused connection abort (0x00002745/10053)
# SSL SYSCALL error: Software caused connection abort (0x00002745/10053)'
#     doesn't match '(?^:SSL error: sslv3 alert certificate revoked)'
...

It seems to me that it can have the same explanation (if openssl can call
send() before recv() under the hood), but maybe it should be investigated
further.

> Review/poking-with-a-stick/trying-to-break-it most welcome.

I could not find anything suspicious in the code, except for maybe a typo
"The are ...".

[1] https://www.postgresql.org/message-id/flat/87k1iy44fd.fsf%40news-spur.riddles.org.uk#ba0c07f13c300d42fd537855dd95dd2b
[2] https://www.postgresql.org/message-id/flat/CAEepm%3D2n6Nv%2B5tFfe8YnkUm1fXgvxR0Mm1FoD%2BQKG-vLNGLyKg%40mail.gmail.com
[3] https://www.postgresql.org/message-id/flat/[email-masked]
[4] 
https://www.postgresql.org/message-id/flat/CALDaNm2tEvr_Kum7SyvFn0%3D6H3P0P-Zkhnd%3DdkkX%2BQ%3DwKutZ%3DA%40mail.gmail.com
[5] https://www.postgresql.org/message-id/flat/20200603201242.ofvm4jztpqytwfye%40alap3.anarazel.de
[6] https://www.postgresql.org/message-id/[email-masked]
[7] https://www.postgresql.org/message-id/flat/90b34057-4176-7bb0-0dbb-9822a5f6425b%40greiz-reinsdorf.de
[8] https://www.postgresql.org/message-id/flat/af5e0bf3-6a61-bb97-6cba-061ddf22ff6b%40dunslane.net
[9] 
https://www.postgresql.org/message-id/flat/CA%2BhUKG%2BOeoETZQ%3DQw5Ub5h3tmwQhBmDA%3DnuNO3KG%3DzWfUypFAw%40mail.gmail.com
[10] https://www.postgresql.org/message-id/flat/17391-304f81bcf724b58b%40postgresql.org

Best regards,
Alexander

diff --git a/contrib/postgres_fdw/meson.build b/contrib/postgres_fdw/meson.build
index 2b451f165e..674ab0a7b9 100644
--- a/contrib/postgres_fdw/meson.build
+++ b/contrib/postgres_fdw/meson.build
@@ -39,4 +39,9 @@ tests += {
     ],
     'regress_args': ['--dlpath', meson.build_root() / 'src/test/regress'],
   },
+  'tap': {
+    'tests': [
+      't/099_postgres_fdw_disconnect.pl',
+    ],
+  },
 }
diff --git a/contrib/postgres_fdw/t/099_postgres_fdw_disconnect.pl b/contrib/postgres_fdw/t/099_postgres_fdw_disconnect.pl
new file mode 100644
index 0000000000..f1b51fb448
--- /dev/null
+++ b/contrib/postgres_fdw/t/099_postgres_fdw_disconnect.pl
@@ -0,0 +1,87 @@
+# Test postgres_fdw reaction on target server disconnection
+
+use strict;
+use warnings;
+
+use PostgreSQL::Test::Utils;
+use Test::More tests => 1;
+use PostgreSQL::Test::Cluster;
+
+use Time::HiRes qw(time usleep);
+use DateTime;
+
+use threads;
+use threads::shared;
+
+sub restart_thread {
+	my @args = @_;
+	my ($server) = @args;
+	$SIG{'KILL'} = sub { threads->exit(); };
+	sleep(3);
+	for (my $i = 0; $i < 100; $i++) {
+		sleep(5);
+		$server->restart();
+	}
+}
+
+my $foreign = PostgreSQL::Test::Cluster->new('foreign');
+$foreign->init();
+
+$foreign->append_conf(
+	'postgresql.conf', qq{
+log_min_messages = DEBUG1
+log_min_error_statement = log
+log_connections = on
+log_disconnections = on
+log_line_prefix = '%m|%u|%d|%c|'
+log_statement = 'all'
+	});
+$foreign->start;
+my $foreign_port = $foreign->port;
+
+my $local = PostgreSQL::Test::Cluster->new('local');
+$local->init();
+
+$local->append_conf(
+	'postgresql.conf', qq{
+log_min_messages = DEBUG1
+log_min_error_statement = log
+log_connections = on
+log_disconnections = on
+log_line_prefix = '%m|%u|%d|%c|'
+log_statement = 'all'
+	});
+$local->start();
+
+$foreign->safe_psql('postgres', qq{
+CREATE TABLE large(a int, t text);
+INSERT INTO large SELECT x, rpad(x::text, 100) FROM generate_series(0, 999999) x;
+});
+
+
+$local->safe_psql('postgres', qq{
+CREATE EXTENSION postgres_fdw;
+CREATE SERVER fpg FOREIGN DATA WRAPPER postgres_fdw OPTIONS (dbname 'postgres', port '$foreign_port');
+CREATE USER MAPPING FOR CURRENT_USER SERVER fpg;
+CREATE FUNCTION fx2(i integer) RETURNS int AS 'begin return i * 2; end;' LANGUAGE plpgsql;
+});
+
+my $outputdir = $PostgreSQL::Test::Utils::tmp_check;
+
+$local->safe_psql('postgres', 'IMPORT FOREIGN SCHEMA public FROM SERVER fpg INTO public;');
+print($local->psql('postgres', 'EXPLAIN (VERBOSE) SELECT * FROM large WHERE a = fx2(a)') . "\n");
+my $thread = threads->create('restart_thread', $foreign);
+$ENV{PGHOST} = $local->host;
+$ENV{PGPORT} = $local->port;
+$ENV{PGCTLTIMEOUT} = 180;
+for (my $i = 1; $i <= 50; $i++) {
+	diag(" executing query ($i)...");
+# Avoid using IPC::Run due to it's own quirks
+#	my ($ret, $stdout, $stderr) = $local->psql('postgres', 'SELECT * FROM large WHERE a = fx2(a)');
+	my $ret = system("psql postgres -c \"SELECT $i i, * FROM large WHERE a = fx2(a)\" >>\"$outputdir/psql.log\" 2>&1");
+	diag(" result: \t$ret");
+}
+$thread->kill('KILL')->detach;
+sleep(2);
+$foreign->_update_pid(1);
+ok(1);
diff --git a/src/test/modules/test_misc/meson.build b/src/test/modules/test_misc/meson.build
index 911084ac0f..bee227eabe 100644
--- a/src/test/modules/test_misc/meson.build
+++ b/src/test/modules/test_misc/meson.build
@@ -10,6 +10,11 @@ tests += {
       't/002_tablespace.pl',
       't/003_check_guc.pl',
       't/004_io_direct.pl',
+      't/099_case_15598.pl',
+      't/051_dropdb_force.pl',
+      't/099_pgbench_with_server_off.pl',
+      't/099_ecpg_16678.pl',
+      't/099_commit_ts_002_standby.pl',
     ],
   },
 }
diff --git a/src/test/modules/test_misc/t/051_dropdb_force.pl b/src/test/modules/test_misc/t/051_dropdb_force.pl
new file mode 100644
index 0000000000..94d170f081
--- /dev/null
+++ b/src/test/modules/test_misc/t/051_dropdb_force.pl
@@ -0,0 +1,106 @@
+#
+# Tests the force option of drop database command.
+#
+use strict;
+use warnings;
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+# To avoid hanging while expecting some specific input from a psql
+# instance being driven by us, add a timeout high enough that it
+# should never trigger even on very slow machines, unless something
+# is really wrong.
+my $psql_timeout = IPC::Run::timer(60);
+
+my $node = PostgreSQL::Test::Cluster->new('main');
+$node->init;
+$node->start;
+
+# Create a database that will be dropped.  This will test that the force
+# option works when no other backend is connected to the database being
+# dropped.
+$node->safe_psql('postgres', 'CREATE DATABASE foobar');
+$node->issues_sql_like(
+        [ 'dropdb', '--force', 'foobar' ],
+        qr/statement: DROP DATABASE foobar WITH \(FORCE\);/,
+        'SQL DROP DATABASE (FORCE) run');
+
+# database foobar must not exist.
+is( $node->safe_psql(
+		'postgres',
+		qq[SELECT EXISTS(SELECT * FROM pg_database WHERE datname='foobar');]
+	),
+	'f',
+	'database foobar was removed');
+
+# Create a database that will be dropped.  This will test that the force
+# option works when one other backend is connected to the database being
+# dropped.
+$node->safe_psql('postgres', 'CREATE DATABASE foobar1');
+
+# Run psql, keeping session alive, so we have an alive backend to kill.
+my ($killme_stdin, $killme_stdout, $killme_stderr) = ('', '', '');
+my $killme = IPC::Run::start(
+	[
+		'psql', '-X', '-qAt', '-v', 'ON_ERROR_STOP=1', '-f', '-', '-d',
+		$node->connstr('foobar1')
+	],
+	'<',
+	\$killme_stdin,
+	'>',
+	\$killme_stdout,
+	'2>',
+	\$killme_stderr,
+	$psql_timeout);
+
+# Ensure killme process is active.
+$killme_stdin .= q[
+SELECT pg_backend_pid();
+];
+ok( pump_until(
+		$killme, $psql_timeout, \$killme_stdout, qr/[[:digit:]]+[\r\n]$/m),
+	'acquired pid for SIGTERM');
+my $pid = $killme_stdout;
+chomp($pid);
+$killme_stdout = '';
+$killme_stderr = '';
+
+# Check the connections on foobar1 database.
+is( $node->safe_psql(
+		'postgres',
+		qq[SELECT pid FROM pg_stat_activity WHERE datname='foobar1' AND pid = $pid;]
+	),
+	$pid,
+	'database foobar1 is used');
+
+# Now drop database with dropdb --force command.
+$node->issues_sql_like(
+	[ 'dropdb', '--force', 'foobar1' ],
+	qr/statement: DROP DATABASE foobar1 WITH \(FORCE\);/,
+	'SQL DROP DATABASE (FORCE) run');
+
+# Check that psql sees the killed backend as having been terminated.
+$killme_stdin .= q[
+SELECT 1;
+];
+ok( pump_until(
+		$killme, $psql_timeout, \$killme_stderr,
+		qr/FATAL:  terminating connection due to administrator command/m),
+	"psql query died successfully after SIGTERM");
+$killme_stderr = '';
+$killme_stdout = '';
+$killme->finish;
+
+# database foobar1 must not exist.
+is( $node->safe_psql(
+		'postgres',
+		qq[SELECT EXISTS(SELECT * FROM pg_database WHERE datname='foobar1');]
+	),
+	'f',
+	'database foobar1 was removed');
+
+$node->stop();
+
+done_testing();
diff --git a/src/test/modules/test_misc/t/099_case_15598.pl b/src/test/modules/test_misc/t/099_case_15598.pl
new file mode 100644
index 0000000000..57243b04d7
--- /dev/null
+++ b/src/test/modules/test_misc/t/099_case_15598.pl
@@ -0,0 +1,61 @@
+# Run ecpg regression tests in a loop
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+use File::Basename;
+
+# Initialize node
+my $node = PostgreSQL::Test::Cluster->new('p1');
+$node->init(
+);
+
+$node->append_conf(
+	'postgresql.conf',
+	qq{
+});
+
+$node->start;
+
+$node->safe_psql('postgres', 'select 1');
+
+my $psql_timeout = IPC::Run::timer(10);
+# Run psql, ...
+my ($psql_stdin, $psql_stdout, $psql_stderr) = ('', '', '');
+my $psql = IPC::Run::start(
+    [
+        'psql', '-X', '-At', '-v', 'ON_ERROR_STOP=1', '-f', '-', '-d',
+        $node->connstr('postgres')
+    ],
+    '<',
+    \$psql_stdin,
+    '>',
+    \$psql_stdout,
+    '2>',
+    \$psql_stderr,
+    $psql_timeout);
+
+$psql_stdin .= q[
+SET idle_in_transaction_session_timeout=500;
+BEGIN;
+SELECT * FROM pg_class;
+];
+
+$psql->pump_nb();
+sleep(1);
+
+$psql_stdin .= q[
+SELECT 'OK';
+];
+
+ok(pump_until(
+        $psql, $psql_timeout, \$psql_stderr, qr/FATAL:  terminating connection due to idle-in-transaction timeout/m), "expected FATAL message received");
+
+print($psql_stdin . "\n");
+print("psql stdout: ". $psql_stdout . "\npsql stderr: " . $psql_stderr. "\n");
+
+ok(1);
+$node->stop;
+
+done_testing();
diff --git a/src/test/modules/test_misc/t/099_commit_ts_002_standby.pl b/src/test/modules/test_misc/t/099_commit_ts_002_standby.pl
new file mode 100644
index 0000000000..59cc2b1244
--- /dev/null
+++ b/src/test/modules/test_misc/t/099_commit_ts_002_standby.pl
@@ -0,0 +1,68 @@
+
+# Copyright (c) 2021-2023, PostgreSQL Global Development Group
+
+# Test simple scenario involving a standby
+
+use strict;
+use warnings;
+
+use PostgreSQL::Test::Utils;
+use Test::More;
+use PostgreSQL::Test::Cluster;
+
+my $bkplabel = 'backup';
+my $primary = PostgreSQL::Test::Cluster->new('primary');
+$primary->init(allows_streaming => 1);
+
+$primary->append_conf(
+	'postgresql.conf', qq{
+	track_commit_timestamp = on
+	max_wal_senders = 5
+	});
+$primary->start;
+$primary->backup($bkplabel);
+
+my $standby = PostgreSQL::Test::Cluster->new('standby');
+$standby->init_from_backup($primary, $bkplabel, has_streaming => 1);
+$standby->start;
+
+for my $i (1 .. 10)
+{
+	$primary->safe_psql('postgres', "create table t$i()");
+}
+my $primary_ts = $primary->safe_psql('postgres',
+	qq{SELECT ts.* FROM pg_class, pg_xact_commit_timestamp(xmin) AS ts WHERE relname = 't10'}
+);
+my $primary_lsn =
+  $primary->safe_psql('postgres', 'select pg_current_wal_lsn()');
+$standby->poll_query_until('postgres',
+	qq{SELECT '$primary_lsn'::pg_lsn <= pg_last_wal_replay_lsn()})
+  or die "standby never caught up";
+
+my $standby_ts = $standby->safe_psql('postgres',
+	qq{select ts.* from pg_class, pg_xact_commit_timestamp(xmin) ts where relname = 't10'}
+);
+is($primary_ts, $standby_ts, "standby gives same value as primary");
+
+$primary->append_conf('postgresql.conf', 'track_commit_timestamp = off');
+$primary->restart;
+$primary->safe_psql('postgres', 'checkpoint');
+$primary_lsn = $primary->safe_psql('postgres', 'select pg_current_wal_lsn()');
+$standby->poll_query_until('postgres',
+	qq{SELECT '$primary_lsn'::pg_lsn <= pg_last_wal_replay_lsn()})
+  or die "standby never caught up";
+$standby->safe_psql('postgres', 'checkpoint');
+
+# This one should raise an error now
+my ($ret, $standby_ts_stdout, $standby_ts_stderr) = $standby->psql('postgres',
+	'select ts.* from pg_class, pg_xact_commit_timestamp(xmin) ts where relname = \'t10\''
+);
+is($ret, 3, 'standby errors when primary turned feature off');
+is($standby_ts_stdout, '',
+	"standby gives no value when primary turned feature off");
+like(
+	$standby_ts_stderr,
+	qr/could not get commit timestamp data/,
+	'expected error when primary turned feature off');
+
+done_testing();
diff --git a/src/test/modules/test_misc/t/099_ecpg_16678.pl b/src/test/modules/test_misc/t/099_ecpg_16678.pl
new file mode 100755
index 0000000000..53d1af7fe2
--- /dev/null
+++ b/src/test/modules/test_misc/t/099_ecpg_16678.pl
@@ -0,0 +1,56 @@
+# Run ecpg regression tests in a loop
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+use File::Basename;
+
+# Initialize primary node
+my $node_primary = PostgreSQL::Test::Cluster->new('p1');
+$node_primary->init(
+	'auth_extra' => [ '--create-role', 'regress_ecpg_user1,regress_ecpg_user2' ]
+);
+
+$node_primary->start;
+
+my $outputdir = $PostgreSQL::Test::Utils::tmp_check;
+my $NUM_ITERATIONS = 2;
+
+for (my $i = 0; $i < $NUM_ITERATIONS; $i++)
+{
+my $extra_opts = $ENV{EXTRA_REGRESS_OPTS} || "";
+
+my $rc =
+  system("\"$outputdir/../../../../src/interfaces/ecpg/test/pg_regress_ecpg\" "
+	  . " $extra_opts "
+	  . "--dbname=ecpg1_regression,ecpg2_regression --create-role=regress_ecpg_user1,regress_ecpg_user2 "
+	  . "--bindir= "
+	  . "--host="
+	  . $node_primary->host . " "
+	  . "--port="
+	  . $node_primary->port . " "
+	  . "--outputdir=$outputdir "
+	  . "--inputdir=$outputdir/../../../../src/interfaces/ecpg/test/ "
+	  . "--expecteddir=../../../../src/interfaces/ecpg/test/ "
+	  . ( "connect/test5 " x 50)
+);
+
+if ($rc != 0)
+{
+	# Dump out the regression diffs file, if there is one
+	my $diffs = "$outputdir/regression.diffs";
+	if (-e $diffs)
+	{
+		print "=== dumping $diffs ===\n";
+		print slurp_file($diffs);
+		print "=== EOF ===\n";
+	}
+}
+is($rc, 0, 'ecpg regression tests pass');
+($rc == 0) || last;
+}
+
+$node_primary->stop;
+
+done_testing();
diff --git a/src/test/modules/test_misc/t/099_pgbench_with_server_off.pl b/src/test/modules/test_misc/t/099_pgbench_with_server_off.pl
new file mode 100644
index 0000000000..409ef1621f
--- /dev/null
+++ b/src/test/modules/test_misc/t/099_pgbench_with_server_off.pl
@@ -0,0 +1,48 @@
+
+# Copyright (c) 2021-2023, PostgreSQL Global Development Group
+
+#
+# test pgbench with server stopped abruptly during the pgbench run
+#
+
+use strict;
+use warnings;
+
+use PostgreSQL::Test::Utils;
+use PostgreSQL::Test::Cluster;
+use Test::More;
+
+my $node = PostgreSQL::Test::Cluster->new('main');
+
+$node->init();
+$node->start;
+
+my $pb_timeout =
+  IPC::Run::timer($PostgreSQL::Test::Utils::timeout_default);
+my ($pb_stdin, $pb_stdout, $pb_stderr) = ('', '', '');
+
+$ENV{PGDATABASE} = 'postgres';
+$ENV{PGHOST} = $node->host;
+$ENV{PGPORT} = $node->port;
+
+my $pb = IPC::Run::start(
+    [
+        'pgbench',
+        '-i', '-s', '1000'
+    ],
+    '<',
+    \$pb_stdin,
+    '>',
+    \$pb_stdout,
+    '2>',
+    \$pb_stderr,
+    $pb_timeout);
+sleep(5);
+$node->stop('immediate');
+ok( pump_until(
+        $pb, $pb_timeout,
+        \$pb_stderr, qr/PQputline failed/),
+    'pgbench reacted to server shutdown during copy');
+$pb->finish();
+
+done_testing();
diff --git a/src/test/ssl/meson.build b/src/test/ssl/meson.build
index abb30ab214..9228acf6fb 100644
--- a/src/test/ssl/meson.build
+++ b/src/test/ssl/meson.build
@@ -13,6 +13,7 @@ tests += {
       't/001_ssltests.pl',
       't/002_scram.pl',
       't/003_sslinfo.pl',
+      't/099_rare_ssl_failures.pl',
     ],
   },
 }
diff --git a/src/test/ssl/t/099_rare_ssl_failures-v12.pl b/src/test/ssl/t/099_rare_ssl_failures-v12.pl
new file mode 100644
index 0000000000..1c43f00301
--- /dev/null
+++ b/src/test/ssl/t/099_rare_ssl_failures-v12.pl
@@ -0,0 +1,84 @@
+use strict;
+use warnings;
+use PostgresNode;
+use TestLib;
+use Test::More tests => 100;
+
+use File::Copy;
+
+use FindBin;
+use lib $FindBin::RealBin;
+
+use SSLServer;
+
+#### Some configuration
+
+# This is the hostname used to connect to the server. This cannot be a
+# hostname, because the server certificate is always for the domain
+# postgresql-ssl-regression.test.
+my $SERVERHOSTADDR = '127.0.0.1';
+
+my $common_connstr;
+
+sub connect_fails
+{
+	my ($node, $connstr, $test_name, %params) = @_;
+
+	my $cmd = [
+		'psql', '-X', '-A', '-t', '-c', "SELECT 'connected with $connstr'",
+		'-d', "$common_connstr $connstr" ];
+
+	my ($stdout, $stderr);
+
+	print("# Running: " . join(" ", @{$cmd}) . "\n");
+	my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr;
+
+	if (defined($params{expected_stderr}))
+	{
+		like($stderr, $params{expected_stderr}, "$test_name: matches");
+	}
+}
+
+# The client's private key must not be world-readable, so take a copy
+# of the key stored in the code tree and update its permissions.
+copy("ssl/client.key", "ssl/client_tmp.key");
+chmod 0600, "ssl/client_tmp.key";
+
+#### Part 0. Set up the server.
+
+note "setting up data directory";
+my $node = get_new_node('master');
+$node->init;
+
+# PGHOST is enforced here to set up the node, subsequent connections
+# will use a dedicated connection string.
+$ENV{PGHOST} = $node->host;
+$ENV{PGPORT} = $node->port;
+$node->start;
+configure_test_server_for_ssl($node, $SERVERHOSTADDR, 'trust');
+switch_server_cert($node, 'server-cn-only');
+
+### Part 1. Run client-side tests.
+###
+### Test that libpq accepts/rejects the connection correctly, depending
+### on sslmode and whether the server's certificate looks correct. No
+### client certificate is used in these tests.
+
+note "running client tests";
+
+$common_connstr =
+"user=ssltestuser dbname=trustdb sslcert=invalid hostaddr=$SERVERHOSTADDR host=common-name.pg-ssltest.test";
+
+
+for (my $i = 1; $i <= 100; $i++) {
+print("iteration $i\n");
+connect_fails(
+	$node,
+	"user=ssltestuser sslcert=ssl/client-revoked.crt sslkey=ssl/client-revoked.key",
+	"certificate authorization fails with revoked client cert",
+	expected_stderr => qr/SSL error: sslv3 alert certificate revoked/,
+);
+}
+
+# clean up
+unlink "ssl/client_tmp.key";
diff --git a/src/test/ssl/t/099_rare_ssl_failures.pl b/src/test/ssl/t/099_rare_ssl_failures.pl
new file mode 100644
index 0000000000..0ca70c0ff6
--- /dev/null
+++ b/src/test/ssl/t/099_rare_ssl_failures.pl
@@ -0,0 +1,101 @@
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+use File::Copy;
+
+use FindBin;
+use lib $FindBin::RealBin;
+
+use SSL::Server;
+
+#### Some configuration
+
+# This is the hostname used to connect to the server. This cannot be a
+# hostname, because the server certificate is always for the domain
+# postgresql-ssl-regression.test.
+my $SERVERHOSTADDR = '127.0.0.1';
+# This is the pattern to use in pg_hba.conf to match incoming connections.
+my $SERVERHOSTCIDR = '127.0.0.1/32';
+
+my $common_connstr;
+
+sub connect_fails
+{
+	my ($node, $connstr, $test_name, %params) = @_;
+
+	my $cmd = [
+		'psql', '-X', '-A', '-t', '-c', "SELECT 'connected with $connstr'",
+		'-d', "$common_connstr $connstr" ];
+
+	my ($stdout, $stderr);
+
+	print("# Running: " . join(" ", @{$cmd}) . "\n");
+	my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr;
+
+	if (defined($params{expected_stderr}))
+	{
+		like($stderr, $params{expected_stderr}, "$test_name: matches");
+	}
+}
+
+my $ssl_server = SSL::Server->new();
+
+# The client's private key must not be world-readable, so take a copy
+# of the key stored in the code tree and update its permissions.
+copy("ssl/client.key", "ssl/client_tmp.key");
+chmod 0600, "ssl/client_tmp.key";
+
+#### Part 0. Set up the server.
+
+note "setting up data directory";
+my $node = PostgreSQL::Test::Cluster->new('primary');
+$node->init;
+
+# PGHOST is enforced here to set up the node, subsequent connections
+# will use a dedicated connection string.
+$ENV{PGHOST} = $node->host;
+$ENV{PGPORT} = $node->port;
+$node->start;
+
+$ssl_server->configure_test_server_for_ssl($node, $SERVERHOSTADDR,
+    $SERVERHOSTCIDR, 'trust');
+
+$ssl_server->switch_server_cert(
+    $node,
+    certfile => 'server-cn-only',
+    crldir => 'root+client-crldir');
+
+### Part 1. Run client-side tests.
+###
+### Test that libpq accepts/rejects the connection correctly, depending
+### on sslmode and whether the server's certificate looks correct. No
+### client certificate is used in these tests.
+
+note "running client tests";
+
+$common_connstr =
+"user=ssltestuser dbname=trustdb sslcert=invalid hostaddr=$SERVERHOSTADDR host=common-name.pg-ssltest.test";
+
+
+for (my $i = 1; $i <= 1000; $i++) {
+print("iteration $i\n");
+$node->connect_fails(
+	"$common_connstr user=ssltestuser sslcert=ssl/client-revoked.crt "
+	. $ssl_server->sslkey('client-revoked.key'),
+	"certificate authorization fails with revoked client cert with server-side CRL directory",
+	expected_stderr => qr/SSL error: sslv3 alert certificate revoked/,
+	# temporarily(?) skip this check due to timing issue
+	#   log_like => [
+	#       qr{Client certificate verification failed at depth 0: certificate revoked},
+	#       qr{Failed certificate data \(unverified\): subject "/CN=ssltestuser", serial number 2315134995201656577, issuer "/CN=Test CA for PostgreSQL SSL regression test client certs"},
+	#   ]
+);
+}
+
+# clean up
+unlink "ssl/client_tmp.key";
+
+done_testing();

---

From: Robert Haas <[email-masked]>
Date: Fri, 5 Jan 2024 12:04:32 -0500

> entirely on reading the manual and vicariously hacking a computer I

I'd first like to congratulate this thread on reaching its second
birthday. The CommitFest entry hasn't quite made it to the two year
mark yet - expect that in another month or so - but thread itself is
over the line.

Regarding 0001, I don't know if we really need SH_RAW_FREE. You can
just define your own SH_FREE implementation in userspace. That doesn't
work for SH_RAW_ALLOCATOR because there's code in simplehash.h that
knows about memory contexts apart from the actual definition of
SH_ALLOCATE - e.g. we include a MemoryContext pointer in SH_TYPE, and
in the signature of SH_CREATE. But SH_FREE doesn't seem to have any
similar issues. Maybe it's still worth doing for convenience -- I
haven't thought about that very hard -- but it doesn't seem to be
required in the same way that SH_RAW_ALLOCATOR was.

I wonder whether we really want 0002. It seems like a pretty
significant behavior change -- now everybody using simplehash has to
worry about whether failure cases are possible. And maybe there's some
performance overhead. And most of the changes are restricted to the
SH_RAW_ALLOCATOR case, but the changes to SH_GROW are not. And making
this contingent on SH_RAW_ALLOCATOR doesn't seem principled.

I kind of wonder whether trying to handle OOM here is the wrong
direction to go. What if we just bail out hard if we can't insert into
the hash table? I think that we don't expect the hash table to ever be
very large (right?) and we don't install these kinds of defenses
everywhere that OOM on a small memory allocation is a possibility (or
at least I don't think we do). I'm actually sort of unclear about why
you're trying to force this to use raw malloc/free instead of
palloc/pfree. Do we need to use this on the frontend side? Do we need
it on the backend side prior to the memory context infrastructure
being up?

-- 
Robert Haas
EDB: http://www.enterprisedb.com

---

From: Peter Smith <[email-masked]>
Date: Mon, 22 Jan 2024 15:26:09 +1100

2024-01 Commitfest.

Hi, This patch has a CF status of "Needs Review" [1], but it seems
like there were CFbot test failures last time it was run [1]. Please
have a look and post an updated version if necessary.

======
1[] https://commitfest.postgresql.org/46/3523/
[1] https://cirrus-ci.com/github/postgresql-cfbot/postgresql/commitfest/46/3523