On Wed, Mar 31, 2021 at 7:02 PM Thomas Munro <thomas.mu...@gmail.com> wrote:
> On Fri, Mar 12, 2021 at 7:55 PM Thomas Munro <thomas.mu...@gmail.com> wrote:
> > On Thu, Mar 11, 2021 at 7:34 PM Michael Paquier <mich...@paquier.xyz> wrote:
> > > Wow.  This probably means that we would be able to get rid of
> > > USE_POSTMASTER_DEATH_SIGNAL?
> >
> > We'll still need it, because there'd still be systems with no signal:
> > NetBSD, OpenBSD, AIX, HPUX, illumos.
>
> Erm, and of course macOS.
>
> There is actually something we could do here: ioctl(I_SETSIG) for
> SysV-derived systems and fcntl(O_ASYNC) for BSD-derived systems seems
> like a promising way to get a SIGIO signal when the postmaster goes
> away and the pipe becomes readable.  Previously I'd discounted this,
> because it's not in POSIX and I doubted it would work well on other
> systems.  But I was flicking through Stevens' UNIX book while trying
> to figure out that POLLHUP stuff from a nearby thread (though it's
> useless for that purpose) and I learned from section 12.6 that SIGIO
> is fairly ancient, originating in 4.2BSD, adopted by SVR4, so it's
> likely present in quite a few systems, maybe even all of our support
> platforms if we're prepared to do it two different ways.  Just a
> thought.

Alright, here's a proof-of-concept patch that does that.  Adding to the next CF.

This seems to work on Linux, macOS, FreeBSD and OpenBSD (and I assume
any other BSD).  Can anyone tell me if it works on illumos, AIX or
HPUX, and if not, how to fix it or disable the feature gracefully?
For now the patch assumes that if you have SIGIO then you can do this;
perhaps it should also test for O_ASYNC?  Perhaps HPUX has the signal
but requires a different incantation with I_SETSIG?

Full disclosure: The reason for my interest in this subject is that I
have a work-in-progress patch set to make latches, locks and condition
variables more efficient using futexes on several OSes, but it needs a
signal to wake on postmaster death.
From acff1b99d464eca453cdde3f5ca8079c925e47ac Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.mu...@gmail.com>
Date: Fri, 9 Apr 2021 16:00:07 +1200
Subject: [PATCH] Use SIGIO to detect postmaster death.

Previously we used non-POSIX calls prctl/procctl to requests a signal on
death of our parent process on Linux and FreeBSD.  Instead, use O_ASYNC
to request SIGIO when the pipe is closed by the postmaster.  The effect
is about the same, but it should work on many other Unix systems too.
It's not in POSIX either, so it remains optional.

Discussion: https://postgr.es/m/CA%2BhUKGJiejg%3DGPTkf3S53N0-Vr4fOQ4wev7DmAVVLHbh%3DO9%2Bdg%40mail.gmail.com
---
 configure                          |  2 +-
 configure.ac                       |  2 --
 src/backend/storage/ipc/pmsignal.c | 41 ++++++++----------------------
 src/include/pg_config.h.in         |  6 -----
 src/include/storage/pmsignal.h     | 11 +-------
 src/tools/msvc/Solution.pm         |  2 --
 6 files changed, 12 insertions(+), 52 deletions(-)

diff --git a/configure b/configure
index 70f4555264..9b69fb203a 100755
--- a/configure
+++ b/configure
@@ -13345,7 +13345,7 @@ $as_echo "#define HAVE_STDBOOL_H 1" >>confdefs.h
 fi
 
 
-for ac_header in atomic.h copyfile.h execinfo.h getopt.h ifaddrs.h langinfo.h mbarrier.h poll.h sys/epoll.h sys/event.h sys/ipc.h sys/prctl.h sys/procctl.h sys/pstat.h sys/resource.h sys/select.h sys/sem.h sys/shm.h sys/sockio.h sys/tas.h sys/uio.h sys/un.h termios.h ucred.h wctype.h
+for ac_header in atomic.h copyfile.h execinfo.h getopt.h ifaddrs.h langinfo.h mbarrier.h poll.h sys/epoll.h sys/event.h sys/ipc.h sys/pstat.h sys/resource.h sys/select.h sys/sem.h sys/shm.h sys/sockio.h sys/tas.h sys/uio.h sys/un.h termios.h ucred.h wctype.h
 do :
   as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh`
 ac_fn_c_check_header_mongrel "$LINENO" "$ac_header" "$as_ac_Header" "$ac_includes_default"
diff --git a/configure.ac b/configure.ac
index ba67c95bcc..f08c60acd6 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1360,8 +1360,6 @@ AC_CHECK_HEADERS(m4_normalize([
 	sys/epoll.h
 	sys/event.h
 	sys/ipc.h
-	sys/prctl.h
-	sys/procctl.h
 	sys/pstat.h
 	sys/resource.h
 	sys/select.h
diff --git a/src/backend/storage/ipc/pmsignal.c b/src/backend/storage/ipc/pmsignal.c
index 280c2395c9..e9f5b4fc41 100644
--- a/src/backend/storage/ipc/pmsignal.c
+++ b/src/backend/storage/ipc/pmsignal.c
@@ -14,6 +14,7 @@
  */
 #include "postgres.h"
 
+#include <fcntl.h>
 #include <signal.h>
 #include <unistd.h>
 
@@ -92,23 +93,6 @@ postmaster_death_handler(int signo)
 {
 	postmaster_possibly_dead = true;
 }
-
-/*
- * The available signals depend on the OS.  SIGUSR1 and SIGUSR2 are already
- * used for other things, so choose another one.
- *
- * Currently, we assume that we can always find a signal to use.  That
- * seems like a reasonable assumption for all platforms that are modern
- * enough to have a parent-death signaling mechanism.
- */
-#if defined(SIGINFO)
-#define POSTMASTER_DEATH_SIGNAL SIGINFO
-#elif defined(SIGPWR)
-#define POSTMASTER_DEATH_SIGNAL SIGPWR
-#else
-#error "cannot find a signal to use for postmaster death"
-#endif
-
 #endif							/* USE_POSTMASTER_DEATH_SIGNAL */
 
 /*
@@ -405,21 +389,16 @@ void
 PostmasterDeathSignalInit(void)
 {
 #ifdef USE_POSTMASTER_DEATH_SIGNAL
-	int			signum = POSTMASTER_DEATH_SIGNAL;
-
 	/* Register our signal handler. */
-	pqsignal(signum, postmaster_death_handler);
-
-	/* Request a signal on parent exit. */
-#if defined(PR_SET_PDEATHSIG)
-	if (prctl(PR_SET_PDEATHSIG, signum) < 0)
-		elog(ERROR, "could not request parent death signal: %m");
-#elif defined(PROC_PDEATHSIG_CTL)
-	if (procctl(P_PID, 0, PROC_PDEATHSIG_CTL, &signum) < 0)
-		elog(ERROR, "could not request parent death signal: %m");
-#else
-#error "USE_POSTMASTER_DEATH_SIGNAL set, but there is no mechanism to request the signal"
-#endif
+	pqsignal(SIGIO, postmaster_death_handler);
+
+	/* Ask for SIGIO if the pipe becomes readable, indicating it was closed. */
+	if (fcntl(postmaster_alive_fds[POSTMASTER_FD_WATCH],
+			  F_SETOWN, MyProcPid) < 0)
+		elog(ERROR, "could not own postmaster pipe: %m");
+	if (fcntl(postmaster_alive_fds[POSTMASTER_FD_WATCH],
+			  F_SETFL, O_ASYNC | O_NONBLOCK) < 0)
+		elog(ERROR, "could not enable SIGIO on postmaster pipe: %m");
 
 	/*
 	 * Just in case the parent was gone already and we missed it, we'd better
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 783b8fc1ba..13f99b36f1 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -620,12 +620,6 @@
 /* Define to 1 if you have the <sys/ipc.h> header file. */
 #undef HAVE_SYS_IPC_H
 
-/* Define to 1 if you have the <sys/prctl.h> header file. */
-#undef HAVE_SYS_PRCTL_H
-
-/* Define to 1 if you have the <sys/procctl.h> header file. */
-#undef HAVE_SYS_PROCCTL_H
-
 /* Define to 1 if you have the <sys/pstat.h> header file. */
 #undef HAVE_SYS_PSTAT_H
 
diff --git a/src/include/storage/pmsignal.h b/src/include/storage/pmsignal.h
index 8ed4d87ae6..7ab00c4024 100644
--- a/src/include/storage/pmsignal.h
+++ b/src/include/storage/pmsignal.h
@@ -16,14 +16,6 @@
 
 #include <signal.h>
 
-#ifdef HAVE_SYS_PRCTL_H
-#include "sys/prctl.h"
-#endif
-
-#ifdef HAVE_SYS_PROCCTL_H
-#include "sys/procctl.h"
-#endif
-
 /*
  * Reasons for signaling the postmaster.  We can cope with simultaneous
  * signals for different reasons.  If the same reason is signaled multiple
@@ -83,8 +75,7 @@ extern void PostmasterDeathSignalInit(void);
  * the parent dies.  Checking the flag first makes PostmasterIsAlive() a lot
  * cheaper in usual case that the postmaster is alive.
  */
-#if (defined(HAVE_SYS_PRCTL_H) && defined(PR_SET_PDEATHSIG)) || \
-	(defined(HAVE_SYS_PROCCTL_H) && defined(PROC_PDEATHSIG_CTL))
+#if defined(SIGIO)
 #define USE_POSTMASTER_DEATH_SIGNAL
 #endif
 
diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm
index d2bc7abef0..b4130dddaf 100644
--- a/src/tools/msvc/Solution.pm
+++ b/src/tools/msvc/Solution.pm
@@ -397,8 +397,6 @@ sub GenerateFiles
 		HAVE_SYS_EPOLL_H                         => undef,
 		HAVE_SYS_EVENT_H                         => undef,
 		HAVE_SYS_IPC_H                           => undef,
-		HAVE_SYS_PRCTL_H                         => undef,
-		HAVE_SYS_PROCCTL_H                       => undef,
 		HAVE_SYS_PSTAT_H                         => undef,
 		HAVE_SYS_RESOURCE_H                      => undef,
 		HAVE_SYS_SELECT_H                        => undef,
-- 
2.30.1

Reply via email to