Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 7753ca4

Browse filesBrowse files
tglsfdctglsfdc
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, but more importantly it prevents recursive invocation of signal handlers when many signals arrive in close succession. (Assuming that the platform's signal infrastructure is designed to avoid consuming stack space in that case, but this is demonstrably true at least on Linux.) The existing code has been seen to recurse to the point of stack overflow, either in the postmaster or in a forked-off child. Back-patch of commit 9abb2bf. At the time, we'd only seen excess postmaster stack consumption in the buildfarm; but we now have a user report of it, and that commit has aged enough to have a fair amount of confidence that it doesn't break anything. This still doesn't change anything about the way that it works on Windows. Perhaps someone else would like to fix that? Per bug #16673 from David Geier. Back-patch to 9.6. Although the problem exists in principle before that, we've only seen it actually materialize in connection with heavy use of parallel workers, so it doesn't seem necessary to do anything in 9.5; and the relevant code is different there, too. Discussion: https://postgr.es/m/16673-d278c604f8e34ec0@postgresql.org Discussion: https://postgr.es/m/14878.1570820201@sss.pgh.pa.us
1 parent c7573ab commit 7753ca4
Copy full SHA for 7753ca4

File tree

Expand file treeCollapse file tree

5 files changed

+116
-59
lines changed
Filter options
Expand file treeCollapse file tree

5 files changed

+116
-59
lines changed

‎src/backend/libpq/pqsignal.c

Copy file name to clipboardExpand all lines: src/backend/libpq/pqsignal.c
+49Lines changed: 49 additions & 0 deletions
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

Copy file name to clipboardExpand all lines: src/backend/postmaster/postmaster.c
+64-25Lines changed: 64 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -622,14 +622,25 @@ PostmasterMain(int argc, char *argv[])
622622
/*
623623
* Set up signal handlers for the postmaster process.
624624
*
625-
* In the postmaster, we want to install non-ignored handlers *without*
626-
* SA_RESTART. This is because they'll be blocked at all times except
627-
* when ServerLoop is waiting for something to happen, and during that
628-
* window, we want signals to exit the select(2) wait so that ServerLoop
629-
* can respond if anything interesting happened. On some platforms,
630-
* signals marked SA_RESTART would not cause the select() wait to end.
631-
* Child processes will generally want SA_RESTART, but we expect them to
632-
* set up their own handlers before unblocking signals.
625+
* In the postmaster, we use pqsignal_pm() rather than pqsignal() (which
626+
* is used by all child processes and client processes). That has a
627+
* couple of special behaviors:
628+
*
629+
* 1. Except on Windows, we tell sigaction() to block all signals for the
630+
* duration of the signal handler. This is faster than our old approach
631+
* of blocking/unblocking explicitly in the signal handler, and it should
632+
* also prevent excessive stack consumption if signals arrive quickly.
633+
*
634+
* 2. We do not set the SA_RESTART flag. This is because signals will be
635+
* blocked at all times except when ServerLoop is waiting for something to
636+
* happen, and during that window, we want signals to exit the select(2)
637+
* wait so that ServerLoop can respond if anything interesting happened.
638+
* On some platforms, signals marked SA_RESTART would not cause the
639+
* select() wait to end.
640+
*
641+
* Child processes will generally want SA_RESTART, so pqsignal() sets that
642+
* flag. We expect children to set up their own handlers before
643+
* unblocking signals.
633644
*
634645
* CAUTION: when changing this list, check for side-effects on the signal
635646
* handling setup of child processes. See tcop/postgres.c,
@@ -641,25 +652,21 @@ PostmasterMain(int argc, char *argv[])
641652
pqinitmask();
642653
PG_SETMASK(&BlockSig);
643654

644-
pqsignal_no_restart(SIGHUP, SIGHUP_handler); /* reread config file
645-
* and have children do
646-
* same */
647-
pqsignal_no_restart(SIGINT, pmdie); /* send SIGTERM and shut down */
648-
pqsignal_no_restart(SIGQUIT, pmdie); /* send SIGQUIT and die */
649-
pqsignal_no_restart(SIGTERM, pmdie); /* wait for children and shut
650-
* down */
651-
pqsignal(SIGALRM, SIG_IGN); /* ignored */
652-
pqsignal(SIGPIPE, SIG_IGN); /* ignored */
653-
pqsignal_no_restart(SIGUSR1, sigusr1_handler); /* message from child
654-
* process */
655-
pqsignal_no_restart(SIGUSR2, dummy_handler); /* unused, reserve for
656-
* children */
657-
pqsignal_no_restart(SIGCHLD, reaper); /* handle child termination */
658-
pqsignal(SIGTTIN, SIG_IGN); /* ignored */
659-
pqsignal(SIGTTOU, SIG_IGN); /* ignored */
655+
pqsignal_pm(SIGHUP, SIGHUP_handler); /* reread config file and have
656+
* children do same */
657+
pqsignal_pm(SIGINT, pmdie); /* send SIGTERM and shut down */
658+
pqsignal_pm(SIGQUIT, pmdie); /* send SIGQUIT and die */
659+
pqsignal_pm(SIGTERM, pmdie); /* wait for children and shut down */
660+
pqsignal_pm(SIGALRM, SIG_IGN); /* ignored */
661+
pqsignal_pm(SIGPIPE, SIG_IGN); /* ignored */
662+
pqsignal_pm(SIGUSR1, sigusr1_handler); /* message from child process */
663+
pqsignal_pm(SIGUSR2, dummy_handler); /* unused, reserve for children */
664+
pqsignal_pm(SIGCHLD, reaper); /* handle child termination */
665+
pqsignal_pm(SIGTTIN, SIG_IGN); /* ignored */
666+
pqsignal_pm(SIGTTOU, SIG_IGN); /* ignored */
660667
/* ignore SIGXFSZ, so that ulimit violations work like disk full */
661668
#ifdef SIGXFSZ
662-
pqsignal(SIGXFSZ, SIG_IGN); /* ignored */
669+
pqsignal_pm(SIGXFSZ, SIG_IGN); /* ignored */
663670
#endif
664671

665672
/*
@@ -2569,7 +2576,13 @@ SIGHUP_handler(SIGNAL_ARGS)
25692576
{
25702577
int save_errno = errno;
25712578

2579+
/*
2580+
* We rely on the signal mechanism to have blocked all signals ... except
2581+
* on Windows, which lacks sigaction(), so we have to do it manually.
2582+
*/
2583+
#ifdef WIN32
25722584
PG_SETMASK(&BlockSig);
2585+
#endif
25732586

25742587
if (Shutdown <= SmartShutdown)
25752588
{
@@ -2611,7 +2624,9 @@ SIGHUP_handler(SIGNAL_ARGS)
26112624
#endif
26122625
}
26132626

2627+
#ifdef WIN32
26142628
PG_SETMASK(&UnBlockSig);
2629+
#endif
26152630

26162631
errno = save_errno;
26172632
}
@@ -2625,7 +2640,13 @@ pmdie(SIGNAL_ARGS)
26252640
{
26262641
int save_errno = errno;
26272642

2643+
/*
2644+
* We rely on the signal mechanism to have blocked all signals ... except
2645+
* on Windows, which lacks sigaction(), so we have to do it manually.
2646+
*/
2647+
#ifdef WIN32
26282648
PG_SETMASK(&BlockSig);
2649+
#endif
26292650

26302651
ereport(DEBUG2,
26312652
(errmsg_internal("postmaster received signal %d",
@@ -2745,7 +2766,9 @@ pmdie(SIGNAL_ARGS)
27452766
break;
27462767
}
27472768

2769+
#ifdef WIN32
27482770
PG_SETMASK(&UnBlockSig);
2771+
#endif
27492772

27502773
errno = save_errno;
27512774
}
@@ -2760,7 +2783,13 @@ reaper(SIGNAL_ARGS)
27602783
int pid; /* process id of dead child process */
27612784
int exitstatus; /* its exit status */
27622785

2786+
/*
2787+
* We rely on the signal mechanism to have blocked all signals ... except
2788+
* on Windows, which lacks sigaction(), so we have to do it manually.
2789+
*/
2790+
#ifdef WIN32
27632791
PG_SETMASK(&BlockSig);
2792+
#endif
27642793

27652794
ereport(DEBUG4,
27662795
(errmsg_internal("reaping dead processes")));
@@ -3061,7 +3090,9 @@ reaper(SIGNAL_ARGS)
30613090
PostmasterStateMachine();
30623091

30633092
/* Done with signal handler */
3093+
#ifdef WIN32
30643094
PG_SETMASK(&UnBlockSig);
3095+
#endif
30653096

30663097
errno = save_errno;
30673098
}
@@ -4975,7 +5006,13 @@ sigusr1_handler(SIGNAL_ARGS)
49755006
{
49765007
int save_errno = errno;
49775008

5009+
/*
5010+
* We rely on the signal mechanism to have blocked all signals ... except
5011+
* on Windows, which lacks sigaction(), so we have to do it manually.
5012+
*/
5013+
#ifdef WIN32
49785014
PG_SETMASK(&BlockSig);
5015+
#endif
49795016

49805017
/* Process background worker state change. */
49815018
if (CheckPostmasterSignal(PMSIGNAL_BACKGROUND_WORKER_CHANGE))
@@ -5118,7 +5155,9 @@ sigusr1_handler(SIGNAL_ARGS)
51185155
signal_child(StartupPID, SIGUSR2);
51195156
}
51205157

5158+
#ifdef WIN32
51215159
PG_SETMASK(&UnBlockSig);
5160+
#endif
51225161

51235162
errno = save_errno;
51245163
}

‎src/include/libpq/pqsignal.h

Copy file name to clipboardExpand all lines: src/include/libpq/pqsignal.h
+3Lines changed: 3 additions & 0 deletions
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

Copy file name to clipboardExpand all lines: src/include/port.h
-5Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -465,11 +465,6 @@ extern int pg_mkdir_p(char *path, int omode);
465465
/* port/pqsignal.c */
466466
typedef void (*pqsigfunc) (int signo);
467467
extern pqsigfunc pqsignal(int signo, pqsigfunc func);
468-
#ifndef WIN32
469-
extern pqsigfunc pqsignal_no_restart(int signo, pqsigfunc func);
470-
#else
471-
#define pqsignal_no_restart(signo, func) pqsignal(signo, func)
472-
#endif
473468

474469
/* port/quotes.c */
475470
extern char *escape_single_quotes_ascii(const char *src);

‎src/port/pqsignal.c

Copy file name to clipboardExpand all lines: src/port/pqsignal.c
-29Lines changed: 0 additions & 29 deletions
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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.