Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 8939020

Browse files
committed
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
1 parent cbc2270 commit 8939020

File tree

3 files changed

+56
-9
lines changed

3 files changed

+56
-9
lines changed

src/backend/postmaster/postmaster.c

+21-8
Original file line numberDiff line numberDiff line change
@@ -611,6 +611,15 @@ PostmasterMain(int argc, char *argv[])
611611
/*
612612
* Set up signal handlers for the postmaster process.
613613
*
614+
* In the postmaster, we want to install non-ignored handlers *without*
615+
* SA_RESTART. This is because they'll be blocked at all times except
616+
* when ServerLoop is waiting for something to happen, and during that
617+
* window, we want signals to exit the select(2) wait so that ServerLoop
618+
* can respond if anything interesting happened. On some platforms,
619+
* signals marked SA_RESTART would not cause the select() wait to end.
620+
* Child processes will generally want SA_RESTART, but we expect them to
621+
* set up their own handlers before unblocking signals.
622+
*
614623
* CAUTION: when changing this list, check for side-effects on the signal
615624
* handling setup of child processes. See tcop/postgres.c,
616625
* bootstrap/bootstrap.c, postmaster/bgwriter.c, postmaster/walwriter.c,
@@ -621,16 +630,20 @@ PostmasterMain(int argc, char *argv[])
621630
pqinitmask();
622631
PG_SETMASK(&BlockSig);
623632

624-
pqsignal(SIGHUP, SIGHUP_handler); /* reread config file and have
625-
* children do same */
626-
pqsignal(SIGINT, pmdie); /* send SIGTERM and shut down */
627-
pqsignal(SIGQUIT, pmdie); /* send SIGQUIT and die */
628-
pqsignal(SIGTERM, pmdie); /* wait for children and shut down */
633+
pqsignal_no_restart(SIGHUP, SIGHUP_handler); /* reread config file
634+
* and have children do
635+
* same */
636+
pqsignal_no_restart(SIGINT, pmdie); /* send SIGTERM and shut down */
637+
pqsignal_no_restart(SIGQUIT, pmdie); /* send SIGQUIT and die */
638+
pqsignal_no_restart(SIGTERM, pmdie); /* wait for children and shut
639+
* down */
629640
pqsignal(SIGALRM, SIG_IGN); /* ignored */
630641
pqsignal(SIGPIPE, SIG_IGN); /* ignored */
631-
pqsignal(SIGUSR1, sigusr1_handler); /* message from child process */
632-
pqsignal(SIGUSR2, dummy_handler); /* unused, reserve for children */
633-
pqsignal(SIGCHLD, reaper); /* handle child termination */
642+
pqsignal_no_restart(SIGUSR1, sigusr1_handler); /* message from child
643+
* process */
644+
pqsignal_no_restart(SIGUSR2, dummy_handler); /* unused, reserve for
645+
* children */
646+
pqsignal_no_restart(SIGCHLD, reaper); /* handle child termination */
634647
pqsignal(SIGTTIN, SIG_IGN); /* ignored */
635648
pqsignal(SIGTTOU, SIG_IGN); /* ignored */
636649
/* ignore SIGXFSZ, so that ulimit violations work like disk full */

src/include/port.h

+5
Original file line numberDiff line numberDiff line change
@@ -469,6 +469,11 @@ extern int pg_mkdir_p(char *path, int omode);
469469
/* port/pqsignal.c */
470470
typedef void (*pqsigfunc) (int signo);
471471
extern pqsigfunc pqsignal(int signo, pqsigfunc func);
472+
#ifndef WIN32
473+
extern pqsigfunc pqsignal_no_restart(int signo, pqsigfunc func);
474+
#else
475+
#define pqsignal_no_restart(signo, func) pqsignal(signo, func)
476+
#endif
472477

473478
/* port/quotes.c */
474479
extern char *escape_single_quotes_ascii(const char *src);

src/port/pqsignal.c

+30-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
#if !defined(WIN32) || defined(FRONTEND)
3333

3434
/*
35-
* Set up a signal handler for signal "signo"
35+
* Set up a signal handler, with SA_RESTART, for signal "signo"
3636
*
3737
* Returns the previous handler.
3838
*/
@@ -58,4 +58,33 @@ pqsignal(int signo, pqsigfunc func)
5858
#endif
5959
}
6060

61+
/*
62+
* Set up a signal handler, without SA_RESTART, for signal "signo"
63+
*
64+
* Returns the previous handler.
65+
*
66+
* On Windows, this would be identical to pqsignal(), so don't bother.
67+
*/
68+
#ifndef WIN32
69+
70+
pqsigfunc
71+
pqsignal_no_restart(int signo, pqsigfunc func)
72+
{
73+
struct sigaction act,
74+
oact;
75+
76+
act.sa_handler = func;
77+
sigemptyset(&act.sa_mask);
78+
act.sa_flags = 0;
79+
#ifdef SA_NOCLDSTOP
80+
if (signo == SIGCHLD)
81+
act.sa_flags |= SA_NOCLDSTOP;
82+
#endif
83+
if (sigaction(signo, &act, &oact) < 0)
84+
return SIG_ERR;
85+
return oact.sa_handler;
86+
}
87+
88+
#endif /* !WIN32 */
89+
6190
#endif /* !defined(WIN32) || defined(FRONTEND) */

0 commit comments

Comments
 (0)