On Thu, Apr 15, 2021 at 10:50 PM Thomas Munro <thomas.mu...@gmail.com> wrote: > On Thu, Apr 15, 2021 at 2:26 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > > It's possible that that argument doesn't apply to the way SIGURG is used > > in this patch, but I don't see a good reason to ignore the convention of > > setting up the handler this way. > > Yeah, will fix. I don't think there is a bug here given the way > latches use shared memory flags, but it might as well be consistent.
Here's a patch to change that. But... on second thoughts, and after coming up with a commit message to explain the change, I'm not 100% convinced it's worth committing. You can't get SIGURG without explicitly asking for it (by setting maybe_sleeping), which makes it a bit more like SIGALRM than SIGUSR2. I don't feel very strongly about this though. What do you think?
From 74bc13551fcdc208d4a5e386a01776def602251e Mon Sep 17 00:00:00 2001 From: Thomas Munro <thomas.mu...@gmail.com> Date: Fri, 16 Apr 2021 15:36:43 +1200 Subject: [PATCH] Make postmaster's SIGURG setup more consistent. Although the only current user of SIGURG isn't at risk of missing an important signal (latch.c doesn't expect SIGURG until it advertises in shmem that it's waiting for it and does even that only after checking that is_set hasn't been set), it's theoretically possible that we could use SIGURG for other purposes in the future. Therefore, give it the same treatment as SIGUSR2, just in case: install a dummy handler before forking, so that any signals are queued until it's unblocked by each child. Reported-by: Tom Lane <t...@sss.pgh.pa.us> Discussion: https://postgr.es/m/3739816.1618453573%40sss.pgh.pa.us --- src/backend/postmaster/postmaster.c | 2 +- src/backend/storage/ipc/latch.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 4a3ca78c1b..a739b461b7 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -660,7 +660,7 @@ PostmasterMain(int argc, char *argv[]) pqsignal_pm(SIGCHLD, reaper); /* handle child termination */ #ifdef SIGURG - pqsignal_pm(SIGURG, SIG_IGN); /* ignored */ + pqsignal_pm(SIGURG, dummy_handler); /* unused, reserve for children */ #endif /* diff --git a/src/backend/storage/ipc/latch.c b/src/backend/storage/ipc/latch.c index 5f3318fa8f..05b364fa9e 100644 --- a/src/backend/storage/ipc/latch.c +++ b/src/backend/storage/ipc/latch.c @@ -268,7 +268,8 @@ InitializeLatchSupport(void) #ifdef WAIT_USE_EPOLL sigset_t signalfd_mask; - /* Block SIGURG, because we'll receive it through a signalfd. */ + /* Ignore SIGURG, and keep it permanently blocked. */ + pqsignal(SIGURG, SIG_IGN); sigaddset(&UnBlockSig, SIGURG); /* Set up the signalfd to receive SIGURG notifications. */ -- 2.30.1
