Run the postmaster's signal handlers without SA_RESTART.

The postmaster keeps signals blocked everywhere except while waiting
for something to happen in ServerLoop().  The code expects that the
select(2) will be cancelled with EINTR if an interrupt occurs; without
that, followup actions that should be performed by ServerLoop() itself
will be delayed.  However, some platforms interpret the SA_RESTART
signal flag as meaning that they should restart rather than cancel
the select(2).  Worse yet, some of them restart it with the original
timeout delay, meaning that a steady stream of signal interrupts can
prevent ServerLoop() from iterating at all if there are no incoming
connection requests.

Observable symptoms of this, on an affected platform such as HPUX 10,
include extremely slow parallel query startup (possibly as much as
30 seconds) and failure to update timestamps on the postmaster's sockets
and lockfiles when no new connections arrive for a long time.

We can fix this by running the postmaster's signal handlers without
SA_RESTART.  That would be quite a scary change if the range of code
where signals are accepted weren't so tiny, but as it is, it seems
safe enough.  (Note that postmaster children do, and must, reset all
the handlers before unblocking signals; so this change should not
affect any child process.)

There is talk of rewriting the postmaster to use a WaitEventSet and
not do signal response work in signal handlers, at which point it might
be appropriate to revert this patch.  But that's not happening before
v11 at the earliest.

Back-patch to 9.6.  The problem exists much further back, but the
worst symptom arises only in connection with parallel query, so it
does not seem worth taking any portability risks in older branches.

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

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 8461c24c268e4ed4c0eefe9e4602c1f8fd00850a..2bb43805331acca4429c42f14f54010dfcc0ac6d 100644 (file)
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -611,6 +611,15 @@ PostmasterMain(int argc, char *argv[])
        /*
         * Set up signal handlers for the postmaster process.
         *
+        * In the postmaster, we want to install non-ignored handlers *without*
+        * SA_RESTART.  This is because they'll 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, but we expect them 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,
@@ -621,16 +630,20 @@ PostmasterMain(int argc, char *argv[])
        pqinitmask();
        PG_SETMASK(&BlockSig);
 
-       pqsignal(SIGHUP, SIGHUP_handler);       /* reread config file and have
-                                                                                * children do same */
-       pqsignal(SIGINT, pmdie);        /* send SIGTERM and shut down */
-       pqsignal(SIGQUIT, pmdie);       /* send SIGQUIT and die */
-       pqsignal(SIGTERM, pmdie);       /* wait for children and shut down */
+       pqsignal_no_restart(SIGHUP, SIGHUP_handler);            /* reread config file
+                                                                                                                * and have children do
+                                                                                                                * same */
+       pqsignal_no_restart(SIGINT, pmdie); /* send SIGTERM and shut down */
+       pqsignal_no_restart(SIGQUIT, pmdie);            /* send SIGQUIT and die */
+       pqsignal_no_restart(SIGTERM, pmdie);            /* wait for children and shut
+                                                                                                * down */
        pqsignal(SIGALRM, SIG_IGN); /* ignored */
        pqsignal(SIGPIPE, SIG_IGN); /* ignored */
-       pqsignal(SIGUSR1, sigusr1_handler); /* message from child process */
-       pqsignal(SIGUSR2, dummy_handler);       /* unused, reserve for children */
-       pqsignal(SIGCHLD, reaper);      /* handle child termination */
+       pqsignal_no_restart(SIGUSR1, sigusr1_handler);          /* message from child
+                                                                                                                * process */
+       pqsignal_no_restart(SIGUSR2, dummy_handler);            /* unused, reserve for
+                                                                                                                * children */
+       pqsignal_no_restart(SIGCHLD, reaper);           /* handle child termination */
        pqsignal(SIGTTIN, SIG_IGN); /* ignored */
        pqsignal(SIGTTOU, SIG_IGN); /* ignored */
        /* ignore SIGXFSZ, so that ulimit violations work like disk full */

diff --git a/src/include/port.h b/src/include/port.h
index c6937e58a8f366935aebdf831ee849a04305a300..52910ed203fc19f1369b5dd31eceaf48def5d10d 100644 (file)
--- a/src/include/port.h
+++ b/src/include/port.h
@@ -469,6 +469,11 @@ extern int pg_mkdir_p(char *path, int omode);
 /* port/pqsignal.c */
 typedef void (*pqsigfunc) (int signo);
 extern pqsigfunc pqsignal(int signo, pqsigfunc func);
+#ifndef WIN32
+extern pqsigfunc pqsignal_no_restart(int signo, pqsigfunc func);
+#else
+#define pqsignal_no_restart(signo, func) pqsignal(signo, func)
+#endif
 
 /* port/quotes.c */
 extern char *escape_single_quotes_ascii(const char *src);

diff --git a/src/port/pqsignal.c b/src/port/pqsignal.c
index 5d366da11e6dbe93ca0ea4c7906cf5b5913a83d6..e7e445101c221e662fd5eec32dadac6bd5015d5c 100644 (file)
--- a/src/port/pqsignal.c
+++ b/src/port/pqsignal.c
@@ -32,7 +32,7 @@
 #if !defined(WIN32) || defined(FRONTEND)
 
 /*
- * Set up a signal handler for signal "signo"
+ * Set up a signal handler, with SA_RESTART, for signal "signo"
  *
  * Returns the previous handler.
  */
@@ -58,4 +58,33 @@ pqsignal(int signo, pqsigfunc func)
 #endif
 }
 
+/*
+ * Set up a signal handler, without SA_RESTART, for signal "signo"
+ *
+ * Returns the previous handler.
+ *
+ * On Windows, this would be identical to pqsignal(), so don't bother.
+ */
+#ifndef WIN32
+
+pqsigfunc
+pqsignal_no_restart(int signo, pqsigfunc func)
+{
+       struct sigaction act,
+                               oact;
+
+       act.sa_handler = func;
+       sigemptyset(&act.sa_mask);
+       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;
+}
+
+#endif   /* !WIN32 */
+
 #endif   /* !defined(WIN32) || defined(FRONTEND) */