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

Commit 44fc6e2

Browse files
committed
Centralize setup of SIGQUIT handling for postmaster child processes.
We decided that the policy established in commit 7634bd4 for the bgwriter, checkpointer, walwriter, and walreceiver processes, namely that they should accept SIGQUIT at all times, really ought to apply uniformly to all postmaster children. Therefore, get rid of the duplicative and inconsistent per-process code for establishing that signal handler and removing SIGQUIT from BlockSig. Instead, make InitPostmasterChild do it. The handler set up by InitPostmasterChild is SignalHandlerForCrashExit, which just summarily does _exit(2). In interactive backends, we almost immediately replace that with quickdie, since we would prefer to try to tell the client that we're dying. However, this patch is changing the behavior of autovacuum (both launcher and workers), as well as walsenders. Those processes formerly also used quickdie, but AFAICS that was just mindless copy-and-paste: they don't have any interactive client that's likely to benefit from being told this. The stats collector continues to be an outlier, in that it thinks SIGQUIT means normal exit. That should probably be changed for consistency, but there's another patch set where that's being dealt with, so I didn't do so here. Discussion: https://postgr.es/m/644875.1599933441@sss.pgh.pa.us
1 parent 2000b6c commit 44fc6e2

File tree

12 files changed

+51
-49
lines changed

12 files changed

+51
-49
lines changed

src/backend/postmaster/autovacuum.c

+10-8
Original file line numberDiff line numberDiff line change
@@ -454,8 +454,8 @@ AutoVacLauncherMain(int argc, char *argv[])
454454
pqsignal(SIGHUP, SignalHandlerForConfigReload);
455455
pqsignal(SIGINT, StatementCancelHandler);
456456
pqsignal(SIGTERM, SignalHandlerForShutdownRequest);
457+
/* SIGQUIT handler was already set up by InitPostmasterChild */
457458

458-
pqsignal(SIGQUIT, quickdie);
459459
InitializeTimeouts(); /* establishes SIGALRM handler */
460460

461461
pqsignal(SIGPIPE, SIG_IGN);
@@ -498,9 +498,10 @@ AutoVacLauncherMain(int argc, char *argv[])
498498
*
499499
* Note that we use sigsetjmp(..., 1), so that the prevailing signal mask
500500
* (to wit, BlockSig) will be restored when longjmp'ing to here. Thus,
501-
* signals will be blocked until we complete error recovery. It might
502-
* seem that this policy makes the HOLD_INTERRUPTS() call redundant, but
503-
* it is not since InterruptPending might be set already.
501+
* signals other than SIGQUIT will be blocked until we complete error
502+
* recovery. It might seem that this policy makes the HOLD_INTERRUPTS()
503+
* call redundant, but it is not since InterruptPending might be set
504+
* already.
504505
*/
505506
if (sigsetjmp(local_sigjmp_buf, 1) != 0)
506507
{
@@ -1531,7 +1532,8 @@ AutoVacWorkerMain(int argc, char *argv[])
15311532
*/
15321533
pqsignal(SIGINT, StatementCancelHandler);
15331534
pqsignal(SIGTERM, die);
1534-
pqsignal(SIGQUIT, quickdie);
1535+
/* SIGQUIT handler was already set up by InitPostmasterChild */
1536+
15351537
InitializeTimeouts(); /* establishes SIGALRM handler */
15361538

15371539
pqsignal(SIGPIPE, SIG_IGN);
@@ -1562,9 +1564,9 @@ AutoVacWorkerMain(int argc, char *argv[])
15621564
*
15631565
* Note that we use sigsetjmp(..., 1), so that the prevailing signal mask
15641566
* (to wit, BlockSig) will be restored when longjmp'ing to here. Thus,
1565-
* signals will be blocked until we exit. It might seem that this policy
1566-
* makes the HOLD_INTERRUPTS() call redundant, but it is not since
1567-
* InterruptPending might be set already.
1567+
* signals other than SIGQUIT will be blocked until we exit. It might
1568+
* seem that this policy makes the HOLD_INTERRUPTS() call redundant, but
1569+
* it is not since InterruptPending might be set already.
15681570
*/
15691571
if (sigsetjmp(local_sigjmp_buf, 1) != 0)
15701572
{

src/backend/postmaster/bgworker.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -731,9 +731,9 @@ StartBackgroundWorker(void)
731731
pqsignal(SIGFPE, SIG_IGN);
732732
}
733733
pqsignal(SIGTERM, bgworker_die);
734+
/* SIGQUIT handler was already set up by InitPostmasterChild */
734735
pqsignal(SIGHUP, SIG_IGN);
735736

736-
pqsignal(SIGQUIT, SignalHandlerForCrashExit);
737737
InitializeTimeouts(); /* establishes SIGALRM handler */
738738

739739
pqsignal(SIGPIPE, SIG_IGN);

src/backend/postmaster/bgwriter.c

+1-5
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ BackgroundWriterMain(void)
104104
pqsignal(SIGHUP, SignalHandlerForConfigReload);
105105
pqsignal(SIGINT, SIG_IGN);
106106
pqsignal(SIGTERM, SignalHandlerForShutdownRequest);
107-
pqsignal(SIGQUIT, SignalHandlerForCrashExit);
107+
/* SIGQUIT handler was already set up by InitPostmasterChild */
108108
pqsignal(SIGALRM, SIG_IGN);
109109
pqsignal(SIGPIPE, SIG_IGN);
110110
pqsignal(SIGUSR1, procsignal_sigusr1_handler);
@@ -115,10 +115,6 @@ BackgroundWriterMain(void)
115115
*/
116116
pqsignal(SIGCHLD, SIG_DFL);
117117

118-
/* We allow SIGQUIT (SignalHandlerForCrashExit) at all times */
119-
sigdelset(&BlockSig, SIGQUIT);
120-
PG_SETMASK(&BlockSig);
121-
122118
/*
123119
* We just started, assume there has been either a shutdown or
124120
* end-of-recovery snapshot.

src/backend/postmaster/checkpointer.c

+1-5
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ CheckpointerMain(void)
198198
pqsignal(SIGHUP, SignalHandlerForConfigReload);
199199
pqsignal(SIGINT, ReqCheckpointHandler); /* request checkpoint */
200200
pqsignal(SIGTERM, SIG_IGN); /* ignore SIGTERM */
201-
pqsignal(SIGQUIT, SignalHandlerForCrashExit);
201+
/* SIGQUIT handler was already set up by InitPostmasterChild */
202202
pqsignal(SIGALRM, SIG_IGN);
203203
pqsignal(SIGPIPE, SIG_IGN);
204204
pqsignal(SIGUSR1, procsignal_sigusr1_handler);
@@ -209,10 +209,6 @@ CheckpointerMain(void)
209209
*/
210210
pqsignal(SIGCHLD, SIG_DFL);
211211

212-
/* We allow SIGQUIT (SignalHandlerForCrashExit) at all times */
213-
sigdelset(&BlockSig, SIGQUIT);
214-
PG_SETMASK(&BlockSig);
215-
216212
/*
217213
* Initialize so that first time-driven event happens at the correct time.
218214
*/

src/backend/postmaster/pgarch.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ PgArchiverMain(int argc, char *argv[])
228228
pqsignal(SIGHUP, SignalHandlerForConfigReload);
229229
pqsignal(SIGINT, SIG_IGN);
230230
pqsignal(SIGTERM, SignalHandlerForShutdownRequest);
231-
pqsignal(SIGQUIT, SignalHandlerForCrashExit);
231+
/* SIGQUIT handler was already set up by InitPostmasterChild */
232232
pqsignal(SIGALRM, SIG_IGN);
233233
pqsignal(SIGPIPE, SIG_IGN);
234234
pqsignal(SIGUSR1, pgarch_waken);

src/backend/postmaster/postmaster.c

+2-6
Original file line numberDiff line numberDiff line change
@@ -4355,7 +4355,7 @@ BackendInitialize(Port *port)
43554355
* cleaned up.
43564356
*/
43574357
pqsignal(SIGTERM, process_startup_packet_die);
4358-
pqsignal(SIGQUIT, SignalHandlerForCrashExit);
4358+
/* SIGQUIT handler was already set up by InitPostmasterChild */
43594359
InitializeTimeouts(); /* establishes SIGALRM handler */
43604360
PG_SETMASK(&StartupBlockSig);
43614361

@@ -4435,7 +4435,7 @@ BackendInitialize(Port *port)
44354435
status = ProcessStartupPacket(port, false, false);
44364436

44374437
/*
4438-
* Disable the timeout, and prevent SIGTERM/SIGQUIT again.
4438+
* Disable the timeout, and prevent SIGTERM again.
44394439
*/
44404440
disable_timeout(STARTUP_PACKET_TIMEOUT, false);
44414441
PG_SETMASK(&BlockSig);
@@ -4983,10 +4983,6 @@ SubPostmasterMain(int argc, char *argv[])
49834983
if (strcmp(argv[1], "--forkavworker") == 0)
49844984
AutovacuumWorkerIAm();
49854985

4986-
/* In EXEC_BACKEND case we will not have inherited these settings */
4987-
pqinitmask();
4988-
PG_SETMASK(&BlockSig);
4989-
49904986
/* Read in remaining GUC variables */
49914987
read_nondefault_variables();
49924988

src/backend/postmaster/startup.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ StartupProcessMain(void)
175175
pqsignal(SIGHUP, StartupProcSigHupHandler); /* reload config file */
176176
pqsignal(SIGINT, SIG_IGN); /* ignore query cancel */
177177
pqsignal(SIGTERM, StartupProcShutdownHandler); /* request shutdown */
178-
pqsignal(SIGQUIT, SignalHandlerForCrashExit);
178+
/* SIGQUIT handler was already set up by InitPostmasterChild */
179179
InitializeTimeouts(); /* establishes SIGALRM handler */
180180
pqsignal(SIGPIPE, SIG_IGN);
181181
pqsignal(SIGUSR1, procsignal_sigusr1_handler);

src/backend/postmaster/walwriter.c

+1-5
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ WalWriterMain(void)
101101
pqsignal(SIGHUP, SignalHandlerForConfigReload);
102102
pqsignal(SIGINT, SignalHandlerForShutdownRequest);
103103
pqsignal(SIGTERM, SignalHandlerForShutdownRequest);
104-
pqsignal(SIGQUIT, SignalHandlerForCrashExit);
104+
/* SIGQUIT handler was already set up by InitPostmasterChild */
105105
pqsignal(SIGALRM, SIG_IGN);
106106
pqsignal(SIGPIPE, SIG_IGN);
107107
pqsignal(SIGUSR1, procsignal_sigusr1_handler);
@@ -112,10 +112,6 @@ WalWriterMain(void)
112112
*/
113113
pqsignal(SIGCHLD, SIG_DFL);
114114

115-
/* We allow SIGQUIT (SignalHandlerForCrashExit) at all times */
116-
sigdelset(&BlockSig, SIGQUIT);
117-
PG_SETMASK(&BlockSig);
118-
119115
/*
120116
* Create a memory context that we will do all our work in. We do this so
121117
* that we can reset the context during error recovery and thereby avoid

src/backend/replication/walreceiver.c

+1-5
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ WalReceiverMain(void)
270270
pqsignal(SIGHUP, WalRcvSigHupHandler); /* set flag to read config file */
271271
pqsignal(SIGINT, SIG_IGN);
272272
pqsignal(SIGTERM, WalRcvShutdownHandler); /* request shutdown */
273-
pqsignal(SIGQUIT, SignalHandlerForCrashExit);
273+
/* SIGQUIT handler was already set up by InitPostmasterChild */
274274
pqsignal(SIGALRM, SIG_IGN);
275275
pqsignal(SIGPIPE, SIG_IGN);
276276
pqsignal(SIGUSR1, procsignal_sigusr1_handler);
@@ -279,10 +279,6 @@ WalReceiverMain(void)
279279
/* Reset some signals that are accepted by postmaster but not here */
280280
pqsignal(SIGCHLD, SIG_DFL);
281281

282-
/* We allow SIGQUIT (SignalHandlerForCrashExit) at all times */
283-
sigdelset(&BlockSig, SIGQUIT);
284-
PG_SETMASK(&BlockSig);
285-
286282
/* Load the libpq-specific functions */
287283
load_file("libpqwalreceiver", false);
288284
if (WalReceiverFunctions == NULL)

src/backend/replication/walsender.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -3041,7 +3041,7 @@ WalSndSignals(void)
30413041
pqsignal(SIGHUP, SignalHandlerForConfigReload);
30423042
pqsignal(SIGINT, StatementCancelHandler); /* query cancel */
30433043
pqsignal(SIGTERM, die); /* request shutdown */
3044-
pqsignal(SIGQUIT, quickdie); /* hard crash time */
3044+
/* SIGQUIT handler was already set up by InitPostmasterChild */
30453045
InitializeTimeouts(); /* establishes SIGALRM handler */
30463046
pqsignal(SIGPIPE, SIG_IGN);
30473047
pqsignal(SIGUSR1, procsignal_sigusr1_handler);

src/backend/tcop/postgres.c

+5-11
Original file line numberDiff line numberDiff line change
@@ -3820,7 +3820,8 @@ PostgresMain(int argc, char *argv[],
38203820
}
38213821

38223822
/*
3823-
* Set up signal handlers and masks.
3823+
* Set up signal handlers. (InitPostmasterChild or InitStandaloneProcess
3824+
* has already set up BlockSig and made that the active signal mask.)
38243825
*
38253826
* Note that postmaster blocked all signals before forking child process,
38263827
* so there is no race condition whereby we might receive a signal before
@@ -3842,6 +3843,9 @@ PostgresMain(int argc, char *argv[],
38423843
pqsignal(SIGTERM, die); /* cancel current query and exit */
38433844

38443845
/*
3846+
* In a postmaster child backend, replace SignalHandlerForCrashExit
3847+
* with quickdie, so we can tell the client we're dying.
3848+
*
38453849
* In a standalone backend, SIGQUIT can be generated from the keyboard
38463850
* easily, while SIGTERM cannot, so we make both signals do die()
38473851
* rather than quickdie().
@@ -3871,16 +3875,6 @@ PostgresMain(int argc, char *argv[],
38713875
* platforms */
38723876
}
38733877

3874-
pqinitmask();
3875-
3876-
if (IsUnderPostmaster)
3877-
{
3878-
/* We allow SIGQUIT (quickdie) at all times */
3879-
sigdelset(&BlockSig, SIGQUIT);
3880-
}
3881-
3882-
PG_SETMASK(&BlockSig); /* block everything except SIGQUIT */
3883-
38843878
if (!IsUnderPostmaster)
38853879
{
38863880
/*

src/backend/utils/init/miscinit.c

+26
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,12 @@
3232
#include "catalog/pg_authid.h"
3333
#include "common/file_perm.h"
3434
#include "libpq/libpq.h"
35+
#include "libpq/pqsignal.h"
3536
#include "mb/pg_wchar.h"
3637
#include "miscadmin.h"
3738
#include "pgstat.h"
3839
#include "postmaster/autovacuum.h"
40+
#include "postmaster/interrupt.h"
3941
#include "postmaster/postmaster.h"
4042
#include "storage/fd.h"
4143
#include "storage/ipc.h"
@@ -133,6 +135,23 @@ InitPostmasterChild(void)
133135
elog(FATAL, "setsid() failed: %m");
134136
#endif
135137

138+
/* In EXEC_BACKEND case we will not have inherited BlockSig etc values */
139+
#ifdef EXEC_BACKEND
140+
pqinitmask();
141+
#endif
142+
143+
/*
144+
* Every postmaster child process is expected to respond promptly to
145+
* SIGQUIT at all times. Therefore we centrally remove SIGQUIT from
146+
* BlockSig and install a suitable signal handler. (Client-facing
147+
* processes may choose to replace this default choice of handler with
148+
* quickdie().) All other blockable signals remain blocked for now.
149+
*/
150+
pqsignal(SIGQUIT, SignalHandlerForCrashExit);
151+
152+
sigdelset(&BlockSig, SIGQUIT);
153+
PG_SETMASK(&BlockSig);
154+
136155
/* Request a signal if the postmaster dies, if possible. */
137156
PostmasterDeathSignalInit();
138157
}
@@ -155,6 +174,13 @@ InitStandaloneProcess(const char *argv0)
155174
InitLatch(MyLatch);
156175
InitializeLatchWaitSet();
157176

177+
/*
178+
* For consistency with InitPostmasterChild, initialize signal mask here.
179+
* But we don't unblock SIGQUIT or provide a default handler for it.
180+
*/
181+
pqinitmask();
182+
PG_SETMASK(&BlockSig);
183+
158184
/* Compute paths, no postmaster to inherit from */
159185
if (my_exec_path[0] == '\0')
160186
{

0 commit comments

Comments
 (0)