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

Commitac695b8

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 parentddbb187 commitac695b8

File tree

1 file changed

+73
-31
lines changed

1 file changed

+73
-31
lines changed

‎src/backend/postmaster/postmaster.c‎

Lines changed: 73 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,8 @@ static void SIGHUP_handler(SIGNAL_ARGS);
410410
staticvoidpmdie(SIGNAL_ARGS);
411411
staticvoidreaper(SIGNAL_ARGS);
412412
staticvoidsigusr1_handler(SIGNAL_ARGS);
413-
staticvoidstartup_die(SIGNAL_ARGS);
413+
staticvoidprocess_startup_packet_die(SIGNAL_ARGS);
414+
staticvoidprocess_startup_packet_quickdie(SIGNAL_ARGS);
414415
staticvoiddummy_handler(SIGNAL_ARGS);
415416
staticvoidStartupPacketTimeoutHandler(void);
416417
staticvoidCleanupBackend(intpid,intexitstatus);
@@ -4195,22 +4196,30 @@ BackendInitialize(Port *port)
41954196
whereToSendOutput=DestRemote;/* now safe to ereport to client */
41964197

41974198
/*
4198-
* We arrange for a simple exit(1) if we receive SIGTERM or SIGQUIT or
4199-
* timeout while trying to collect the startup packet. Otherwise the
4200-
* postmaster cannot shutdown the database FAST or IMMED cleanly if a
4201-
* buggy client fails to send the packet promptly. XXX it follows that
4202-
* the remainder of this function must tolerate losing control at any
4203-
* instant. Likewise, any pg_on_exit_callback registered before or during
4204-
* this function must be prepared to execute at any instant between here
4205-
* and the end of this function. Furthermore, affected callbacks execute
4206-
* partially or not at all when a second exit-inducing signal arrives
4207-
* after proc_exit_prepare() decrements on_proc_exit_index. (Thanks to
4208-
* that mechanic, callbacks need not anticipate more than one call.) This
4209-
* is fragile; it ought to instead follow the norm of handling interrupts
4210-
* at selected, safe opportunities.
4199+
* We arrange to do proc_exit(1) if we receive SIGTERM or timeout while
4200+
* trying to collect the startup packet; while SIGQUIT results in
4201+
* _exit(2). Otherwise the postmaster cannot shutdown the database FAST
4202+
* or IMMED cleanly if a buggy client fails to send the packet promptly.
4203+
*
4204+
* XXX this is pretty dangerous; signal handlers should not call anything
4205+
* as complex as proc_exit() directly. We minimize the hazard by not
4206+
* keeping these handlers active for longer than we must. However, it
4207+
* seems necessary to be able to escape out of DNS lookups as well as the
4208+
* startup packet reception proper, so we can't narrow the scope further
4209+
* than is done here.
4210+
*
4211+
* XXX it follows that the remainder of this function must tolerate losing
4212+
* control at any instant. Likewise, any pg_on_exit_callback registered
4213+
* before or during this function must be prepared to execute at any
4214+
* instant between here and the end of this function. Furthermore,
4215+
* affected callbacks execute partially or not at all when a second
4216+
* exit-inducing signal arrives after proc_exit_prepare() decrements
4217+
* on_proc_exit_index. (Thanks to that mechanic, callbacks need not
4218+
* anticipate more than one call.) This is fragile; it ought to instead
4219+
* follow the norm of handling interrupts at selected, safe opportunities.
42114220
*/
4212-
pqsignal(SIGTERM,startup_die);
4213-
pqsignal(SIGQUIT,startup_die);
4221+
pqsignal(SIGTERM,process_startup_packet_die);
4222+
pqsignal(SIGQUIT,process_startup_packet_quickdie);
42144223
InitializeTimeouts();/* establishes SIGALRM handler */
42154224
PG_SETMASK(&StartupBlockSig);
42164225

@@ -4270,8 +4279,8 @@ BackendInitialize(Port *port)
42704279
port->remote_hostname=strdup(remote_host);
42714280

42724281
/*
4273-
* Ready to begin client interaction. We will give up andexit(1) after a
4274-
* time delay, so that a broken client can't hog a connection
4282+
* Ready to begin client interaction. We will give up andproc_exit(1)
4283+
*after atime delay, so that a broken client can't hog a connection
42754284
* indefinitely. PreAuthDelay and any DNS interactions above don't count
42764285
* against the time limit.
42774286
*
@@ -4293,6 +4302,12 @@ BackendInitialize(Port *port)
42934302
*/
42944303
status=ProcessStartupPacket(port, false);
42954304

4305+
/*
4306+
* Disable the timeout, and prevent SIGTERM/SIGQUIT again.
4307+
*/
4308+
disable_timeout(STARTUP_PACKET_TIMEOUT, false);
4309+
PG_SETMASK(&BlockSig);
4310+
42964311
/*
42974312
* Stop here if it was bad or a cancel packet. ProcessStartupPacket
42984313
* already did any appropriate error reporting.
@@ -4318,12 +4333,6 @@ BackendInitialize(Port *port)
43184333
else
43194334
init_ps_display(port->user_name,port->database_name,remote_ps_data,
43204335
update_process_title ?"authentication" :"");
4321-
4322-
/*
4323-
* Disable the timeout, and prevent SIGTERM/SIGQUIT again.
4324-
*/
4325-
disable_timeout(STARTUP_PACKET_TIMEOUT, false);
4326-
PG_SETMASK(&BlockSig);
43274336
}
43284337

43294338

@@ -5198,20 +5207,49 @@ sigusr1_handler(SIGNAL_ARGS)
51985207
}
51995208

52005209
/*
5201-
* SIGTERMor SIGQUITwhile processing startup packet.
5210+
* SIGTERM while processing startup packet.
52025211
* Clean up and exit(1).
52035212
*
5204-
* XXX: possible future improvement: try to send a message indicating
5205-
* why we are disconnecting. Problem is to be sure we don't block while
5206-
* doing so, nor mess up SSL initialization. In practice, if the client
5207-
* has wedged here, it probably couldn't do anything with the message anyway.
5213+
* Running proc_exit() from a signal handler is pretty unsafe, since we
5214+
* can't know what code we've interrupted. But the alternative of using
5215+
* _exit(2) is also unpalatable, since it'd mean that a "fast shutdown"
5216+
* would cause a database crash cycle (forcing WAL replay at restart)
5217+
* if any sessions are in authentication. So we live with it for now.
5218+
*
5219+
* One might be tempted to try to send a message indicating why we are
5220+
* disconnecting. However, that would make this even more unsafe. Also,
5221+
* it seems undesirable to provide clues about the database's state to
5222+
* a client that has not yet completed authentication.
52085223
*/
52095224
staticvoid
5210-
startup_die(SIGNAL_ARGS)
5225+
process_startup_packet_die(SIGNAL_ARGS)
52115226
{
52125227
proc_exit(1);
52135228
}
52145229

5230+
/*
5231+
* SIGQUIT while processing startup packet.
5232+
*
5233+
* Some backend has bought the farm,
5234+
* so we need to stop what we're doing and exit.
5235+
*/
5236+
staticvoid
5237+
process_startup_packet_quickdie(SIGNAL_ARGS)
5238+
{
5239+
/*
5240+
* We DO NOT want to run proc_exit() or atexit() callbacks; they wouldn't
5241+
* be safe to run from a signal handler. Just nail the windows shut and
5242+
* get out of town.
5243+
*
5244+
* Note we do _exit(2) not _exit(1). This is to force the postmaster into
5245+
* a system reset cycle if someone sends a manual SIGQUIT to a random
5246+
* backend. (While it might be safe to do _exit(1), since this session
5247+
* shouldn't have touched shared memory yet, there seems little point in
5248+
* taking any risks.)
5249+
*/
5250+
_exit(2);
5251+
}
5252+
52155253
/*
52165254
* Dummy signal handler
52175255
*
@@ -5228,7 +5266,11 @@ dummy_handler(SIGNAL_ARGS)
52285266

52295267
/*
52305268
* Timeout while processing startup packet.
5231-
* As for startup_die(), we clean up and exit(1).
5269+
* As for process_startup_packet_die(), we clean up and exit(1).
5270+
*
5271+
* This is theoretically just as hazardous as in process_startup_packet_die(),
5272+
* although in practice we're almost certainly waiting for client input,
5273+
* which greatly reduces the risk.
52325274
*/
52335275
staticvoid
52345276
StartupPacketTimeoutHandler(void)

0 commit comments

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