Centralize logic for restoring errno in signal handlers.

Presently, we rely on each individual signal handler to save the
initial value of errno and then restore it before returning if
needed.  This is easily forgotten and, if missed, often goes
undetected for a long time.

In commit 3b00fdba9f, we introduced a wrapper signal handler
function that checks whether MyProcPid matches getpid().  This
commit moves the aforementioned errno restoration code from the
individual signal handlers to the new wrapper handler so that we no
longer need to worry about missing it.

Reviewed-by: Andres Freund, Noah Misch
Discussion: https://postgr.es/m/20231121212008.GA3742740%40nathanxps13
This commit is contained in:
Nathan Bossart 2024-02-14 16:34:18 -06:00
parent 3b00fdba9f
commit 28e4632509
15 changed files with 6 additions and 87 deletions

View File

@ -1007,18 +1007,10 @@ MemoryContextSwitchTo(MemoryContext context)
static void
handle_sighup(SIGNAL_ARGS)
{
int save_errno = errno;
got_SIGHUP = true;
SetLatch(MyLatch);
errno = save_errno;
}
</programlisting>
<varname>errno</varname> is saved and restored because
<function>SetLatch()</function> might change it. If that were not done
interrupted code that's currently inspecting <varname>errno</varname> might see the wrong
value.
</para>
</simplesect>

View File

@ -1410,12 +1410,8 @@ AutoVacWorkerFailed(void)
static void
avl_sigusr2_handler(SIGNAL_ARGS)
{
int save_errno = errno;
got_SIGUSR2 = true;
SetLatch(MyLatch);
errno = save_errno;
}

View File

@ -852,15 +852,11 @@ IsCheckpointOnSchedule(double progress)
static void
ReqCheckpointHandler(SIGNAL_ARGS)
{
int save_errno = errno;
/*
* The signaling process should have set ckpt_flags nonzero, so all we
* need do is ensure that our main loop gets kicked out of any wait.
*/
SetLatch(MyLatch);
errno = save_errno;
}

View File

@ -60,12 +60,8 @@ HandleMainLoopInterrupts(void)
void
SignalHandlerForConfigReload(SIGNAL_ARGS)
{
int save_errno = errno;
ConfigReloadPending = true;
SetLatch(MyLatch);
errno = save_errno;
}
/*
@ -108,10 +104,6 @@ SignalHandlerForCrashExit(SIGNAL_ARGS)
void
SignalHandlerForShutdownRequest(SIGNAL_ARGS)
{
int save_errno = errno;
ShutdownRequestPending = true;
SetLatch(MyLatch);
errno = save_errno;
}

View File

@ -283,13 +283,9 @@ PgArchWakeup(void)
static void
pgarch_waken_stop(SIGNAL_ARGS)
{
int save_errno = errno;
/* set flag to do a final cycle and shut down afterwards */
ready_to_stop = true;
SetLatch(MyLatch);
errno = save_errno;
}
/*

View File

@ -2612,12 +2612,8 @@ InitProcessGlobals(void)
static void
handle_pm_pmsignal_signal(SIGNAL_ARGS)
{
int save_errno = errno;
pending_pm_pmsignal = true;
SetLatch(MyLatch);
errno = save_errno;
}
/*
@ -2626,12 +2622,8 @@ handle_pm_pmsignal_signal(SIGNAL_ARGS)
static void
handle_pm_reload_request_signal(SIGNAL_ARGS)
{
int save_errno = errno;
pending_pm_reload_request = true;
SetLatch(MyLatch);
errno = save_errno;
}
/*
@ -2711,8 +2703,6 @@ process_pm_reload_request(void)
static void
handle_pm_shutdown_request_signal(SIGNAL_ARGS)
{
int save_errno = errno;
switch (postgres_signal_arg)
{
case SIGTERM:
@ -2729,8 +2719,6 @@ handle_pm_shutdown_request_signal(SIGNAL_ARGS)
break;
}
SetLatch(MyLatch);
errno = save_errno;
}
/*
@ -2890,12 +2878,8 @@ process_pm_shutdown_request(void)
static void
handle_pm_child_exit_signal(SIGNAL_ARGS)
{
int save_errno = errno;
pending_pm_child_exit = true;
SetLatch(MyLatch);
errno = save_errno;
}
/*

View File

@ -95,32 +95,22 @@ static void StartupProcExit(int code, Datum arg);
static void
StartupProcTriggerHandler(SIGNAL_ARGS)
{
int save_errno = errno;
promote_signaled = true;
WakeupRecovery();
errno = save_errno;
}
/* SIGHUP: set flag to re-read config file at next convenient time */
static void
StartupProcSigHupHandler(SIGNAL_ARGS)
{
int save_errno = errno;
got_SIGHUP = true;
WakeupRecovery();
errno = save_errno;
}
/* SIGTERM: set flag to abort redo and exit */
static void
StartupProcShutdownHandler(SIGNAL_ARGS)
{
int save_errno = errno;
if (in_restore_command)
{
/*
@ -139,8 +129,6 @@ StartupProcShutdownHandler(SIGNAL_ARGS)
else
shutdown_requested = true;
WakeupRecovery();
errno = save_errno;
}
/*

View File

@ -1642,10 +1642,6 @@ RemoveLogrotateSignalFiles(void)
static void
sigUsr1Handler(SIGNAL_ARGS)
{
int save_errno = errno;
rotation_requested = true;
SetLatch(MyLatch);
errno = save_errno;
}

View File

@ -3476,12 +3476,8 @@ HandleWalSndInitStopping(void)
static void
WalSndLastCycleHandler(SIGNAL_ARGS)
{
int save_errno = errno;
got_SIGUSR2 = true;
SetLatch(MyLatch);
errno = save_errno;
}
/* Set up signal handlers */

View File

@ -2243,12 +2243,8 @@ GetNumRegisteredWaitEvents(WaitEventSet *set)
static void
latch_sigurg_handler(SIGNAL_ARGS)
{
int save_errno = errno;
if (waiting)
sendSelfPipeByte();
errno = save_errno;
}
/* Send one byte to the self-pipe, to wake up WaitLatch */

View File

@ -638,8 +638,6 @@ CheckProcSignal(ProcSignalReason reason)
void
procsignal_sigusr1_handler(SIGNAL_ARGS)
{
int save_errno = errno;
if (CheckProcSignal(PROCSIG_CATCHUP_INTERRUPT))
HandleCatchupInterrupt();
@ -683,6 +681,4 @@ procsignal_sigusr1_handler(SIGNAL_ARGS)
HandleRecoveryConflictInterrupt(PROCSIG_RECOVERY_CONFLICT_BUFFERPIN);
SetLatch(MyLatch);
errno = save_errno;
}

View File

@ -2970,8 +2970,6 @@ quickdie(SIGNAL_ARGS)
void
die(SIGNAL_ARGS)
{
int save_errno = errno;
/* Don't joggle the elbow of proc_exit */
if (!proc_exit_inprogress)
{
@ -2993,8 +2991,6 @@ die(SIGNAL_ARGS)
*/
if (DoingCommandRead && whereToSendOutput != DestRemote)
ProcessInterrupts();
errno = save_errno;
}
/*
@ -3004,8 +3000,6 @@ die(SIGNAL_ARGS)
void
StatementCancelHandler(SIGNAL_ARGS)
{
int save_errno = errno;
/*
* Don't joggle the elbow of proc_exit
*/
@ -3017,8 +3011,6 @@ StatementCancelHandler(SIGNAL_ARGS)
/* If we're still here, waken anything waiting on the process latch */
SetLatch(MyLatch);
errno = save_errno;
}
/* signal handler for floating point exception */

View File

@ -363,8 +363,6 @@ schedule_alarm(TimestampTz now)
static void
handle_sig_alarm(SIGNAL_ARGS)
{
int save_errno = errno;
/*
* Bump the holdoff counter, to make sure nothing we call will process
* interrupts directly. No timeout handler should do that, but these
@ -452,8 +450,6 @@ handle_sig_alarm(SIGNAL_ARGS)
}
RESUME_INTERRUPTS();
errno = save_errno;
}

View File

@ -152,7 +152,6 @@ ResetCancelConn(void)
static void
handle_sigint(SIGNAL_ARGS)
{
int save_errno = errno;
char errbuf[256];
CancelRequested = true;
@ -173,8 +172,6 @@ handle_sigint(SIGNAL_ARGS)
write_stderr(errbuf);
}
}
errno = save_errno; /* just in case the write changed it */
}
/*

View File

@ -80,10 +80,14 @@ static volatile pqsigfunc pqsignal_handlers[PG_NSIG];
* processes do not modify shared memory, which is often detrimental. If the
* check succeeds, the function originally provided to pqsignal() is called.
* Otherwise, the default signal handler is installed and then called.
*
* This wrapper also handles restoring the value of errno.
*/
static void
wrapper_handler(SIGNAL_ARGS)
{
int save_errno = errno;
#ifndef FRONTEND
/*
@ -102,6 +106,8 @@ wrapper_handler(SIGNAL_ARGS)
#endif
(*pqsignal_handlers[postgres_signal_arg]) (postgres_signal_arg);
errno = save_errno;
}
/*