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

Commit 9abb2bf

Browse files
committed
In the postmaster, rely on the signal infrastructure to block signals.
POSIX sigaction(2) can be told to block a set of signals while a signal handler executes. Make use of that instead of manually blocking and unblocking signals in the postmaster's signal handlers. This should save a few cycles, and it also prevents recursive invocation of signal handlers when many signals arrive in close succession. We have seen buildfarm failures that seem to be due to postmaster stack overflow caused by such recursion (exacerbated by a Linux PPC64 kernel bug). This doesn't change anything about the way that it works on Windows. Somebody might consider adjusting port/win32/signal.c to let it work similarly, but I'm not in a position to do that. For the moment, just apply to HEAD. Possibly we should consider back-patching this, but it'd be good to let it age awhile first. Discussion: https://postgr.es/m/14878.1570820201@sss.pgh.pa.us
1 parent f38291e commit 9abb2bf

File tree

5 files changed

+116
-57
lines changed

5 files changed

+116
-57
lines changed

src/backend/libpq/pqsignal.c

+49
Original file line numberDiff line numberDiff line change
@@ -95,3 +95,52 @@ pqinitmask(void)
9595
sigdelset(&StartupBlockSig, SIGALRM);
9696
#endif
9797
}
98+
99+
/*
100+
* Set up a postmaster signal handler for signal "signo"
101+
*
102+
* Returns the previous handler.
103+
*
104+
* This is used only in the postmaster, which has its own odd approach to
105+
* signal handling. For signals with handlers, we block all signals for the
106+
* duration of signal handler execution. We also do not set the SA_RESTART
107+
* flag; this should be safe given the tiny range of code in which the
108+
* postmaster ever unblocks signals.
109+
*
110+
* pqinitmask() must have been invoked previously.
111+
*
112+
* On Windows, this function is just an alias for pqsignal()
113+
* (and note that it's calling the code in src/backend/port/win32/signal.c,
114+
* not src/port/pqsignal.c). On that platform, the postmaster's signal
115+
* handlers still have to block signals for themselves.
116+
*/
117+
pqsigfunc
118+
pqsignal_pm(int signo, pqsigfunc func)
119+
{
120+
#ifndef WIN32
121+
struct sigaction act,
122+
oact;
123+
124+
act.sa_handler = func;
125+
if (func == SIG_IGN || func == SIG_DFL)
126+
{
127+
/* in these cases, act the same as pqsignal() */
128+
sigemptyset(&act.sa_mask);
129+
act.sa_flags = SA_RESTART;
130+
}
131+
else
132+
{
133+
act.sa_mask = BlockSig;
134+
act.sa_flags = 0;
135+
}
136+
#ifdef SA_NOCLDSTOP
137+
if (signo == SIGCHLD)
138+
act.sa_flags |= SA_NOCLDSTOP;
139+
#endif
140+
if (sigaction(signo, &act, &oact) < 0)
141+
return SIG_ERR;
142+
return oact.sa_handler;
143+
#else /* WIN32 */
144+
return pqsignal(signo, func);
145+
#endif
146+
}

src/backend/postmaster/postmaster.c

+64-23
Original file line numberDiff line numberDiff line change
@@ -606,14 +606,25 @@ PostmasterMain(int argc, char *argv[])
606606
/*
607607
* Set up signal handlers for the postmaster process.
608608
*
609-
* In the postmaster, we want to install non-ignored handlers *without*
610-
* SA_RESTART. This is because they'll be blocked at all times except
611-
* when ServerLoop is waiting for something to happen, and during that
612-
* window, we want signals to exit the select(2) wait so that ServerLoop
613-
* can respond if anything interesting happened. On some platforms,
614-
* signals marked SA_RESTART would not cause the select() wait to end.
615-
* Child processes will generally want SA_RESTART, but we expect them to
616-
* set up their own handlers before unblocking signals.
609+
* In the postmaster, we use pqsignal_pm() rather than pqsignal() (which
610+
* is used by all child processes and client processes). That has a
611+
* couple of special behaviors:
612+
*
613+
* 1. Except on Windows, we tell sigaction() to block all signals for the
614+
* duration of the signal handler. This is faster than our old approach
615+
* of blocking/unblocking explicitly in the signal handler, and it should
616+
* also prevent excessive stack consumption if signals arrive quickly.
617+
*
618+
* 2. We do not set the SA_RESTART flag. This is because signals will be
619+
* blocked at all times except when ServerLoop is waiting for something to
620+
* happen, and during that window, we want signals to exit the select(2)
621+
* wait so that ServerLoop can respond if anything interesting happened.
622+
* On some platforms, signals marked SA_RESTART would not cause the
623+
* select() wait to end.
624+
*
625+
* Child processes will generally want SA_RESTART, so pqsignal() sets that
626+
* flag. We expect children to set up their own handlers before
627+
* unblocking signals.
617628
*
618629
* CAUTION: when changing this list, check for side-effects on the signal
619630
* handling setup of child processes. See tcop/postgres.c,
@@ -625,18 +636,16 @@ PostmasterMain(int argc, char *argv[])
625636
pqinitmask();
626637
PG_SETMASK(&BlockSig);
627638

628-
pqsignal_no_restart(SIGHUP, SIGHUP_handler); /* reread config file and
629-
* have children do same */
630-
pqsignal_no_restart(SIGINT, pmdie); /* send SIGTERM and shut down */
631-
pqsignal_no_restart(SIGQUIT, pmdie); /* send SIGQUIT and die */
632-
pqsignal_no_restart(SIGTERM, pmdie); /* wait for children and shut down */
633-
pqsignal(SIGALRM, SIG_IGN); /* ignored */
634-
pqsignal(SIGPIPE, SIG_IGN); /* ignored */
635-
pqsignal_no_restart(SIGUSR1, sigusr1_handler); /* message from child
636-
* process */
637-
pqsignal_no_restart(SIGUSR2, dummy_handler); /* unused, reserve for
638-
* children */
639-
pqsignal_no_restart(SIGCHLD, reaper); /* handle child termination */
639+
pqsignal_pm(SIGHUP, SIGHUP_handler); /* reread config file and have
640+
* children do same */
641+
pqsignal_pm(SIGINT, pmdie); /* send SIGTERM and shut down */
642+
pqsignal_pm(SIGQUIT, pmdie); /* send SIGQUIT and die */
643+
pqsignal_pm(SIGTERM, pmdie); /* wait for children and shut down */
644+
pqsignal_pm(SIGALRM, SIG_IGN); /* ignored */
645+
pqsignal_pm(SIGPIPE, SIG_IGN); /* ignored */
646+
pqsignal_pm(SIGUSR1, sigusr1_handler); /* message from child process */
647+
pqsignal_pm(SIGUSR2, dummy_handler); /* unused, reserve for children */
648+
pqsignal_pm(SIGCHLD, reaper); /* handle child termination */
640649

641650
/*
642651
* No other place in Postgres should touch SIGTTIN/SIGTTOU handling. We
@@ -646,15 +655,15 @@ PostmasterMain(int argc, char *argv[])
646655
* child processes should just allow the inherited settings to stand.
647656
*/
648657
#ifdef SIGTTIN
649-
pqsignal(SIGTTIN, SIG_IGN); /* ignored */
658+
pqsignal_pm(SIGTTIN, SIG_IGN); /* ignored */
650659
#endif
651660
#ifdef SIGTTOU
652-
pqsignal(SIGTTOU, SIG_IGN); /* ignored */
661+
pqsignal_pm(SIGTTOU, SIG_IGN); /* ignored */
653662
#endif
654663

655664
/* ignore SIGXFSZ, so that ulimit violations work like disk full */
656665
#ifdef SIGXFSZ
657-
pqsignal(SIGXFSZ, SIG_IGN); /* ignored */
666+
pqsignal_pm(SIGXFSZ, SIG_IGN); /* ignored */
658667
#endif
659668

660669
/*
@@ -2640,7 +2649,13 @@ SIGHUP_handler(SIGNAL_ARGS)
26402649
{
26412650
int save_errno = errno;
26422651

2652+
/*
2653+
* We rely on the signal mechanism to have blocked all signals ... except
2654+
* on Windows, which lacks sigaction(), so we have to do it manually.
2655+
*/
2656+
#ifdef WIN32
26432657
PG_SETMASK(&BlockSig);
2658+
#endif
26442659

26452660
if (Shutdown <= SmartShutdown)
26462661
{
@@ -2700,7 +2715,9 @@ SIGHUP_handler(SIGNAL_ARGS)
27002715
#endif
27012716
}
27022717

2718+
#ifdef WIN32
27032719
PG_SETMASK(&UnBlockSig);
2720+
#endif
27042721

27052722
errno = save_errno;
27062723
}
@@ -2714,7 +2731,13 @@ pmdie(SIGNAL_ARGS)
27142731
{
27152732
int save_errno = errno;
27162733

2734+
/*
2735+
* We rely on the signal mechanism to have blocked all signals ... except
2736+
* on Windows, which lacks sigaction(), so we have to do it manually.
2737+
*/
2738+
#ifdef WIN32
27172739
PG_SETMASK(&BlockSig);
2740+
#endif
27182741

27192742
ereport(DEBUG2,
27202743
(errmsg_internal("postmaster received signal %d",
@@ -2880,7 +2903,9 @@ pmdie(SIGNAL_ARGS)
28802903
break;
28812904
}
28822905

2906+
#ifdef WIN32
28832907
PG_SETMASK(&UnBlockSig);
2908+
#endif
28842909

28852910
errno = save_errno;
28862911
}
@@ -2895,7 +2920,13 @@ reaper(SIGNAL_ARGS)
28952920
int pid; /* process id of dead child process */
28962921
int exitstatus; /* its exit status */
28972922

2923+
/*
2924+
* We rely on the signal mechanism to have blocked all signals ... except
2925+
* on Windows, which lacks sigaction(), so we have to do it manually.
2926+
*/
2927+
#ifdef WIN32
28982928
PG_SETMASK(&BlockSig);
2929+
#endif
28992930

29002931
ereport(DEBUG4,
29012932
(errmsg_internal("reaping dead processes")));
@@ -3212,7 +3243,9 @@ reaper(SIGNAL_ARGS)
32123243
PostmasterStateMachine();
32133244

32143245
/* Done with signal handler */
3246+
#ifdef WIN32
32153247
PG_SETMASK(&UnBlockSig);
3248+
#endif
32163249

32173250
errno = save_errno;
32183251
}
@@ -5114,7 +5147,13 @@ sigusr1_handler(SIGNAL_ARGS)
51145147
{
51155148
int save_errno = errno;
51165149

5150+
/*
5151+
* We rely on the signal mechanism to have blocked all signals ... except
5152+
* on Windows, which lacks sigaction(), so we have to do it manually.
5153+
*/
5154+
#ifdef WIN32
51175155
PG_SETMASK(&BlockSig);
5156+
#endif
51185157

51195158
/* Process background worker state change. */
51205159
if (CheckPostmasterSignal(PMSIGNAL_BACKGROUND_WORKER_CHANGE))
@@ -5272,7 +5311,9 @@ sigusr1_handler(SIGNAL_ARGS)
52725311
signal_child(StartupPID, SIGUSR2);
52735312
}
52745313

5314+
#ifdef WIN32
52755315
PG_SETMASK(&UnBlockSig);
5316+
#endif
52765317

52775318
errno = save_errno;
52785319
}

src/include/libpq/pqsignal.h

+3
Original file line numberDiff line numberDiff line change
@@ -36,4 +36,7 @@ extern sigset_t UnBlockSig,
3636

3737
extern void pqinitmask(void);
3838

39+
/* pqsigfunc is declared in src/include/port.h */
40+
extern pqsigfunc pqsignal_pm(int signo, pqsigfunc func);
41+
3942
#endif /* PQSIGNAL_H */

src/include/port.h

-5
Original file line numberDiff line numberDiff line change
@@ -525,11 +525,6 @@ extern int pg_mkdir_p(char *path, int omode);
525525
/* port/pqsignal.c */
526526
typedef void (*pqsigfunc) (int signo);
527527
extern pqsigfunc pqsignal(int signo, pqsigfunc func);
528-
#ifndef WIN32
529-
extern pqsigfunc pqsignal_no_restart(int signo, pqsigfunc func);
530-
#else
531-
#define pqsignal_no_restart(signo, func) pqsignal(signo, func)
532-
#endif
533528

534529
/* port/quotes.c */
535530
extern char *escape_single_quotes_ascii(const char *src);

src/port/pqsignal.c

-29
Original file line numberDiff line numberDiff line change
@@ -58,33 +58,4 @@ 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-
9061
#endif /* !defined(WIN32) || defined(FRONTEND) */

0 commit comments

Comments
 (0)