Hi,

Here's a work-in-progress patch that uses WaitEventSet for the main
event loop in the postmaster, with a latch as the wakeup mechanism for
"PM signals" (requests from backends to do things like start a
background worker, etc).  There are still raw signals that are part of
the external interface (SIGHUP etc), but those handlers just set a
flag and set the latch, instead of doing the state machine work.  Some
advantages I can think of:

1.  Inherits various slightly more efficient modern kernel APIs for
multiplexing.
2.  Will automatically benefit from later improvements to WaitEventSet.
3.  Looks much more like the rest of our code.
4.  Requires no obscure signal programming knowledge to understand.
5.  Removes the strange call stacks we have, where most of postgres is
forked from inside a signal handler.
6.  Might help with weirdness and bugs in some signal implementations
(Cygwin, NetBSD?).
7.  Removes the need to stat() PROMOTE_SIGNAL_FILE and
LOGROTATE_SIGNAL_FILE whenever PM signals are sent, now that SIGUSR1
is less overloaded.
8.  It's a small step towards removing the need to emulate signals on Windows.

In order to avoid adding a new dependency on the contents of shared
memory, I introduced SetLatchRobustly() that will always use the slow
path kernel wakeup primitive, even in cases where SetLatch() would
not.  The idea here is that if one backend trashes shared memory,
others backends can still wake the postmaster even though it may
appear that the postmaster isn't waiting or the latch is already set.
It would be possible to go further and have a robust wait mode that
doesn't read is_set too.  It was indecision here that stopped me
proposing this sooner...

One thing that might need more work is cleanup of the PM's WES in
child processes.  Also I noticed in passing that the Windows kernel
event handles for latches are probably leaked on crash-reinit, but
that was already true, this just adds one more.  Also the way I re-add
the latch every time through the event loop in case there was a
crash-reinit is stupid, I'll tidy that up.

This is something I extracted and rejuvenated from a larger set of
patches I was hacking on a year or two ago to try to get rid of lots
of users of raw signals.  The recent thread about mamba's struggles
and the possibility that latches might help reminded me to dust this
part off, and potentially avoid some duplicated effort.

I'm not saying this is free of bugs, but it's passing on CI and seems
like enough to share and see what people think.

(Some other ideas I thought about back then: we could invent
WL_SIGNAL, and not need all those global flag variables or the
handlers that set the latch.  For eg kqueue it's trivial, and for
ancient Unixes you could do a sort of home-made signalfd with a single
generic signal handler that just does write(self_pipe, &siginfo,
sizeof(siginfo)).  But that starts to seems like refactoring for
refactoring's sake; that's probably how I'd write a native kqueue
program, but it's not even that obvious to me that we really should be
pretending that Windows has signals, which put me off that idea.
Perhaps in a post-fake-signals world we just have the postmaster's
event loop directly consume commands from pg_ctl from a control pipe?
Too many decisions at once, I gave that line of thinking up for now.)
From 978aa358885312372f842cd47549bb04a78477ab Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.mu...@gmail.com>
Date: Wed, 9 Nov 2022 22:59:58 +1300
Subject: [PATCH] Give the postmaster a WaitEventSet and a latch.

Traditionally, the postmaster's architecture was quite unusual.  It did
its main work entirely inside signal handlers, which were only unblocked
while waiting in select().

Switch to a more typical architecture, where signal handlers just set
flags and use a latch to close races.  Now the postmaster looks like
all other PostgreSQL processes, multiplexing its event processing in
epoll_wait()/kevent()/poll()/WaitForMultipleObjects() depending on the
OS.

Work in progress!
---
 src/backend/libpq/pqsignal.c        |  40 ----
 src/backend/postmaster/postmaster.c | 336 ++++++++++++++--------------
 src/backend/storage/ipc/latch.c     |  54 ++++-
 src/backend/storage/ipc/pmsignal.c  |  26 ++-
 src/backend/utils/init/miscinit.c   |  13 +-
 src/include/libpq/pqsignal.h        |   3 -
 src/include/miscadmin.h             |   1 +
 src/include/storage/latch.h         |   9 +-
 src/include/storage/pmsignal.h      |   3 +
 9 files changed, 259 insertions(+), 226 deletions(-)

diff --git a/src/backend/libpq/pqsignal.c b/src/backend/libpq/pqsignal.c
index 1ab34c5214..718043a39d 100644
--- a/src/backend/libpq/pqsignal.c
+++ b/src/backend/libpq/pqsignal.c
@@ -97,43 +97,3 @@ pqinitmask(void)
 	sigdelset(&StartupBlockSig, SIGALRM);
 #endif
 }
-
-/*
- * Set up a postmaster signal handler for signal "signo"
- *
- * Returns the previous handler.
- *
- * This is used only in the postmaster, which has its own odd approach to
- * signal handling.  For signals with handlers, we block all signals for the
- * duration of signal handler execution.  We also do not set the SA_RESTART
- * flag; this should be safe given the tiny range of code in which the
- * postmaster ever unblocks signals.
- *
- * pqinitmask() must have been invoked previously.
- */
-pqsigfunc
-pqsignal_pm(int signo, pqsigfunc func)
-{
-	struct sigaction act,
-				oact;
-
-	act.sa_handler = func;
-	if (func == SIG_IGN || func == SIG_DFL)
-	{
-		/* in these cases, act the same as pqsignal() */
-		sigemptyset(&act.sa_mask);
-		act.sa_flags = SA_RESTART;
-	}
-	else
-	{
-		act.sa_mask = BlockSig;
-		act.sa_flags = 0;
-	}
-#ifdef SA_NOCLDSTOP
-	if (signo == SIGCHLD)
-		act.sa_flags |= SA_NOCLDSTOP;
-#endif
-	if (sigaction(signo, &act, &oact) < 0)
-		return SIG_ERR;
-	return oact.sa_handler;
-}
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index a8a246921f..528ad494c3 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -362,6 +362,13 @@ static volatile sig_atomic_t WalReceiverRequested = false;
 static volatile bool StartWorkerNeeded = true;
 static volatile bool HaveCrashedWorker = false;
 
+/* set when signals arrive */
+static volatile sig_atomic_t pending_reload_request;
+static volatile sig_atomic_t pending_shutdown_request;
+static volatile sig_atomic_t pending_child_exit;
+static volatile sig_atomic_t pending_logrotate_check_request;
+static volatile sig_atomic_t pending_promote_check_request;
+
 #ifdef USE_SSL
 /* Set when and if SSL has been initialized properly */
 static bool LoadedSSL = false;
@@ -380,10 +387,14 @@ static void getInstallationPaths(const char *argv0);
 static void checkControlFile(void);
 static Port *ConnCreate(int serverFd);
 static void ConnFree(Port *port);
-static void SIGHUP_handler(SIGNAL_ARGS);
-static void pmdie(SIGNAL_ARGS);
-static void reaper(SIGNAL_ARGS);
-static void sigusr1_handler(SIGNAL_ARGS);
+static void handle_file_check_request_signal(SIGNAL_ARGS);
+static void handle_reload_request_signal(SIGNAL_ARGS);
+static void process_reload_request(void);
+static void handle_shutdown_request_signal(SIGNAL_ARGS);
+static void process_shutdown_request(void);
+static void handle_child_exit_signal(SIGNAL_ARGS);
+static void process_child_exit(void);
+static void process_backend_request(void);
 static void process_startup_packet_die(SIGNAL_ARGS);
 static void dummy_handler(SIGNAL_ARGS);
 static void StartupPacketTimeoutHandler(void);
@@ -401,7 +412,6 @@ static int	BackendStartup(Port *port);
 static int	ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done);
 static void SendNegotiateProtocolVersion(List *unrecognized_protocol_options);
 static void processCancelRequest(Port *port, void *pkt);
-static int	initMasks(fd_set *rmask);
 static void report_fork_failure_to_client(Port *port, int errnum);
 static CAC_state canAcceptConnections(int backend_type);
 static bool RandomCancelKey(int32 *cancel_key);
@@ -575,6 +585,7 @@ PostmasterMain(int argc, char *argv[])
 
 	IsPostmasterEnvironment = true;
 
+
 	/*
 	 * Start our win32 signal implementation
 	 */
@@ -609,26 +620,6 @@ PostmasterMain(int argc, char *argv[])
 	/*
 	 * Set up signal handlers for the postmaster process.
 	 *
-	 * In the postmaster, we use pqsignal_pm() rather than pqsignal() (which
-	 * is used by all child processes and client processes).  That has a
-	 * couple of special behaviors:
-	 *
-	 * 1. We tell sigaction() to block all signals for the duration of the
-	 * signal handler.  This is faster than our old approach of
-	 * blocking/unblocking explicitly in the signal handler, and it should also
-	 * prevent excessive stack consumption if signals arrive quickly.
-	 *
-	 * 2. We do not set the SA_RESTART flag.  This is because signals will be
-	 * blocked at all times except when ServerLoop is waiting for something to
-	 * happen, and during that window, we want signals to exit the select(2)
-	 * wait so that ServerLoop can respond if anything interesting happened.
-	 * On some platforms, signals marked SA_RESTART would not cause the
-	 * select() wait to end.
-	 *
-	 * Child processes will generally want SA_RESTART, so pqsignal() sets that
-	 * flag.  We expect children to set up their own handlers before
-	 * unblocking signals.
-	 *
 	 * CAUTION: when changing this list, check for side-effects on the signal
 	 * handling setup of child processes.  See tcop/postgres.c,
 	 * bootstrap/bootstrap.c, postmaster/bgwriter.c, postmaster/walwriter.c,
@@ -638,26 +629,21 @@ PostmasterMain(int argc, char *argv[])
 	pqinitmask();
 	PG_SETMASK(&BlockSig);
 
-	pqsignal_pm(SIGHUP, SIGHUP_handler);	/* reread config file and have
-											 * children do same */
-	pqsignal_pm(SIGINT, pmdie); /* send SIGTERM and shut down */
-	pqsignal_pm(SIGQUIT, pmdie);	/* send SIGQUIT and die */
-	pqsignal_pm(SIGTERM, pmdie);	/* wait for children and shut down */
-	pqsignal_pm(SIGALRM, SIG_IGN);	/* ignored */
-	pqsignal_pm(SIGPIPE, SIG_IGN);	/* ignored */
-	pqsignal_pm(SIGUSR1, sigusr1_handler);	/* message from child process */
-	pqsignal_pm(SIGUSR2, dummy_handler);	/* unused, reserve for children */
-	pqsignal_pm(SIGCHLD, reaper);	/* handle child termination */
+	pqsignal(SIGHUP, handle_reload_request_signal);
+	pqsignal(SIGINT, handle_shutdown_request_signal);
+	pqsignal(SIGQUIT, handle_shutdown_request_signal);
+	pqsignal(SIGTERM, handle_shutdown_request_signal);
+	pqsignal(SIGALRM, SIG_IGN);	/* ignored */
+	pqsignal(SIGPIPE, SIG_IGN);	/* ignored */
+	pqsignal(SIGUSR1, handle_file_check_request_signal);
+	pqsignal(SIGUSR2, dummy_handler);	/* unused, reserve for children */
+	pqsignal(SIGCHLD, handle_child_exit_signal);
 
-#ifdef SIGURG
+	/* This may configure SIGURG, depending on platform. */
+	InitializeLatchSupport();
+	InitLocalLatch();
 
-	/*
-	 * Ignore SIGURG for now.  Child processes may change this (see
-	 * InitializeLatchSupport), but they will not receive any such signals
-	 * until they wait on a latch.
-	 */
-	pqsignal_pm(SIGURG, SIG_IGN);	/* ignored */
-#endif
+	PG_SETMASK(&UnBlockSig);
 
 	/*
 	 * No other place in Postgres should touch SIGTTIN/SIGTTOU handling.  We
@@ -667,15 +653,15 @@ PostmasterMain(int argc, char *argv[])
 	 * child processes should just allow the inherited settings to stand.
 	 */
 #ifdef SIGTTIN
-	pqsignal_pm(SIGTTIN, SIG_IGN);	/* ignored */
+	pqsignal(SIGTTIN, SIG_IGN);	/* ignored */
 #endif
 #ifdef SIGTTOU
-	pqsignal_pm(SIGTTOU, SIG_IGN);	/* ignored */
+	pqsignal(SIGTTOU, SIG_IGN);	/* ignored */
 #endif
 
 	/* ignore SIGXFSZ, so that ulimit violations work like disk full */
 #ifdef SIGXFSZ
-	pqsignal_pm(SIGXFSZ, SIG_IGN);	/* ignored */
+	pqsignal(SIGXFSZ, SIG_IGN);	/* ignored */
 #endif
 
 	/*
@@ -1706,97 +1692,90 @@ DetermineSleepTime(struct timeval *timeout)
 static int
 ServerLoop(void)
 {
-	fd_set		readmask;
-	int			nSockets;
 	time_t		last_lockfile_recheck_time,
 				last_touch_time;
+	WaitEventSet *wes;
+	WaitEvent	events[MAXLISTEN];
+	int			nevents;
 
-	last_lockfile_recheck_time = last_touch_time = time(NULL);
+	/* Set up a WaitEventSet for our latch and listening sockets. */
+	MyLatch = GetPostmasterLatch();
+	OwnLatch(MyLatch);
+	wes = CreateWaitEventSet(CurrentMemoryContext, 1 + MAXLISTEN);
+	AddWaitEventToSet(wes, WL_LATCH_SET, PGINVALID_SOCKET, MyLatch, NULL);
+	for (int i = 0; i < MAXLISTEN; i++)
+	{
+		int			fd = ListenSocket[i];
 
-	nSockets = initMasks(&readmask);
+		if (fd == PGINVALID_SOCKET)
+			break;
+		AddWaitEventToSet(wes, WL_SOCKET_ACCEPT, fd, NULL, NULL);
+	}
+
+	last_lockfile_recheck_time = last_touch_time = time(NULL);
 
 	for (;;)
 	{
-		fd_set		rmask;
-		int			selres;
 		time_t		now;
 
 		/*
 		 * Wait for a connection request to arrive.
 		 *
-		 * We block all signals except while sleeping. That makes it safe for
-		 * signal handlers, which again block all signals while executing, to
-		 * do nontrivial work.
-		 *
 		 * If we are in PM_WAIT_DEAD_END state, then we don't want to accept
-		 * any new connections, so we don't call select(), and just sleep.
+		 * any new connections, so we just sleep.
 		 */
-		memcpy((char *) &rmask, (char *) &readmask, sizeof(fd_set));
-
 		if (pmState == PM_WAIT_DEAD_END)
 		{
-			PG_SETMASK(&UnBlockSig);
-
 			pg_usleep(100000L); /* 100 msec seems reasonable */
-			selres = 0;
-
-			PG_SETMASK(&BlockSig);
+			nevents = 0;
 		}
 		else
 		{
-			/* must set timeout each time; some OSes change it! */
 			struct timeval timeout;
 
-			/* Needs to run with blocked signals! */
 			DetermineSleepTime(&timeout);
 
-			PG_SETMASK(&UnBlockSig);
-
-			selres = select(nSockets, &rmask, NULL, NULL, &timeout);
-
-			PG_SETMASK(&BlockSig);
+			ModifyWaitEvent(wes, 0, WL_LATCH_SET, MyLatch);		/* XXX because address changes on crash! */
+			nevents = WaitEventSetWait(wes,
+									   timeout.tv_sec * 1000 + timeout.tv_usec / 1000,
+									   events,
+									   lengthof(events),
+									   0 /* postmaster posts no wait_events */);
 		}
 
-		/* Now check the select() result */
-		if (selres < 0)
-		{
-			if (errno != EINTR && errno != EWOULDBLOCK)
-			{
-				ereport(LOG,
-						(errcode_for_socket_access(),
-						 errmsg("select() failed in postmaster: %m")));
-				return STATUS_ERROR;
-			}
-		}
+		if (pending_reload_request)
+			process_reload_request();
+		if (pending_shutdown_request)
+			process_shutdown_request();
+		if (pending_child_exit)
+			process_child_exit();
 
 		/*
-		 * New connection pending on any of our sockets? If so, fork a child
-		 * process to deal with it.
+		 * Latch set, or new connection pending on any of our sockets? If so,
+		 * fork a child process to deal with it.
 		 */
-		if (selres > 0)
+		for (int i = 0; i < nevents; i++)
 		{
-			int			i;
-
-			for (i = 0; i < MAXLISTEN; i++)
+			if (events[i].events & WL_LATCH_SET)
 			{
-				if (ListenSocket[i] == PGINVALID_SOCKET)
-					break;
-				if (FD_ISSET(ListenSocket[i], &rmask))
+				ResetLatch(MyLatch);
+				process_backend_request();
+			}
+			else if (events[i].events & WL_SOCKET_ACCEPT)
+			{
+				Port	   *port;
+
+				port = ConnCreate(events[i].fd);
+				if (port)
 				{
-					Port	   *port;
+					BackendStartup(port);
 
-					port = ConnCreate(ListenSocket[i]);
-					if (port)
-					{
-						BackendStartup(port);
-
-						/*
-						 * We no longer need the open socket or port structure
-						 * in this process
-						 */
-						StreamClose(port->sock);
-						ConnFree(port);
-					}
+					/*
+					 * We no longer need the open socket or port structure
+					 * in this process
+					 */
+					StreamClose(port->sock);
+					ConnFree(port);
 				}
 			}
 		}
@@ -1939,34 +1918,6 @@ ServerLoop(void)
 	}
 }
 
-/*
- * Initialise the masks for select() for the ports we are listening on.
- * Return the number of sockets to listen on.
- */
-static int
-initMasks(fd_set *rmask)
-{
-	int			maxsock = -1;
-	int			i;
-
-	FD_ZERO(rmask);
-
-	for (i = 0; i < MAXLISTEN; i++)
-	{
-		int			fd = ListenSocket[i];
-
-		if (fd == PGINVALID_SOCKET)
-			break;
-		FD_SET(fd, rmask);
-
-		if (fd > maxsock)
-			maxsock = fd;
-	}
-
-	return maxsock + 1;
-}
-
-
 /*
  * Read a client's startup packet and do something according to it.
  *
@@ -2707,14 +2658,41 @@ InitProcessGlobals(void)
 #endif
 }
 
+/*
+ * pg_ctl uses SIGUSR1 to ask postmaster to check for logrotate and promote
+ * files.
+ */
+static void
+handle_file_check_request_signal(SIGNAL_ARGS)
+{
+	int save_errno = errno;
+
+	/* Schedule a check for both signal files. */
+	pending_logrotate_check_request = true;
+	pending_promote_check_request = true;
+	SetLatch(MyLatch);
+
+	errno = save_errno;
+}
+
+static void
+handle_reload_request_signal(SIGNAL_ARGS)
+{
+	int save_errno = errno;
+
+	pending_reload_request = true;
+	SetLatch(MyLatch);
+
+	errno = save_errno;
+}
 
 /*
- * SIGHUP -- reread config files, and tell children to do same
+ * Re-read config files, and tell children to do same.
  */
 static void
-SIGHUP_handler(SIGNAL_ARGS)
+process_reload_request(void)
 {
-	int			save_errno = errno;
+	pending_reload_request = false;
 
 	if (Shutdown <= SmartShutdown)
 	{
@@ -2771,27 +2749,46 @@ SIGHUP_handler(SIGNAL_ARGS)
 		write_nondefault_variables(PGC_SIGHUP);
 #endif
 	}
-
-	errno = save_errno;
 }
 
-
 /*
- * pmdie -- signal handler for processing various postmaster signals.
+ * Handler for the three shutdown signals.
  */
 static void
-pmdie(SIGNAL_ARGS)
+handle_shutdown_request_signal(SIGNAL_ARGS)
 {
-	int			save_errno = errno;
-
-	ereport(DEBUG2,
-			(errmsg_internal("postmaster received signal %d",
-							 postgres_signal_arg)));
+	int save_errno = errno;
 
 	switch (postgres_signal_arg)
 	{
 		case SIGTERM:
+			pending_shutdown_request = SmartShutdown;
+			break;
+		case SIGINT:
+			pending_shutdown_request = FastShutdown;
+			break;
+		case SIGQUIT:
+			pending_shutdown_request = ImmediateShutdown;
+			break;
+	}
+	SetLatch(MyLatch);
+
+	errno = save_errno;
+}
+
+/*
+ * Process shutdown request.
+ */
+static void
+process_shutdown_request(void)
+{
+	int		mode = pending_shutdown_request;
 
+	pending_shutdown_request = NoShutdown;
+
+	switch (mode)
+	{
+		case SmartShutdown:
 			/*
 			 * Smart Shutdown:
 			 *
@@ -2830,8 +2827,7 @@ pmdie(SIGNAL_ARGS)
 			PostmasterStateMachine();
 			break;
 
-		case SIGINT:
-
+		case FastShutdown:
 			/*
 			 * Fast Shutdown:
 			 *
@@ -2871,8 +2867,7 @@ pmdie(SIGNAL_ARGS)
 			PostmasterStateMachine();
 			break;
 
-		case SIGQUIT:
-
+		case ImmediateShutdown:
 			/*
 			 * Immediate Shutdown:
 			 *
@@ -2908,20 +2903,30 @@ pmdie(SIGNAL_ARGS)
 			PostmasterStateMachine();
 			break;
 	}
+}
+
+static void
+handle_child_exit_signal(SIGNAL_ARGS)
+{
+	int			save_errno = errno;
+
+	pending_child_exit = true;
+	SetLatch(MyLatch);
 
 	errno = save_errno;
 }
 
 /*
- * Reaper -- signal handler to cleanup after a child process dies.
+ * Cleanup after a child process dies.
  */
 static void
-reaper(SIGNAL_ARGS)
+process_child_exit(void)
 {
-	int			save_errno = errno;
 	int			pid;			/* process id of dead child process */
 	int			exitstatus;		/* its exit status */
 
+	pending_child_exit = false;
+
 	ereport(DEBUG4,
 			(errmsg_internal("reaping dead processes")));
 
@@ -3213,8 +3218,6 @@ reaper(SIGNAL_ARGS)
 	 * or actions to make.
 	 */
 	PostmasterStateMachine();
-
-	errno = save_errno;
 }
 
 /*
@@ -3642,8 +3645,9 @@ LogChildExit(int lev, const char *procname, int pid, int exitstatus)
 /*
  * Advance the postmaster's state machine and take actions as appropriate
  *
- * This is common code for pmdie(), reaper() and sigusr1_handler(), which
- * receive the signals that might mean we need to change state.
+ * This is common code for process_shutdown_request(), process_child_exit() and
+ * process_backend_request(), which process the signals and latches that might
+ * mean we need to change state.
  */
 static void
 PostmasterStateMachine(void)
@@ -3899,6 +3903,9 @@ PostmasterStateMachine(void)
 		/* re-create shared memory and semaphores */
 		CreateSharedMemoryAndSemaphores();
 
+		MyLatch = GetPostmasterLatch();
+		OwnLatch(MyLatch);
+
 		StartupPID = StartupDataBase();
 		Assert(StartupPID != 0);
 		StartupStatus = STARTUP_RUNNING;
@@ -4094,6 +4101,7 @@ BackendStartup(Port *port)
 	/* Hasn't asked to be notified about any bgworkers yet */
 	bn->bgworker_notify = false;
 
+	PG_SETMASK(&BlockSig);
 #ifdef EXEC_BACKEND
 	pid = backend_forkexec(port);
 #else							/* !EXEC_BACKEND */
@@ -4137,6 +4145,7 @@ BackendStartup(Port *port)
 		ereport(LOG,
 				(errmsg("could not fork new process for connection: %m")));
 		report_fork_failure_to_client(port, save_errno);
+		PG_SETMASK(&UnBlockSig);
 		return STATUS_ERROR;
 	}
 
@@ -4158,6 +4167,7 @@ BackendStartup(Port *port)
 		ShmemBackendArrayAdd(bn);
 #endif
 
+	PG_SETMASK(&UnBlockSig);
 	return STATUS_OK;
 }
 
@@ -5013,13 +5023,11 @@ ExitPostmaster(int status)
 }
 
 /*
- * sigusr1_handler - handle signal conditions from child processes
+ * Handle pmsignal conditions representing requests from backends.
  */
 static void
-sigusr1_handler(SIGNAL_ARGS)
+process_backend_request(void)
 {
-	int			save_errno = errno;
-
 	/*
 	 * RECOVERY_STARTED and BEGIN_HOT_STANDBY signals are ignored in
 	 * unexpected states. If the startup process quickly starts up, completes
@@ -5088,8 +5096,9 @@ sigusr1_handler(SIGNAL_ARGS)
 		maybe_start_bgworkers();
 
 	/* Tell syslogger to rotate logfile if requested */
-	if (SysLoggerPID != 0)
+	if (SysLoggerPID != 0 && pending_logrotate_check_request)
 	{
+		pending_logrotate_check_request = false;
 		if (CheckLogrotateSignal())
 		{
 			signal_child(SysLoggerPID, SIGUSR1);
@@ -5149,7 +5158,8 @@ sigusr1_handler(SIGNAL_ARGS)
 	if (StartupPID != 0 &&
 		(pmState == PM_STARTUP || pmState == PM_RECOVERY ||
 		 pmState == PM_HOT_STANDBY) &&
-		CheckPromoteSignal())
+		pending_promote_check_request &&
+		(pending_promote_check_request = false, CheckPromoteSignal()))
 	{
 		/*
 		 * Tell startup process to finish recovery.
@@ -5159,8 +5169,6 @@ sigusr1_handler(SIGNAL_ARGS)
 		 */
 		signal_child(StartupPID, SIGUSR2);
 	}
-
-	errno = save_errno;
 }
 
 /*
diff --git a/src/backend/storage/ipc/latch.c b/src/backend/storage/ipc/latch.c
index eb3a569aae..a27c6a1752 100644
--- a/src/backend/storage/ipc/latch.c
+++ b/src/backend/storage/ipc/latch.c
@@ -197,6 +197,8 @@ static void WaitEventAdjustWin32(WaitEventSet *set, WaitEvent *event);
 static inline int WaitEventSetWaitBlock(WaitEventSet *set, int cur_timeout,
 										WaitEvent *occurred_events, int nevents);
 
+static void WakeLatch(Latch *latch);
+
 /*
  * Initialize the process-local latch infrastructure.
  *
@@ -283,6 +285,17 @@ InitializeLatchSupport(void)
 #ifdef WAIT_USE_SIGNALFD
 	sigset_t	signalfd_mask;
 
+	if (IsUnderPostmaster)
+	{
+		if (signal_fd != -1)
+		{
+			/* Release postmaster's signal FD; ignore any error */
+			(void) close(signal_fd);
+			signal_fd = -1;
+			ReleaseExternalFD();
+		}
+	}
+
 	/* Block SIGURG, because we'll receive it through a signalfd. */
 	sigaddset(&UnBlockSig, SIGURG);
 
@@ -590,12 +603,6 @@ WaitLatchOrSocket(Latch *latch, int wakeEvents, pgsocket sock,
 void
 SetLatch(Latch *latch)
 {
-#ifndef WIN32
-	pid_t		owner_pid;
-#else
-	HANDLE		handle;
-#endif
-
 	/*
 	 * The memory barrier has to be placed here to ensure that any flag
 	 * variables possibly changed by this process have been flushed to main
@@ -613,6 +620,33 @@ SetLatch(Latch *latch)
 	if (!latch->maybe_sleeping)
 		return;
 
+	WakeLatch(latch);
+}
+
+/*
+ * A variant of SetLatch() used when waking the postmaster.  This skips the
+ * optimizations that normally avoid a system call if the owner isn't currently
+ * waiting or the latch is already set.  This is intended for waking the
+ * postmaster, which couldn't often benefit from such optimizations anyway
+ * because it spends its whole time waiting.  This should reduce opportunities
+ * for memory corruption to prevent the delivery of a wakeup.
+ */
+void
+SetLatchRobustly(Latch *latch)
+{
+	latch->is_set = true;
+	WakeLatch(latch);
+}
+
+static void
+WakeLatch(Latch *latch)
+{
+#ifndef WIN32
+	pid_t		owner_pid;
+#else
+	HANDLE		handle;
+#endif
+
 #ifndef WIN32
 
 	/*
@@ -1312,6 +1346,8 @@ WaitEventAdjustWin32(WaitEventSet *set, WaitEvent *event)
 			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)
 		{
@@ -2067,6 +2103,12 @@ WaitEventSetWaitBlock(WaitEventSet *set, int cur_timeout,
 			/* connected */
 			occurred_events->events |= WL_SOCKET_CONNECTED;
 		}
+		if ((cur_event->events & WL_SOCKET_ACCEPT) &&
+			(resEvents.lNetworkEvents & FD_ACCEPT))
+		{
+			/* connected */
+			occurred_events->events |= WL_SOCKET_ACCEPT;
+		}
 		if (resEvents.lNetworkEvents & FD_CLOSE)
 		{
 			/* EOF/error, so signal all caller-requested socket flags */
diff --git a/src/backend/storage/ipc/pmsignal.c b/src/backend/storage/ipc/pmsignal.c
index c85521d364..7a7bf593c4 100644
--- a/src/backend/storage/ipc/pmsignal.c
+++ b/src/backend/storage/ipc/pmsignal.c
@@ -24,13 +24,14 @@
 #include "miscadmin.h"
 #include "postmaster/postmaster.h"
 #include "replication/walsender.h"
+#include "storage/latch.h"
 #include "storage/pmsignal.h"
 #include "storage/shmem.h"
 #include "utils/memutils.h"
 
 
 /*
- * The postmaster is signaled by its children by sending SIGUSR1.  The
+ * The postmaster is signaled by its children by setting its latch.  The
  * specific reason is communicated via flags in shared memory.  We keep
  * a boolean flag for each possible "reason", so that different reasons
  * can be signaled by different backends at the same time.  (However,
@@ -70,17 +71,19 @@
 /* "typedef struct PMSignalData PMSignalData" appears in pmsignal.h */
 struct PMSignalData
 {
+	/* latch for waking postmaster */
+	Latch		pm_latch;
 	/* per-reason flags for signaling the postmaster */
-	sig_atomic_t PMSignalFlags[NUM_PMSIGNALS];
+	volatile sig_atomic_t PMSignalFlags[NUM_PMSIGNALS];
 	/* global flags for signals from postmaster to children */
-	QuitSignalReason sigquit_reason;	/* why SIGQUIT was sent */
+	volatile QuitSignalReason sigquit_reason;	/* why SIGQUIT was sent */
 	/* per-child-process flags */
 	int			num_child_flags;	/* # of entries in PMChildFlags[] */
-	sig_atomic_t PMChildFlags[FLEXIBLE_ARRAY_MEMBER];
+	volatile sig_atomic_t PMChildFlags[FLEXIBLE_ARRAY_MEMBER];
 };
 
 /* PMSignalState pointer is valid in both postmaster and child processes */
-NON_EXEC_STATIC volatile PMSignalData *PMSignalState = NULL;
+NON_EXEC_STATIC PMSignalData *PMSignalState = NULL;
 
 /*
  * These static variables are valid only in the postmaster.  We keep a
@@ -151,9 +154,10 @@ PMSignalShmemInit(void)
 	if (!found)
 	{
 		/* initialize all flags to zeroes */
-		MemSet(unvolatize(PMSignalData *, PMSignalState), 0, PMSignalShmemSize());
+		MemSet(PMSignalState, 0, PMSignalShmemSize());
 		num_child_inuse = MaxLivePostmasterChildren();
 		PMSignalState->num_child_flags = num_child_inuse;
+		InitSharedLatch(&PMSignalState->pm_latch);
 
 		/*
 		 * Also allocate postmaster's private PMChildInUse[] array.  We
@@ -186,13 +190,13 @@ SendPostmasterSignal(PMSignalReason reason)
 	/* Atomically set the proper flag */
 	PMSignalState->PMSignalFlags[reason] = true;
 	/* Send signal to postmaster */
-	kill(PostmasterPid, SIGUSR1);
+	SetLatchRobustly(&PMSignalState->pm_latch);
 }
 
 /*
  * CheckPostmasterSignal - check to see if a particular reason has been
  * signaled, and clear the signal flag.  Should be called by postmaster
- * after receiving SIGUSR1.
+ * after its latch is set.
  */
 bool
 CheckPostmasterSignal(PMSignalReason reason)
@@ -206,6 +210,12 @@ CheckPostmasterSignal(PMSignalReason reason)
 	return false;
 }
 
+Latch *
+GetPostmasterLatch(void)
+{
+	return &PMSignalState->pm_latch;
+}
+
 /*
  * SetQuitSignalReason - broadcast the reason for a system shutdown.
  * Should be called by postmaster before sending SIGQUIT to children.
diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index eb1046450b..1348261220 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -135,8 +135,7 @@ InitPostmasterChild(void)
 
 	/* Initialize process-local latch support */
 	InitializeLatchSupport();
-	MyLatch = &LocalLatchData;
-	InitLatch(MyLatch);
+	InitLocalLatch();
 	InitializeLatchWaitSet();
 
 	/*
@@ -189,8 +188,7 @@ InitStandaloneProcess(const char *argv0)
 
 	/* Initialize process-local latch support */
 	InitializeLatchSupport();
-	MyLatch = &LocalLatchData;
-	InitLatch(MyLatch);
+	InitLocalLatch();
 	InitializeLatchWaitSet();
 
 	/*
@@ -232,6 +230,13 @@ SwitchToSharedLatch(void)
 	SetLatch(MyLatch);
 }
 
+void
+InitLocalLatch(void)
+{
+	MyLatch = &LocalLatchData;
+	InitLatch(MyLatch);
+}
+
 void
 SwitchBackToLocalLatch(void)
 {
diff --git a/src/include/libpq/pqsignal.h b/src/include/libpq/pqsignal.h
index 7890b426a8..76eb380a4f 100644
--- a/src/include/libpq/pqsignal.h
+++ b/src/include/libpq/pqsignal.h
@@ -53,7 +53,4 @@ extern PGDLLIMPORT sigset_t StartupBlockSig;
 
 extern void pqinitmask(void);
 
-/* pqsigfunc is declared in src/include/port.h */
-extern pqsigfunc pqsignal_pm(int signo, pqsigfunc func);
-
 #endif							/* PQSIGNAL_H */
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 795182fa51..0975867197 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -310,6 +310,7 @@ extern PGDLLIMPORT char *DatabasePath;
 /* now in utils/init/miscinit.c */
 extern void InitPostmasterChild(void);
 extern void InitStandaloneProcess(const char *argv0);
+extern void InitLocalLatch(void);
 extern void SwitchToSharedLatch(void);
 extern void SwitchBackToLocalLatch(void);
 
diff --git a/src/include/storage/latch.h b/src/include/storage/latch.h
index 68ab740f16..2e8b64cef2 100644
--- a/src/include/storage/latch.h
+++ b/src/include/storage/latch.h
@@ -135,10 +135,16 @@ typedef struct Latch
 #define WL_SOCKET_CONNECTED  WL_SOCKET_WRITEABLE
 #endif
 #define WL_SOCKET_CLOSED 	 (1 << 7)
+#ifdef WIN32
+#define WL_SOCKET_ACCEPT	 (1 << 8)
+#else
+#define WL_SOCKET_ACCEPT	 WL_SOCKET_READABLE
+#endif
 #define WL_SOCKET_MASK		(WL_SOCKET_READABLE | \
 							 WL_SOCKET_WRITEABLE | \
 							 WL_SOCKET_CONNECTED | \
-							 WL_SOCKET_CLOSED)
+							 WL_SOCKET_CLOSED | \
+							 WL_SOCKET_ACCEPT)
 
 typedef struct WaitEvent
 {
@@ -163,6 +169,7 @@ extern void InitSharedLatch(Latch *latch);
 extern void OwnLatch(Latch *latch);
 extern void DisownLatch(Latch *latch);
 extern void SetLatch(Latch *latch);
+extern void SetLatchRobustly(Latch *latch);
 extern void ResetLatch(Latch *latch);
 extern void ShutdownLatchSupport(void);
 
diff --git a/src/include/storage/pmsignal.h b/src/include/storage/pmsignal.h
index 58f4ddf476..ba657f716b 100644
--- a/src/include/storage/pmsignal.h
+++ b/src/include/storage/pmsignal.h
@@ -24,6 +24,8 @@
 #include "sys/procctl.h"
 #endif
 
+struct Latch;
+
 /*
  * Reasons for signaling the postmaster.  We can cope with simultaneous
  * signals for different reasons.  If the same reason is signaled multiple
@@ -74,6 +76,7 @@ extern void MarkPostmasterChildInactive(void);
 extern void MarkPostmasterChildWalSender(void);
 extern bool PostmasterIsAliveInternal(void);
 extern void PostmasterDeathSignalInit(void);
+extern struct Latch *GetPostmasterLatch(void);
 
 
 /*
-- 
2.38.1

Reply via email to