Centralize logic for restoring errno in signal handlers.
authorNathan Bossart <nathan@postgresql.org>
Wed, 14 Feb 2024 22:34:18 +0000 (16:34 -0600)
committerNathan Bossart <nathan@postgresql.org>
Wed, 14 Feb 2024 22:34:18 +0000 (16:34 -0600)
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

15 files changed:
doc/src/sgml/sources.sgml
src/backend/postmaster/autovacuum.c
src/backend/postmaster/checkpointer.c
src/backend/postmaster/interrupt.c
src/backend/postmaster/pgarch.c
src/backend/postmaster/postmaster.c
src/backend/postmaster/startup.c
src/backend/postmaster/syslogger.c
src/backend/replication/walsender.c
src/backend/storage/ipc/latch.c
src/backend/storage/ipc/procsignal.c
src/backend/tcop/postgres.c
src/backend/utils/misc/timeout.c
src/fe_utils/cancel.c
src/port/pqsignal.c

index 5d1d510f8e74a397f9af5ea19fbb4ff30e845f04..0dae4d9158f09a4a88c1d3536961b898c72ccb6a 100644 (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>
 
index 2c3099f76f17f14ff46b6c1ad6a0b5d36acf979b..c9ce380f0f1e5133abecc2be55da780e3dac22b6 100644 (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;
 }
 
 
index 0646c5f8594798aa93cb12b1992eeed888ecf56f..46197d56f861cd35ea49ccaa0a2850d7f2047ec6 100644 (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;
 }
 
 
index 7c38f5fadf127940709c489390818ee943fd98b5..eedc0980cf113c273c85ba0b0aae9162a940571a 100644 (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;
 }
index 67693b05806365d4c163a29f4605a60abceab976..02814cd2c8fceefd7cbe495dc9139a3445f05bd4 100644 (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;
 }
 
 /*
index a066800a1cfe52cb8263ecbafb0f6969e1287e4f..df945a5ac4dd6378d1c70e7c4e8babcd39ef7f3b 100644 (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;
 }
 
 /*
index d53c37d062cd6bc23450ba52bfd5d7dcf73de976..b6b53cd25f522f3db559649c8c8f8037e081c6da 100644 (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;
 }
 
 /*
index d3b4fc2fe6052c90bb96beb9ff19876cd3603be3..6db280e483e2f6ff7e605d221aefb1515d09bf03 100644 (file)
@@ -1642,10 +1642,6 @@ RemoveLogrotateSignalFiles(void)
 static void
 sigUsr1Handler(SIGNAL_ARGS)
 {
-   int         save_errno = errno;
-
    rotation_requested = true;
    SetLatch(MyLatch);
-
-   errno = save_errno;
 }
index 4e54779a9ebb50ebd21317e7050d9d2b3bd08b55..e5477c1de1b986f99c1ebade0ed49dd0a826da42 100644 (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 */
index 91ede1d0eb213cdd9a27295aaced9e6aad77551b..6386995e6c76fb96120ed95035d742287360126d 100644 (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 */
index e84619e5a586d8990c78d47f6608e8753bb2104a..0f9f90d2c7bdff91f760ab72490e463d36411488 100644 (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;
 }
index 1a34bd3715fb36fe9665779ac74b504a46db6f29..01b5530f0b1e8813e5807dd744475e8e0d8e1f09 100644 (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 */
index aaaad8bb163b2a3fbf62c4978131022b64082e22..4055dd5f8d3043db85563833129be702fd7bed6e 100644 (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;
 }
 
 
index 12f005818d63e90e00caffe5d2ab71da44c7db63..dcff9a85641846b3829e61c329dfd0518c1e983e 100644 (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 */
 }
 
 /*
index 92382b3c348b07a17e10221b6c5c203e2b4ee70a..6ca2d4e20a840f8f767431dff19d19635ef0dd6e 100644 (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;
 }
 
 /*