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

Commit4e10c0c

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 parent697c8e6 commit4e10c0c

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
@@ -401,7 +401,8 @@ static void SIGHUP_handler(SIGNAL_ARGS);
401401
staticvoidpmdie(SIGNAL_ARGS);
402402
staticvoidreaper(SIGNAL_ARGS);
403403
staticvoidsigusr1_handler(SIGNAL_ARGS);
404-
staticvoidstartup_die(SIGNAL_ARGS);
404+
staticvoidprocess_startup_packet_die(SIGNAL_ARGS);
405+
staticvoidprocess_startup_packet_quickdie(SIGNAL_ARGS);
405406
staticvoiddummy_handler(SIGNAL_ARGS);
406407
staticvoidStartupPacketTimeoutHandler(void);
407408
staticvoidCleanupBackend(intpid,intexitstatus);
@@ -4258,22 +4259,30 @@ BackendInitialize(Port *port)
42584259
whereToSendOutput=DestRemote;/* now safe to ereport to client */
42594260

42604261
/*
4261-
* We arrange for a simple exit(1) if we receive SIGTERM or SIGQUIT or
4262-
* timeout while trying to collect the startup packet. Otherwise the
4263-
* postmaster cannot shutdown the database FAST or IMMED cleanly if a
4264-
* buggy client fails to send the packet promptly. XXX it follows that
4265-
* the remainder of this function must tolerate losing control at any
4266-
* instant. Likewise, any pg_on_exit_callback registered before or during
4267-
* this function must be prepared to execute at any instant between here
4268-
* and the end of this function. Furthermore, affected callbacks execute
4269-
* partially or not at all when a second exit-inducing signal arrives
4270-
* after proc_exit_prepare() decrements on_proc_exit_index. (Thanks to
4271-
* that mechanic, callbacks need not anticipate more than one call.) This
4272-
* is fragile; it ought to instead follow the norm of handling interrupts
4273-
* at selected, safe opportunities.
4262+
* We arrange to do proc_exit(1) if we receive SIGTERM or timeout while
4263+
* trying to collect the startup packet; while SIGQUIT results in
4264+
* _exit(2). Otherwise the postmaster cannot shutdown the database FAST
4265+
* or IMMED cleanly if a buggy client fails to send the packet promptly.
4266+
*
4267+
* XXX this is pretty dangerous; signal handlers should not call anything
4268+
* as complex as proc_exit() directly. We minimize the hazard by not
4269+
* keeping these handlers active for longer than we must. However, it
4270+
* seems necessary to be able to escape out of DNS lookups as well as the
4271+
* startup packet reception proper, so we can't narrow the scope further
4272+
* than is done here.
4273+
*
4274+
* XXX it follows that the remainder of this function must tolerate losing
4275+
* control at any instant. Likewise, any pg_on_exit_callback registered
4276+
* before or during this function must be prepared to execute at any
4277+
* instant between here and the end of this function. Furthermore,
4278+
* affected callbacks execute partially or not at all when a second
4279+
* exit-inducing signal arrives after proc_exit_prepare() decrements
4280+
* on_proc_exit_index. (Thanks to that mechanic, callbacks need not
4281+
* anticipate more than one call.) This is fragile; it ought to instead
4282+
* follow the norm of handling interrupts at selected, safe opportunities.
42744283
*/
4275-
pqsignal(SIGTERM,startup_die);
4276-
pqsignal(SIGQUIT,startup_die);
4284+
pqsignal(SIGTERM,process_startup_packet_die);
4285+
pqsignal(SIGQUIT,process_startup_packet_quickdie);
42774286
InitializeTimeouts();/* establishes SIGALRM handler */
42784287
PG_SETMASK(&StartupBlockSig);
42794288

@@ -4333,8 +4342,8 @@ BackendInitialize(Port *port)
43334342
port->remote_hostname=strdup(remote_host);
43344343

43354344
/*
4336-
* Ready to begin client interaction. We will give up andexit(1) after a
4337-
* time delay, so that a broken client can't hog a connection
4345+
* Ready to begin client interaction. We will give up andproc_exit(1)
4346+
*after atime delay, so that a broken client can't hog a connection
43384347
* indefinitely. PreAuthDelay and any DNS interactions above don't count
43394348
* against the time limit.
43404349
*
@@ -4356,6 +4365,12 @@ BackendInitialize(Port *port)
43564365
*/
43574366
status=ProcessStartupPacket(port, false, false);
43584367

4368+
/*
4369+
* Disable the timeout, and prevent SIGTERM/SIGQUIT again.
4370+
*/
4371+
disable_timeout(STARTUP_PACKET_TIMEOUT, false);
4372+
PG_SETMASK(&BlockSig);
4373+
43594374
/*
43604375
* Stop here if it was bad or a cancel packet. ProcessStartupPacket
43614376
* already did any appropriate error reporting.
@@ -4381,12 +4396,6 @@ BackendInitialize(Port *port)
43814396
else
43824397
init_ps_display(port->user_name,port->database_name,remote_ps_data,
43834398
update_process_title ?"authentication" :"");
4384-
4385-
/*
4386-
* Disable the timeout, and prevent SIGTERM/SIGQUIT again.
4387-
*/
4388-
disable_timeout(STARTUP_PACKET_TIMEOUT, false);
4389-
PG_SETMASK(&BlockSig);
43904399
}
43914400

43924401

@@ -5267,20 +5276,49 @@ sigusr1_handler(SIGNAL_ARGS)
52675276
}
52685277

52695278
/*
5270-
* SIGTERMor SIGQUITwhile processing startup packet.
5279+
* SIGTERM while processing startup packet.
52715280
* Clean up and exit(1).
52725281
*
5273-
* XXX: possible future improvement: try to send a message indicating
5274-
* why we are disconnecting. Problem is to be sure we don't block while
5275-
* doing so, nor mess up SSL initialization. In practice, if the client
5276-
* has wedged here, it probably couldn't do anything with the message anyway.
5282+
* Running proc_exit() from a signal handler is pretty unsafe, since we
5283+
* can't know what code we've interrupted. But the alternative of using
5284+
* _exit(2) is also unpalatable, since it'd mean that a "fast shutdown"
5285+
* would cause a database crash cycle (forcing WAL replay at restart)
5286+
* if any sessions are in authentication. So we live with it for now.
5287+
*
5288+
* One might be tempted to try to send a message indicating why we are
5289+
* disconnecting. However, that would make this even more unsafe. Also,
5290+
* it seems undesirable to provide clues about the database's state to
5291+
* a client that has not yet completed authentication.
52775292
*/
52785293
staticvoid
5279-
startup_die(SIGNAL_ARGS)
5294+
process_startup_packet_die(SIGNAL_ARGS)
52805295
{
52815296
proc_exit(1);
52825297
}
52835298

5299+
/*
5300+
* SIGQUIT while processing startup packet.
5301+
*
5302+
* Some backend has bought the farm,
5303+
* so we need to stop what we're doing and exit.
5304+
*/
5305+
staticvoid
5306+
process_startup_packet_quickdie(SIGNAL_ARGS)
5307+
{
5308+
/*
5309+
* We DO NOT want to run proc_exit() or atexit() callbacks; they wouldn't
5310+
* be safe to run from a signal handler. Just nail the windows shut and
5311+
* get out of town.
5312+
*
5313+
* Note we do _exit(2) not _exit(1). This is to force the postmaster into
5314+
* a system reset cycle if someone sends a manual SIGQUIT to a random
5315+
* backend. (While it might be safe to do _exit(1), since this session
5316+
* shouldn't have touched shared memory yet, there seems little point in
5317+
* taking any risks.)
5318+
*/
5319+
_exit(2);
5320+
}
5321+
52845322
/*
52855323
* Dummy signal handler
52865324
*
@@ -5297,7 +5335,11 @@ dummy_handler(SIGNAL_ARGS)
52975335

52985336
/*
52995337
* Timeout while processing startup packet.
5300-
* As for startup_die(), we clean up and exit(1).
5338+
* As for process_startup_packet_die(), we clean up and exit(1).
5339+
*
5340+
* This is theoretically just as hazardous as in process_startup_packet_die(),
5341+
* although in practice we're almost certainly waiting for client input,
5342+
* which greatly reduces the risk.
53015343
*/
53025344
staticvoid
53035345
StartupPacketTimeoutHandler(void)

0 commit comments

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