Unify SIGHUP handling between normal and walsender backends.

Because walsender and normal backends share the same main loop it's
problematic to have two different flag variables, set in signal
handlers, indicating a pending configuration reload.  Only certain
walsender commands reach code paths checking for the
variable (START_[LOGICAL_]REPLICATION, CREATE_REPLICATION_SLOT
... LOGICAL, notably not base backups).

This is a bug present since the introduction of walsender, but has
gotten worse in releases since then which allow walsender to do more.

A later patch, not slated for v10, will similarly unify SIGHUP
handling in other types of processes as well.

Author: Petr Jelinek, Andres Freund
Reviewed-By: Michael Paquier
Discussion: https://postgr.es/m/20170423235941[email protected]
Backpatch: 9.2-, bug is present since 9.0

diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 94da622f057ccbc9771997b3c1bf4d1dd73f86ca..2798e69e96f562db88c3905dedd8f83a898b6b1e 100644 (file)
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -109,12 +109,10 @@ static StringInfoData reply_message;
 static TimestampTz last_reply_timestamp;
 
 /* Flags set by signal handlers for later service in main loop */
-static volatile sig_atomic_t got_SIGHUP = false;
 volatile sig_atomic_t walsender_shutdown_requested = false;
 volatile sig_atomic_t walsender_ready_to_stop = false;
 
 /* Signal handlers */
-static void WalSndSigHupHandler(SIGNAL_ARGS);
 static void WalSndShutdownHandler(SIGNAL_ARGS);
 static void WalSndQuickDieHandler(SIGNAL_ARGS);
 static void WalSndXLogSendHandler(SIGNAL_ARGS);
@@ -237,10 +235,11 @@ WalSndHandshake(void)
         * Check for any other interesting events that happened while we
         * slept.
         */
-       if (got_SIGHUP)
+       if (ConfigReloadPending)
        {
-           got_SIGHUP = false;
+           ConfigReloadPending = false;
            ProcessConfigFile(PGC_SIGHUP);
+           SyncRepInitConfig();
        }
 
        /* Handle the very limited subset of commands expected in this phase */
@@ -745,9 +744,9 @@ WalSndLoop(void)
            exit(1);
 
        /* Process any requests or signals received recently */
-       if (got_SIGHUP)
+       if (ConfigReloadPending)
        {
-           got_SIGHUP = false;
+           ConfigReloadPending = false;
            ProcessConfigFile(PGC_SIGHUP);
            SyncRepInitConfig();
        }
@@ -1302,19 +1301,6 @@ WalSndRqstFileReload(void)
    }
 }
 
-/* SIGHUP: set flag to re-read config file at next convenient time */
-static void
-WalSndSigHupHandler(SIGNAL_ARGS)
-{
-   int         save_errno = errno;
-
-   got_SIGHUP = true;
-   if (MyWalSnd)
-       SetLatch(&MyWalSnd->latch);
-
-   errno = save_errno;
-}
-
 /* SIGTERM: set flag to shut down */
 static void
 WalSndShutdownHandler(SIGNAL_ARGS)
@@ -1396,7 +1382,7 @@ void
 WalSndSignals(void)
 {
    /* Set up signal handlers */
-   pqsignal(SIGHUP, WalSndSigHupHandler);      /* set flag to read config
+   pqsignal(SIGHUP, PostgresSigHupHandler);    /* set flag to read config
                                                 * file */
    pqsignal(SIGINT, SIG_IGN);  /* not used */
    pqsignal(SIGTERM, WalSndShutdownHandler);   /* request shutdown */

diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index ad356d5d7dd04276546b34135b165af30610dd90..da26c1ab1214178a754c76a08bc34eac5f781432 100644 (file)
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -129,13 +129,6 @@ char      *stack_base_ptr = NULL;
 char      *register_stack_base_ptr = NULL;
 #endif
 
-/*
- * Flag to mark SIGHUP. Whenever the main loop comes around it
- * will reread the configuration file. (Better than doing the
- * reading in the signal handler, ey?)
- */
-static volatile sig_atomic_t got_SIGHUP = false;
-
 /*
  * Flag to keep track of whether we have started a transaction.
  * For extended query protocol this has to be remembered across messages.
@@ -203,7 +196,6 @@ static bool IsTransactionExitStmt(Node *parsetree);
 static bool IsTransactionExitStmtList(List *parseTrees);
 static bool IsTransactionStmtList(List *parseTrees);
 static void drop_unnamed_stmt(void);
-static void SigHupHandler(SIGNAL_ARGS);
 static void log_disconnections(int code, Datum arg);
 
 
@@ -2683,13 +2675,19 @@ FloatExceptionHandler(SIGNAL_ARGS)
                       "invalid operation, such as division by zero.")));
 }
 
-/* SIGHUP: set flag to re-read config file at next convenient time */
-static void
-SigHupHandler(SIGNAL_ARGS)
+/*
+ * SIGHUP: set flag to re-read config file at next convenient time.
+ *
+ * Sets the ConfigReloadPending flag, which should be checked at convenient
+ * places inside main loops. (Better than doing the reading in the signal
+ * handler, ey?)
+ */
+void
+PostgresSigHupHandler(SIGNAL_ARGS)
 {
    int         save_errno = errno;
 
-   got_SIGHUP = true;
+   ConfigReloadPending = true;
    if (MyProc)
        SetLatch(&MyProc->procLatch);
 
@@ -3650,8 +3648,8 @@ PostgresMain(int argc, char *argv[],
        WalSndSignals();
    else
    {
-       pqsignal(SIGHUP, SigHupHandler);        /* set flag to read config
-                                                * file */
+       pqsignal(SIGHUP, PostgresSigHupHandler);        /* set flag to read config
+                                                        * file */
        pqsignal(SIGINT, StatementCancelHandler);       /* cancel current query */
        pqsignal(SIGTERM, die); /* cancel current query and exit */
 
@@ -4010,9 +4008,9 @@ PostgresMain(int argc, char *argv[],
         * (5) check for any other interesting events that happened while we
         * slept.
         */
-       if (got_SIGHUP)
+       if (ConfigReloadPending)
        {
-           got_SIGHUP = false;
+           ConfigReloadPending = false;
            ProcessConfigFile(PGC_SIGHUP);
        }
 

diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c
index e4928a2815accc3919f3215005c70e9cc799ed10..b4978772293c6c1da5f3704a6f49429dc3207a53 100644 (file)
--- a/src/backend/utils/init/globals.c
+++ b/src/backend/utils/init/globals.c
@@ -31,6 +31,7 @@ volatile bool QueryCancelPending = false;
 volatile bool ProcDiePending = false;
 volatile bool ClientConnectionLost = false;
 volatile bool ImmediateInterruptOK = false;
+volatile sig_atomic_t ConfigReloadPending = false;
 volatile uint32 InterruptHoldoffCount = 0;
 volatile uint32 QueryCancelHoldoffCount = 0;
 volatile uint32 CritSectionCount = 0;

diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 859e40c2ca9bc850d3b8d85672b9153fdbd5f937..b4163b4fe41229800f868f88a81e067bef0c588a 100644 (file)
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -23,6 +23,8 @@
 #ifndef MISCADMIN_H
 #define MISCADMIN_H
 
+#include <signal.h>
+
 #include "pgtime.h"                /* for pg_time_t */
 
 
@@ -78,6 +80,7 @@
 extern PGDLLIMPORT volatile bool InterruptPending;
 extern volatile bool QueryCancelPending;
 extern volatile bool ProcDiePending;
+extern PGDLLIMPORT volatile sig_atomic_t ConfigReloadPending;
 
 extern volatile bool ClientConnectionLost;
 
@@ -275,6 +278,8 @@ extern void restore_stack_base(pg_stack_base_t base);
 extern void check_stack_depth(void);
 extern bool stack_is_too_deep(void);
 
+extern void PostgresSigHupHandler(SIGNAL_ARGS);
+
 /* in tcop/utility.c */
 extern void PreventCommandIfReadOnly(const char *cmdname);
 extern void PreventCommandDuringRecovery(const char *cmdname);