Centralize setup of SIGQUIT handling for postmaster child processes.

We decided that the policy established in commit 7634bd4f6 for
the bgwriter, checkpointer, walwriter, and walreceiver processes,
namely that they should accept SIGQUIT at all times, really ought
to apply uniformly to all postmaster children.  Therefore, get
rid of the duplicative and inconsistent per-process code for
establishing that signal handler and removing SIGQUIT from BlockSig.
Instead, make InitPostmasterChild do it.

The handler set up by InitPostmasterChild is SignalHandlerForCrashExit,
which just summarily does _exit(2).  In interactive backends, we
almost immediately replace that with quickdie, since we would prefer
to try to tell the client that we're dying.  However, this patch is
changing the behavior of autovacuum (both launcher and workers), as
well as walsenders.  Those processes formerly also used quickdie,
but AFAICS that was just mindless copy-and-paste: they don't have
any interactive client that's likely to benefit from being told this.

The stats collector continues to be an outlier, in that it thinks
SIGQUIT means normal exit.  That should probably be changed for
consistency, but there's another patch set where that's being
dealt with, so I didn't do so here.

Discussion: https://postgr.es/m/644875.1599933441@sss.pgh.pa.us

diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 19ba26b914e96cc5fa9828910137ca89d0c4ad4f..2cef56f115f4e3265eeb7f07a44fefa1545a7cf1 100644 (file)
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -454,8 +454,8 @@ AutoVacLauncherMain(int argc, char *argv[])
    pqsignal(SIGHUP, SignalHandlerForConfigReload);
    pqsignal(SIGINT, StatementCancelHandler);
    pqsignal(SIGTERM, SignalHandlerForShutdownRequest);
+   /* SIGQUIT handler was already set up by InitPostmasterChild */
 
-   pqsignal(SIGQUIT, quickdie);
    InitializeTimeouts();       /* establishes SIGALRM handler */
 
    pqsignal(SIGPIPE, SIG_IGN);
@@ -498,9 +498,10 @@ AutoVacLauncherMain(int argc, char *argv[])
     *
     * Note that we use sigsetjmp(..., 1), so that the prevailing signal mask
     * (to wit, BlockSig) will be restored when longjmp'ing to here.  Thus,
-    * signals will be blocked until we complete error recovery.  It might
-    * seem that this policy makes the HOLD_INTERRUPTS() call redundant, but
-    * it is not since InterruptPending might be set already.
+    * signals other than SIGQUIT will be blocked until we complete error
+    * recovery.  It might seem that this policy makes the HOLD_INTERRUPTS()
+    * call redundant, but it is not since InterruptPending might be set
+    * already.
     */
    if (sigsetjmp(local_sigjmp_buf, 1) != 0)
    {
@@ -1531,7 +1532,8 @@ AutoVacWorkerMain(int argc, char *argv[])
     */
    pqsignal(SIGINT, StatementCancelHandler);
    pqsignal(SIGTERM, die);
-   pqsignal(SIGQUIT, quickdie);
+   /* SIGQUIT handler was already set up by InitPostmasterChild */
+
    InitializeTimeouts();       /* establishes SIGALRM handler */
 
    pqsignal(SIGPIPE, SIG_IGN);
@@ -1562,9 +1564,9 @@ AutoVacWorkerMain(int argc, char *argv[])
     *
     * Note that we use sigsetjmp(..., 1), so that the prevailing signal mask
     * (to wit, BlockSig) will be restored when longjmp'ing to here.  Thus,
-    * signals will be blocked until we exit.  It might seem that this policy
-    * makes the HOLD_INTERRUPTS() call redundant, but it is not since
-    * InterruptPending might be set already.
+    * signals other than SIGQUIT will be blocked until we exit.  It might
+    * seem that this policy makes the HOLD_INTERRUPTS() call redundant, but
+    * it is not since InterruptPending might be set already.
     */
    if (sigsetjmp(local_sigjmp_buf, 1) != 0)
    {

diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c
index d043ced6861a4ab6906c9868d3a5c21f357b99b4..5a9a0e34353767e68587a53e6d980f1f853c0fa5 100644 (file)
--- a/src/backend/postmaster/bgworker.c
+++ b/src/backend/postmaster/bgworker.c
@@ -731,9 +731,9 @@ StartBackgroundWorker(void)
        pqsignal(SIGFPE, SIG_IGN);
    }
    pqsignal(SIGTERM, bgworker_die);
+   /* SIGQUIT handler was already set up by InitPostmasterChild */
    pqsignal(SIGHUP, SIG_IGN);
 
-   pqsignal(SIGQUIT, SignalHandlerForCrashExit);
    InitializeTimeouts();       /* establishes SIGALRM handler */
 
    pqsignal(SIGPIPE, SIG_IGN);

diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bgwriter.c
index c96568149fe03e7cd75ea4ea1aa1717a4f5d9780..a7afa758b618d4c5519b24d24b5572e4d0ee0b0c 100644 (file)
--- a/src/backend/postmaster/bgwriter.c
+++ b/src/backend/postmaster/bgwriter.c
@@ -104,7 +104,7 @@ BackgroundWriterMain(void)
    pqsignal(SIGHUP, SignalHandlerForConfigReload);
    pqsignal(SIGINT, SIG_IGN);
    pqsignal(SIGTERM, SignalHandlerForShutdownRequest);
-   pqsignal(SIGQUIT, SignalHandlerForCrashExit);
+   /* SIGQUIT handler was already set up by InitPostmasterChild */
    pqsignal(SIGALRM, SIG_IGN);
    pqsignal(SIGPIPE, SIG_IGN);
    pqsignal(SIGUSR1, procsignal_sigusr1_handler);
@@ -115,10 +115,6 @@ BackgroundWriterMain(void)
     */
    pqsignal(SIGCHLD, SIG_DFL);
 
-   /* We allow SIGQUIT (SignalHandlerForCrashExit) at all times */
-   sigdelset(&BlockSig, SIGQUIT);
-   PG_SETMASK(&BlockSig);
-
    /*
     * We just started, assume there has been either a shutdown or
     * end-of-recovery snapshot.

diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index 45f5deca72ee88c022c5370dbc9fde50ec83b055..3e7dcd4f764da93f08718da4f9c914106c22e13e 100644 (file)
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -198,7 +198,7 @@ CheckpointerMain(void)
    pqsignal(SIGHUP, SignalHandlerForConfigReload);
    pqsignal(SIGINT, ReqCheckpointHandler); /* request checkpoint */
    pqsignal(SIGTERM, SIG_IGN); /* ignore SIGTERM */
-   pqsignal(SIGQUIT, SignalHandlerForCrashExit);
+   /* SIGQUIT handler was already set up by InitPostmasterChild */
    pqsignal(SIGALRM, SIG_IGN);
    pqsignal(SIGPIPE, SIG_IGN);
    pqsignal(SIGUSR1, procsignal_sigusr1_handler);
@@ -209,10 +209,6 @@ CheckpointerMain(void)
     */
    pqsignal(SIGCHLD, SIG_DFL);
 
-   /* We allow SIGQUIT (SignalHandlerForCrashExit) at all times */
-   sigdelset(&BlockSig, SIGQUIT);
-   PG_SETMASK(&BlockSig);
-
    /*
     * Initialize so that first time-driven event happens at the correct time.
     */

diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index 37be0e2bbbe7155b7410c335ea220f972fb40f2c..ed1b65358df8534a82b42da0a2463eeafad6dc7a 100644 (file)
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -228,7 +228,7 @@ PgArchiverMain(int argc, char *argv[])
    pqsignal(SIGHUP, SignalHandlerForConfigReload);
    pqsignal(SIGINT, SIG_IGN);
    pqsignal(SIGTERM, SignalHandlerForShutdownRequest);
-   pqsignal(SIGQUIT, SignalHandlerForCrashExit);
+   /* SIGQUIT handler was already set up by InitPostmasterChild */
    pqsignal(SIGALRM, SIG_IGN);
    pqsignal(SIGPIPE, SIG_IGN);
    pqsignal(SIGUSR1, pgarch_waken);

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 3cd6fa30eb0a050f5af0ee0e6c8baea6aa3642ee..959e3b88738189cc6bad5b975207a8d60e9ca959 100644 (file)
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -4355,7 +4355,7 @@ BackendInitialize(Port *port)
     * cleaned up.
     */
    pqsignal(SIGTERM, process_startup_packet_die);
-   pqsignal(SIGQUIT, SignalHandlerForCrashExit);
+   /* SIGQUIT handler was already set up by InitPostmasterChild */
    InitializeTimeouts();       /* establishes SIGALRM handler */
    PG_SETMASK(&StartupBlockSig);
 
@@ -4435,7 +4435,7 @@ BackendInitialize(Port *port)
    status = ProcessStartupPacket(port, false, false);
 
    /*
-    * Disable the timeout, and prevent SIGTERM/SIGQUIT again.
+    * Disable the timeout, and prevent SIGTERM again.
     */
    disable_timeout(STARTUP_PACKET_TIMEOUT, false);
    PG_SETMASK(&BlockSig);
@@ -4983,10 +4983,6 @@ SubPostmasterMain(int argc, char *argv[])
    if (strcmp(argv[1], "--forkavworker") == 0)
        AutovacuumWorkerIAm();
 
-   /* In EXEC_BACKEND case we will not have inherited these settings */
-   pqinitmask();
-   PG_SETMASK(&BlockSig);
-
    /* Read in remaining GUC variables */
    read_nondefault_variables();
 

diff --git a/src/backend/postmaster/startup.c b/src/backend/postmaster/startup.c
index fd9ac35dac1ffb924de1bc517c6a7c69932fafa0..64af7b8707cc6fcf89c346b088b621f3f851f111 100644 (file)
--- a/src/backend/postmaster/startup.c
+++ b/src/backend/postmaster/startup.c
@@ -175,7 +175,7 @@ StartupProcessMain(void)
    pqsignal(SIGHUP, StartupProcSigHupHandler); /* reload config file */
    pqsignal(SIGINT, SIG_IGN);  /* ignore query cancel */
    pqsignal(SIGTERM, StartupProcShutdownHandler);  /* request shutdown */
-   pqsignal(SIGQUIT, SignalHandlerForCrashExit);
+   /* SIGQUIT handler was already set up by InitPostmasterChild */
    InitializeTimeouts();       /* establishes SIGALRM handler */
    pqsignal(SIGPIPE, SIG_IGN);
    pqsignal(SIGUSR1, procsignal_sigusr1_handler);

diff --git a/src/backend/postmaster/walwriter.c b/src/backend/postmaster/walwriter.c
index 358c0916ac2341ca8d525e47c1ff6c8bcd070be7..a52832fe900aac6a5422100298120071ed0a8ea9 100644 (file)
--- a/src/backend/postmaster/walwriter.c
+++ b/src/backend/postmaster/walwriter.c
@@ -101,7 +101,7 @@ WalWriterMain(void)
    pqsignal(SIGHUP, SignalHandlerForConfigReload);
    pqsignal(SIGINT, SignalHandlerForShutdownRequest);
    pqsignal(SIGTERM, SignalHandlerForShutdownRequest);
-   pqsignal(SIGQUIT, SignalHandlerForCrashExit);
+   /* SIGQUIT handler was already set up by InitPostmasterChild */
    pqsignal(SIGALRM, SIG_IGN);
    pqsignal(SIGPIPE, SIG_IGN);
    pqsignal(SIGUSR1, procsignal_sigusr1_handler);
@@ -112,10 +112,6 @@ WalWriterMain(void)
     */
    pqsignal(SIGCHLD, SIG_DFL);
 
-   /* We allow SIGQUIT (SignalHandlerForCrashExit) at all times */
-   sigdelset(&BlockSig, SIGQUIT);
-   PG_SETMASK(&BlockSig);
-
    /*
     * Create a memory context that we will do all our work in.  We do this so
     * that we can reset the context during error recovery and thereby avoid

diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
index b180598507f1ccc0918768d47667c54c0a41088b..17f1a49f8711f2d63d5a18f407e59e889008d1bc 100644 (file)
--- a/src/backend/replication/walreceiver.c
+++ b/src/backend/replication/walreceiver.c
@@ -270,7 +270,7 @@ WalReceiverMain(void)
    pqsignal(SIGHUP, WalRcvSigHupHandler);  /* set flag to read config file */
    pqsignal(SIGINT, SIG_IGN);
    pqsignal(SIGTERM, WalRcvShutdownHandler);   /* request shutdown */
-   pqsignal(SIGQUIT, SignalHandlerForCrashExit);
+   /* SIGQUIT handler was already set up by InitPostmasterChild */
    pqsignal(SIGALRM, SIG_IGN);
    pqsignal(SIGPIPE, SIG_IGN);
    pqsignal(SIGUSR1, procsignal_sigusr1_handler);
@@ -279,10 +279,6 @@ WalReceiverMain(void)
    /* Reset some signals that are accepted by postmaster but not here */
    pqsignal(SIGCHLD, SIG_DFL);
 
-   /* We allow SIGQUIT (SignalHandlerForCrashExit) at all times */
-   sigdelset(&BlockSig, SIGQUIT);
-   PG_SETMASK(&BlockSig);
-
    /* Load the libpq-specific functions */
    load_file("libpqwalreceiver", false);
    if (WalReceiverFunctions == NULL)

diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 67093383e66bfb4a335bb7101194ffdd666d3ee8..4dbffea240a38db2c9e5eed9ce5ec8506a1877f0 100644 (file)
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -3041,7 +3041,7 @@ WalSndSignals(void)
    pqsignal(SIGHUP, SignalHandlerForConfigReload);
    pqsignal(SIGINT, StatementCancelHandler);   /* query cancel */
    pqsignal(SIGTERM, die);     /* request shutdown */
-   pqsignal(SIGQUIT, quickdie);    /* hard crash time */
+   /* SIGQUIT handler was already set up by InitPostmasterChild */
    InitializeTimeouts();       /* establishes SIGALRM handler */
    pqsignal(SIGPIPE, SIG_IGN);
    pqsignal(SIGUSR1, procsignal_sigusr1_handler);

diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index c9424f167c8d9c40b244d695020a0716e4a08c2f..411cfadbff35bfff41b7d9b2fb6d240b1a5fb0f9 100644 (file)
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -3820,7 +3820,8 @@ PostgresMain(int argc, char *argv[],
    }
 
    /*
-    * Set up signal handlers and masks.
+    * Set up signal handlers.  (InitPostmasterChild or InitStandaloneProcess
+    * has already set up BlockSig and made that the active signal mask.)
     *
     * Note that postmaster blocked all signals before forking child process,
     * so there is no race condition whereby we might receive a signal before
@@ -3842,6 +3843,9 @@ PostgresMain(int argc, char *argv[],
        pqsignal(SIGTERM, die); /* cancel current query and exit */
 
        /*
+        * In a postmaster child backend, replace SignalHandlerForCrashExit
+        * with quickdie, so we can tell the client we're dying.
+        *
         * In a standalone backend, SIGQUIT can be generated from the keyboard
         * easily, while SIGTERM cannot, so we make both signals do die()
         * rather than quickdie().
@@ -3871,16 +3875,6 @@ PostgresMain(int argc, char *argv[],
                                     * platforms */
    }
 
-   pqinitmask();
-
-   if (IsUnderPostmaster)
-   {
-       /* We allow SIGQUIT (quickdie) at all times */
-       sigdelset(&BlockSig, SIGQUIT);
-   }
-
-   PG_SETMASK(&BlockSig);      /* block everything except SIGQUIT */
-
    if (!IsUnderPostmaster)
    {
        /*

diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index cf8f9579c345f2d674d3d3b10aa05265725bbd65..ed2ab4b5b29a8088724ee1c19d8eb5fed45ba830 100644 (file)
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -32,10 +32,12 @@
 #include "catalog/pg_authid.h"
 #include "common/file_perm.h"
 #include "libpq/libpq.h"
+#include "libpq/pqsignal.h"
 #include "mb/pg_wchar.h"
 #include "miscadmin.h"
 #include "pgstat.h"
 #include "postmaster/autovacuum.h"
+#include "postmaster/interrupt.h"
 #include "postmaster/postmaster.h"
 #include "storage/fd.h"
 #include "storage/ipc.h"
@@ -133,6 +135,23 @@ InitPostmasterChild(void)
        elog(FATAL, "setsid() failed: %m");
 #endif
 
+   /* In EXEC_BACKEND case we will not have inherited BlockSig etc values */
+#ifdef EXEC_BACKEND
+   pqinitmask();
+#endif
+
+   /*
+    * Every postmaster child process is expected to respond promptly to
+    * SIGQUIT at all times.  Therefore we centrally remove SIGQUIT from
+    * BlockSig and install a suitable signal handler.  (Client-facing
+    * processes may choose to replace this default choice of handler with
+    * quickdie().)  All other blockable signals remain blocked for now.
+    */
+   pqsignal(SIGQUIT, SignalHandlerForCrashExit);
+
+   sigdelset(&BlockSig, SIGQUIT);
+   PG_SETMASK(&BlockSig);
+
    /* Request a signal if the postmaster dies, if possible. */
    PostmasterDeathSignalInit();
 }
@@ -155,6 +174,13 @@ InitStandaloneProcess(const char *argv0)
    InitLatch(MyLatch);
    InitializeLatchWaitSet();
 
+   /*
+    * For consistency with InitPostmasterChild, initialize signal mask here.
+    * But we don't unblock SIGQUIT or provide a default handler for it.
+    */
+   pqinitmask();
+   PG_SETMASK(&BlockSig);
+
    /* Compute paths, no postmaster to inherit from */
    if (my_exec_path[0] == '\0')
    {