Movatterモバイル変換

[0]ホーム
URL:

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

 Sign up
Appearance settings

Commit58c6fec

Browse files
committed
Use _exit(2) for SIGQUIT during ProcessStartupPacket, too.
Bring the signal handling for startup-packet collection into linewith the policy established in commitsbedadc7 and8e19a82,namely don't risk running atexit callbacks when handling SIGQUIT.Ideally, we'd not do so for SIGTERM or timeout interrupts either,but that change seems a bit too risky for the back branches.For now, just improve the comments in this area to describe the risk.Also relocate where BackendInitialize re-disables these interrupts,to minimize the code span where they're active. This doesn't buya whole lot of safety, but it can't hurt.In passing, rename startup_die() to remove confusion about whetherit is for the startup process.Like the previous commits, back-patch to all supported branches.Discussion:https://postgr.es/m/1850884.1599601164@sss.pgh.pa.us
1 parent34a947c commit58c6fec

File tree

1 file changed

+51
-32
lines changed

1 file changed

+51
-32
lines changed

‎src/backend/postmaster/postmaster.c

Lines changed: 51 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@
112112
#include"postmaster/autovacuum.h"
113113
#include"postmaster/bgworker_internals.h"
114114
#include"postmaster/fork_process.h"
115+
#include"postmaster/interrupt.h"
115116
#include"postmaster/pgarch.h"
116117
#include"postmaster/postmaster.h"
117118
#include"postmaster/syslogger.h"
@@ -405,7 +406,7 @@ static void SIGHUP_handler(SIGNAL_ARGS);
405406
staticvoidpmdie(SIGNAL_ARGS);
406407
staticvoidreaper(SIGNAL_ARGS);
407408
staticvoidsigusr1_handler(SIGNAL_ARGS);
408-
staticvoidstartup_die(SIGNAL_ARGS);
409+
staticvoidprocess_startup_packet_die(SIGNAL_ARGS);
409410
staticvoiddummy_handler(SIGNAL_ARGS);
410411
staticvoidStartupPacketTimeoutHandler(void);
411412
staticvoidCleanupBackend(intpid,intexitstatus);
@@ -4340,22 +4341,30 @@ BackendInitialize(Port *port)
43404341
whereToSendOutput=DestRemote;/* now safe to ereport to client */
43414342

43424343
/*
4343-
* We arrange for a simple exit(1) if we receive SIGTERM or SIGQUIT or
4344-
* timeout while trying to collect the startup packet. Otherwise the
4345-
* postmaster cannot shutdown the database FAST or IMMED cleanly if a
4346-
* buggy client fails to send the packet promptly. XXX it follows that
4347-
* the remainder of this function must tolerate losing control at any
4348-
* instant. Likewise, any pg_on_exit_callback registered before or during
4349-
* this function must be prepared to execute at any instant between here
4350-
* and the end of this function. Furthermore, affected callbacks execute
4351-
* partially or not at all when a second exit-inducing signal arrives
4352-
* after proc_exit_prepare() decrements on_proc_exit_index. (Thanks to
4353-
* that mechanic, callbacks need not anticipate more than one call.) This
4354-
* is fragile; it ought to instead follow the norm of handling interrupts
4355-
* at selected, safe opportunities.
4356-
*/
4357-
pqsignal(SIGTERM,startup_die);
4358-
pqsignal(SIGQUIT,startup_die);
4344+
* We arrange to do proc_exit(1) if we receive SIGTERM or timeout while
4345+
* trying to collect the startup packet; while SIGQUIT results in
4346+
* _exit(2). Otherwise the postmaster cannot shutdown the database FAST
4347+
* or IMMED cleanly if a buggy client fails to send the packet promptly.
4348+
*
4349+
* XXX this is pretty dangerous; signal handlers should not call anything
4350+
* as complex as proc_exit() directly. We minimize the hazard by not
4351+
* keeping these handlers active for longer than we must. However, it
4352+
* seems necessary to be able to escape out of DNS lookups as well as the
4353+
* startup packet reception proper, so we can't narrow the scope further
4354+
* than is done here.
4355+
*
4356+
* XXX it follows that the remainder of this function must tolerate losing
4357+
* control at any instant. Likewise, any pg_on_exit_callback registered
4358+
* before or during this function must be prepared to execute at any
4359+
* instant between here and the end of this function. Furthermore,
4360+
* affected callbacks execute partially or not at all when a second
4361+
* exit-inducing signal arrives after proc_exit_prepare() decrements
4362+
* on_proc_exit_index. (Thanks to that mechanic, callbacks need not
4363+
* anticipate more than one call.) This is fragile; it ought to instead
4364+
* follow the norm of handling interrupts at selected, safe opportunities.
4365+
*/
4366+
pqsignal(SIGTERM,process_startup_packet_die);
4367+
pqsignal(SIGQUIT,SignalHandlerForCrashExit);
43594368
InitializeTimeouts();/* establishes SIGALRM handler */
43604369
PG_SETMASK(&StartupBlockSig);
43614370

@@ -4411,8 +4420,8 @@ BackendInitialize(Port *port)
44114420
port->remote_hostname=strdup(remote_host);
44124421

44134422
/*
4414-
* Ready to begin client interaction. We will give up andexit(1) after a
4415-
* time delay, so that a broken client can't hog a connection
4423+
* Ready to begin client interaction. We will give up andproc_exit(1)
4424+
*after atime delay, so that a broken client can't hog a connection
44164425
* indefinitely. PreAuthDelay and any DNS interactions above don't count
44174426
* against the time limit.
44184427
*
@@ -4434,6 +4443,12 @@ BackendInitialize(Port *port)
44344443
*/
44354444
status=ProcessStartupPacket(port, false, false);
44364445

4446+
/*
4447+
* Disable the timeout, and prevent SIGTERM/SIGQUIT again.
4448+
*/
4449+
disable_timeout(STARTUP_PACKET_TIMEOUT, false);
4450+
PG_SETMASK(&BlockSig);
4451+
44374452
/*
44384453
* Stop here if it was bad or a cancel packet. ProcessStartupPacket
44394454
* already did any appropriate error reporting.
@@ -4459,12 +4474,6 @@ BackendInitialize(Port *port)
44594474
pfree(ps_data.data);
44604475

44614476
set_ps_display("initializing");
4462-
4463-
/*
4464-
* Disable the timeout, and prevent SIGTERM/SIGQUIT again.
4465-
*/
4466-
disable_timeout(STARTUP_PACKET_TIMEOUT, false);
4467-
PG_SETMASK(&BlockSig);
44684477
}
44694478

44704479

@@ -5359,16 +5368,22 @@ sigusr1_handler(SIGNAL_ARGS)
53595368
}
53605369

53615370
/*
5362-
* SIGTERMor SIGQUITwhile processing startup packet.
5371+
* SIGTERM while processing startup packet.
53635372
* Clean up and exit(1).
53645373
*
5365-
* XXX: possible future improvement: try to send a message indicating
5366-
* why we are disconnecting. Problem is to be sure we don't block while
5367-
* doing so, nor mess up SSL initialization. In practice, if the client
5368-
* has wedged here, it probably couldn't do anything with the message anyway.
5374+
* Running proc_exit() from a signal handler is pretty unsafe, since we
5375+
* can't know what code we've interrupted. But the alternative of using
5376+
* _exit(2) is also unpalatable, since it'd mean that a "fast shutdown"
5377+
* would cause a database crash cycle (forcing WAL replay at restart)
5378+
* if any sessions are in authentication. So we live with it for now.
5379+
*
5380+
* One might be tempted to try to send a message indicating why we are
5381+
* disconnecting. However, that would make this even more unsafe. Also,
5382+
* it seems undesirable to provide clues about the database's state to
5383+
* a client that has not yet completed authentication.
53695384
*/
53705385
staticvoid
5371-
startup_die(SIGNAL_ARGS)
5386+
process_startup_packet_die(SIGNAL_ARGS)
53725387
{
53735388
proc_exit(1);
53745389
}
@@ -5389,7 +5404,11 @@ dummy_handler(SIGNAL_ARGS)
53895404

53905405
/*
53915406
* Timeout while processing startup packet.
5392-
* As for startup_die(), we clean up and exit(1).
5407+
* As for process_startup_packet_die(), we clean up and exit(1).
5408+
*
5409+
* This is theoretically just as hazardous as in process_startup_packet_die(),
5410+
* although in practice we're almost certainly waiting for client input,
5411+
* which greatly reduces the risk.
53935412
*/
53945413
staticvoid
53955414
StartupPacketTimeoutHandler(void)

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp