From 214c7a4f0b1784ce855512c2961b09c9f51dafd8 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 1 Feb 2014 16:20:56 -0500 Subject: [PATCH] Fix some more bugs in signal handlers and process shutdown logic. WalSndKill was doing things exactly backwards: it should first clear MyWalSnd (to stop signal handlers from touching MyWalSnd->latch), then disown the latch, and only then mark the WalSnd struct unused by clearing its pid field. Also, WalRcvSigUsr1Handler and worker_spi_sighup failed to preserve errno, which is surely a requirement for any signal handler. Per discussion of recent buildfarm failures. Back-patch as far as the relevant code exists. --- contrib/worker_spi/worker_spi.c | 7 ++++++- src/backend/replication/walreceiver.c | 4 ++++ src/backend/replication/walsender.c | 18 ++++++++++++------ 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/contrib/worker_spi/worker_spi.c b/contrib/worker_spi/worker_spi.c index 23ace7a8af..560be21814 100644 --- a/contrib/worker_spi/worker_spi.c +++ b/contrib/worker_spi/worker_spi.c @@ -42,6 +42,7 @@ #include "tcop/utility.h" PG_MODULE_MAGIC; + PG_FUNCTION_INFO_V1(worker_spi_launch); void _PG_init(void); @@ -82,15 +83,19 @@ worker_spi_sigterm(SIGNAL_ARGS) /* * Signal handler for SIGHUP - * Set a flag to let the main loop to reread the config file, and set + * Set a flag to tell the main loop to reread the config file, and set * our latch to wake it up. */ static void worker_spi_sighup(SIGNAL_ARGS) { + int save_errno = errno; + got_sighup = true; if (MyProc) SetLatch(&MyProc->procLatch); + + errno = save_errno; } /* diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c index cc3d775307..e31977eee0 100644 --- a/src/backend/replication/walreceiver.c +++ b/src/backend/replication/walreceiver.c @@ -740,7 +740,11 @@ WalRcvSigHupHandler(SIGNAL_ARGS) static void WalRcvSigUsr1Handler(SIGNAL_ARGS) { + int save_errno = errno; + latch_sigusr1_handler(); + + errno = save_errno; } /* SIGTERM: set flag for main loop, or shutdown immediately if safe */ diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c index 119a920af2..a661d88277 100644 --- a/src/backend/replication/walsender.c +++ b/src/backend/replication/walsender.c @@ -1401,17 +1401,23 @@ InitWalSenderSlot(void) static void WalSndKill(int code, Datum arg) { - Assert(MyWalSnd != NULL); + WalSnd *walsnd = MyWalSnd; + + Assert(walsnd != NULL); + + /* + * Clear MyWalSnd first; then disown the latch. This is so that signal + * handlers won't try to touch the latch after it's no longer ours. + */ + MyWalSnd = NULL; + + DisownLatch(&walsnd->latch); /* * Mark WalSnd struct no longer in use. Assume that no lock is required * for this. */ - MyWalSnd->pid = 0; - DisownLatch(&MyWalSnd->latch); - - /* WalSnd struct isn't mine anymore */ - MyWalSnd = NULL; + walsnd->pid = 0; } /*