Use _exit(2) for SIGQUIT during ProcessStartupPacket, too.
Bring the signal handling for startup-packet collection into line with the policy established in commitsbedadc732
and8e19a8264
, 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 buy a whole lot of safety, but it can't hurt. In passing, rename startup_die() to remove confusion about whether it 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
This commit is contained in:
parent
697c8e6204
commit
4e10c0c8a6
|
@ -401,7 +401,8 @@ static void SIGHUP_handler(SIGNAL_ARGS);
|
|||
static void pmdie(SIGNAL_ARGS);
|
||||
static void reaper(SIGNAL_ARGS);
|
||||
static void sigusr1_handler(SIGNAL_ARGS);
|
||||
static void startup_die(SIGNAL_ARGS);
|
||||
static void process_startup_packet_die(SIGNAL_ARGS);
|
||||
static void process_startup_packet_quickdie(SIGNAL_ARGS);
|
||||
static void dummy_handler(SIGNAL_ARGS);
|
||||
static void StartupPacketTimeoutHandler(void);
|
||||
static void CleanupBackend(int pid, int exitstatus);
|
||||
|
@ -4258,22 +4259,30 @@ BackendInitialize(Port *port)
|
|||
whereToSendOutput = DestRemote; /* now safe to ereport to client */
|
||||
|
||||
/*
|
||||
* We arrange for a simple exit(1) if we receive SIGTERM or SIGQUIT or
|
||||
* timeout while trying to collect the startup packet. Otherwise the
|
||||
* postmaster cannot shutdown the database FAST or IMMED cleanly if a
|
||||
* buggy client fails to send the packet promptly. XXX it follows that
|
||||
* the remainder of this function must tolerate losing control at any
|
||||
* instant. Likewise, any pg_on_exit_callback registered before or during
|
||||
* this function must be prepared to execute at any instant between here
|
||||
* and the end of this function. Furthermore, affected callbacks execute
|
||||
* partially or not at all when a second exit-inducing signal arrives
|
||||
* after proc_exit_prepare() decrements on_proc_exit_index. (Thanks to
|
||||
* that mechanic, callbacks need not anticipate more than one call.) This
|
||||
* is fragile; it ought to instead follow the norm of handling interrupts
|
||||
* at selected, safe opportunities.
|
||||
* We arrange to do proc_exit(1) if we receive SIGTERM or timeout while
|
||||
* trying to collect the startup packet; while SIGQUIT results in
|
||||
* _exit(2). Otherwise the postmaster cannot shutdown the database FAST
|
||||
* or IMMED cleanly if a buggy client fails to send the packet promptly.
|
||||
*
|
||||
* XXX this is pretty dangerous; signal handlers should not call anything
|
||||
* as complex as proc_exit() directly. We minimize the hazard by not
|
||||
* keeping these handlers active for longer than we must. However, it
|
||||
* seems necessary to be able to escape out of DNS lookups as well as the
|
||||
* startup packet reception proper, so we can't narrow the scope further
|
||||
* than is done here.
|
||||
*
|
||||
* XXX it follows that the remainder of this function must tolerate losing
|
||||
* control at any instant. Likewise, any pg_on_exit_callback registered
|
||||
* before or during this function must be prepared to execute at any
|
||||
* instant between here and the end of this function. Furthermore,
|
||||
* affected callbacks execute partially or not at all when a second
|
||||
* exit-inducing signal arrives after proc_exit_prepare() decrements
|
||||
* on_proc_exit_index. (Thanks to that mechanic, callbacks need not
|
||||
* anticipate more than one call.) This is fragile; it ought to instead
|
||||
* follow the norm of handling interrupts at selected, safe opportunities.
|
||||
*/
|
||||
pqsignal(SIGTERM, startup_die);
|
||||
pqsignal(SIGQUIT, startup_die);
|
||||
pqsignal(SIGTERM, process_startup_packet_die);
|
||||
pqsignal(SIGQUIT, process_startup_packet_quickdie);
|
||||
InitializeTimeouts(); /* establishes SIGALRM handler */
|
||||
PG_SETMASK(&StartupBlockSig);
|
||||
|
||||
|
@ -4333,8 +4342,8 @@ BackendInitialize(Port *port)
|
|||
port->remote_hostname = strdup(remote_host);
|
||||
|
||||
/*
|
||||
* Ready to begin client interaction. We will give up and exit(1) after a
|
||||
* time delay, so that a broken client can't hog a connection
|
||||
* Ready to begin client interaction. We will give up and proc_exit(1)
|
||||
* after a time delay, so that a broken client can't hog a connection
|
||||
* indefinitely. PreAuthDelay and any DNS interactions above don't count
|
||||
* against the time limit.
|
||||
*
|
||||
|
@ -4356,6 +4365,12 @@ BackendInitialize(Port *port)
|
|||
*/
|
||||
status = ProcessStartupPacket(port, false, false);
|
||||
|
||||
/*
|
||||
* Disable the timeout, and prevent SIGTERM/SIGQUIT again.
|
||||
*/
|
||||
disable_timeout(STARTUP_PACKET_TIMEOUT, false);
|
||||
PG_SETMASK(&BlockSig);
|
||||
|
||||
/*
|
||||
* Stop here if it was bad or a cancel packet. ProcessStartupPacket
|
||||
* already did any appropriate error reporting.
|
||||
|
@ -4381,12 +4396,6 @@ BackendInitialize(Port *port)
|
|||
else
|
||||
init_ps_display(port->user_name, port->database_name, remote_ps_data,
|
||||
update_process_title ? "authentication" : "");
|
||||
|
||||
/*
|
||||
* Disable the timeout, and prevent SIGTERM/SIGQUIT again.
|
||||
*/
|
||||
disable_timeout(STARTUP_PACKET_TIMEOUT, false);
|
||||
PG_SETMASK(&BlockSig);
|
||||
}
|
||||
|
||||
|
||||
|
@ -5267,20 +5276,49 @@ sigusr1_handler(SIGNAL_ARGS)
|
|||
}
|
||||
|
||||
/*
|
||||
* SIGTERM or SIGQUIT while processing startup packet.
|
||||
* SIGTERM while processing startup packet.
|
||||
* Clean up and exit(1).
|
||||
*
|
||||
* XXX: possible future improvement: try to send a message indicating
|
||||
* why we are disconnecting. Problem is to be sure we don't block while
|
||||
* doing so, nor mess up SSL initialization. In practice, if the client
|
||||
* has wedged here, it probably couldn't do anything with the message anyway.
|
||||
* Running proc_exit() from a signal handler is pretty unsafe, since we
|
||||
* can't know what code we've interrupted. But the alternative of using
|
||||
* _exit(2) is also unpalatable, since it'd mean that a "fast shutdown"
|
||||
* would cause a database crash cycle (forcing WAL replay at restart)
|
||||
* if any sessions are in authentication. So we live with it for now.
|
||||
*
|
||||
* One might be tempted to try to send a message indicating why we are
|
||||
* disconnecting. However, that would make this even more unsafe. Also,
|
||||
* it seems undesirable to provide clues about the database's state to
|
||||
* a client that has not yet completed authentication.
|
||||
*/
|
||||
static void
|
||||
startup_die(SIGNAL_ARGS)
|
||||
process_startup_packet_die(SIGNAL_ARGS)
|
||||
{
|
||||
proc_exit(1);
|
||||
}
|
||||
|
||||
/*
|
||||
* SIGQUIT while processing startup packet.
|
||||
*
|
||||
* Some backend has bought the farm,
|
||||
* so we need to stop what we're doing and exit.
|
||||
*/
|
||||
static void
|
||||
process_startup_packet_quickdie(SIGNAL_ARGS)
|
||||
{
|
||||
/*
|
||||
* We DO NOT want to run proc_exit() or atexit() callbacks; they wouldn't
|
||||
* be safe to run from a signal handler. Just nail the windows shut and
|
||||
* get out of town.
|
||||
*
|
||||
* Note we do _exit(2) not _exit(1). This is to force the postmaster into
|
||||
* a system reset cycle if someone sends a manual SIGQUIT to a random
|
||||
* backend. (While it might be safe to do _exit(1), since this session
|
||||
* shouldn't have touched shared memory yet, there seems little point in
|
||||
* taking any risks.)
|
||||
*/
|
||||
_exit(2);
|
||||
}
|
||||
|
||||
/*
|
||||
* Dummy signal handler
|
||||
*
|
||||
|
@ -5297,7 +5335,11 @@ dummy_handler(SIGNAL_ARGS)
|
|||
|
||||
/*
|
||||
* Timeout while processing startup packet.
|
||||
* As for startup_die(), we clean up and exit(1).
|
||||
* As for process_startup_packet_die(), we clean up and exit(1).
|
||||
*
|
||||
* This is theoretically just as hazardous as in process_startup_packet_die(),
|
||||
* although in practice we're almost certainly waiting for client input,
|
||||
* which greatly reduces the risk.
|
||||
*/
|
||||
static void
|
||||
StartupPacketTimeoutHandler(void)
|
||||
|
|
Loading…
Reference in New Issue