Hi folks
A few times lately I've been doing things in extensions that've made me
want to be able to run my own code whenever InterruptPending is true and
CHECK_FOR_INTERRUPTS() calls ProcessInterrupts()
So here's a simple patch to add ProcessInterrupts_hook. It follows the
usual pattern like ProcessUtility_hook and standard_ProcessUtility.
Why? Because sometimes I want most of the behaviour of die(), but the
option to override it with some bgworker-specific choices occasionally.
HOLD_INTERRUPTS() is too big a hammer.
What I really want to go along with this is a way for any backend to
observe the postmaster's pmState and its "Shutdown" variable's value, so
any backend can tell if we're in FastShutdown, SmartShutdown, etc. Copies
in shmem only obviously. But I'm not convinced it's right to just copy
these vars as-is to shmem, and I don't want to use the memory for a
ProcSignal slot for something that won't be relevant for most backends for
most of the postmaster lifetime. Ideas welcomed.
From 1c8c477a5814420011fa034323e82d8eabc6bc5f Mon Sep 17 00:00:00 2001
From: Craig Ringer <craig.rin...@2ndquadrant.com>
Date: Mon, 18 Jan 2021 14:14:46 +0800
Subject: [PATCH v1 1/4] Provide a hook for ProcessInterrupts()
Code may now register a ProcessInterrupts_hook to fire its own
logic before and/or after the main ProcessInterrupts handler
for signal processing. The hook must call standard_ProcessInterrupts
to execute the normal interrupt handling or set InterruptsPending to
true to cause the interrupt to be re-processed at the next opportunity.
This follows the consistent pattern used by other PostgreSQL hooks like
the ProcessUtility_hook and standard_ProcessUtility() function.
The purpose of this hook is to allow extensions to react to their own
custom interrupt flags during CHECK_FOR_INTERRUPTS() calls invoked
in main PostgreSQL code paths.
---
src/backend/tcop/postgres.c | 20 ++++++++++++++++++++
src/include/miscadmin.h | 3 +++
2 files changed, 23 insertions(+)
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 28055680aa..8cf1359e33 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -102,6 +102,8 @@ int max_stack_depth = 100;
/* wait N seconds to allow attach from a debugger */
int PostAuthDelay = 0;
+/* Intercept CHECK_FOR_INTERRUPTS() responses */
+ProcessInterrupts_hook_type ProcessInterrupts_hook = NULL;
/* ----------------
@@ -3064,8 +3066,26 @@ ProcessInterrupts(void)
/* OK to accept any interrupts now? */
if (InterruptHoldoffCount != 0 || CritSectionCount != 0)
return;
+
InterruptPending = false;
+ if (ProcessInterrupts_hook)
+ ProcessInterrupts_hook();
+ else
+ standard_ProcessInterrupts();
+}
+
+/*
+ * Implement the default signal handling behaviour for most backend types
+ * including user backends and bgworkers.
+ *
+ * This is where CHECK_FOR_INTERRUPTS() eventually lands up unless intercepted
+ * by ProcessInterrupts_hook.
+ */
+void
+standard_ProcessInterrupts(void)
+{
+
if (ProcDiePending)
{
ProcDiePending = false;
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 1bdc97e308..f082d04448 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -94,6 +94,9 @@ extern PGDLLIMPORT volatile uint32 CritSectionCount;
/* in tcop/postgres.c */
extern void ProcessInterrupts(void);
+typedef void (*ProcessInterrupts_hook_type)(void);
+extern ProcessInterrupts_hook_type ProcessInterrupts_hook;
+extern void standard_ProcessInterrupts(void);
#ifndef WIN32
--
2.29.2
From 58b9ac884ef5bd35a2aac9da85079e24097612be Mon Sep 17 00:00:00 2001
From: Craig Ringer <craig.rin...@2ndquadrant.com>
Date: Mon, 18 Jan 2021 15:26:43 +0800
Subject: [PATCH v1 2/4] Test for ProcessInterrupts_hook
---
src/test/modules/Makefile | 3 +-
src/test/modules/test_hooks/.gitignore | 4 +
src/test/modules/test_hooks/Makefile | 28 ++++
.../expected/test_processinterrupt_hook.out | 0
src/test/modules/test_hooks/hooks.conf | 1 +
.../sql/test_processinterrupt_hook.sql | 16 ++
.../modules/test_hooks/test_hooks--1.0.sql | 8 +
src/test/modules/test_hooks/test_hooks.c | 37 +++++
.../modules/test_hooks/test_hooks.control | 4 +
src/test/modules/test_hooks/test_hooks.h | 20 +++
.../test_hooks/test_process_interrupts_hook.c | 140 ++++++++++++++++++
11 files changed, 260 insertions(+), 1 deletion(-)
create mode 100644 src/test/modules/test_hooks/.gitignore
create mode 100644 src/test/modules/test_hooks/Makefile
create mode 100644 src/test/modules/test_hooks/expected/test_processinterrupt_hook.out
create mode 100644 src/test/modules/test_hooks/hooks.conf
create mode 100644 src/test/modules/test_hooks/sql/test_processinterrupt_hook.sql
create mode 100644 src/test/modules/test_hooks/test_hooks--1.0.sql
create mode 100644 src/test/modules/test_hooks/test_hooks.c
create mode 100644 src/test/modules/test_hooks/test_hooks.control
create mode 100644 src/test/modules/test_hooks/test_hooks.h
create mode 100644 src/test/modules/test_hooks/test_process_interrupts_hook.c
diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile
index 59921b46cf..b6f1f25451 100644
--- a/src/test/modules/Makefile
+++ b/src/test/modules/Makefile
@@ -26,7 +26,8 @@ SUBDIRS = \
test_rls_hooks \
test_shm_mq \
unsafe_tests \
- worker_spi
+ worker_spi \
+ test_hooks
ifeq ($(with_openssl),yes)
SUBDIRS += ssl_passphrase_callback
diff --git a/src/test/modules/test_hooks/.gitignore b/src/test/modules/test_hooks/.gitignore
new file mode 100644
index 0000000000..5dcb3ff972
--- /dev/null
+++ b/src/test/modules/test_hooks/.gitignore
@@ -0,0 +1,4 @@
+# Generated subdirectories
+/log/
+/results/
+/tmp_check/
diff --git a/src/test/modules/test_hooks/Makefile b/src/test/modules/test_hooks/Makefile
new file mode 100644
index 0000000000..50be27ea34
--- /dev/null
+++ b/src/test/modules/test_hooks/Makefile
@@ -0,0 +1,28 @@
+# src/test/modules/test_hooks/Makefile
+
+MODULE_big = test_hooks
+OBJS = \
+ $(WIN32RES) \
+ test_hooks.o \
+ test_process_interrupts_hook.o
+PGFILEDESC = "test_hooks - example use of assorted PostgreSQL hooks"
+
+EXTENSION = test_hooks
+DATA = test_hooks--1.0.sql
+
+REGRESS = test_processinterrupt_hook
+REGRESS_OPTS = --temp-config=$(top_srcdir)/src/test/modules/test_hooks/hooks.conf
+# Disabled because these tests require "shared_preload_libraries=test_hooks",
+# which typical installcheck users do not have (e.g. buildfarm clients).
+NO_INSTALLCHECK = 1
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = src/test/modules/test_hooks
+top_builddir = ../../../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
diff --git a/src/test/modules/test_hooks/expected/test_processinterrupt_hook.out b/src/test/modules/test_hooks/expected/test_processinterrupt_hook.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/src/test/modules/test_hooks/hooks.conf b/src/test/modules/test_hooks/hooks.conf
new file mode 100644
index 0000000000..d574606694
--- /dev/null
+++ b/src/test/modules/test_hooks/hooks.conf
@@ -0,0 +1 @@
+shared_preload_libraries = test_hooks
diff --git a/src/test/modules/test_hooks/sql/test_processinterrupt_hook.sql b/src/test/modules/test_hooks/sql/test_processinterrupt_hook.sql
new file mode 100644
index 0000000000..4de9712346
--- /dev/null
+++ b/src/test/modules/test_hooks/sql/test_processinterrupt_hook.sql
@@ -0,0 +1,16 @@
+CREATE EXTENSION test_hooks;
+
+SELECT get_times_processed_interrupts() > 0;
+
+SELECT test_hold_interrupts_hook();
+
+SELECT set_exit_deferred(true);
+SELECT pg_terminate_backend(pg_backend_pid());
+SELECT 1;
+SELECT set_exit_deferred(false);
+SELECT 1;
+
+SELECT pg_backend_pid();
+SELECT pg_terminate_backend(pg_backend_pid());
+SELECT pg_backend_pid();
+SELECT 1;
diff --git a/src/test/modules/test_hooks/test_hooks--1.0.sql b/src/test/modules/test_hooks/test_hooks--1.0.sql
new file mode 100644
index 0000000000..5a9ea757a7
--- /dev/null
+++ b/src/test/modules/test_hooks/test_hooks--1.0.sql
@@ -0,0 +1,8 @@
+CREATE FUNCTION get_times_processed_interrupts()
+RETURNS pg_catalog.int4 LANGUAGE c VOLATILE AS 'MODULE_PATHNAME';
+
+CREATE FUNCTION set_exit_deferred(pg_catalog.bool)
+RETURNS pg_catalog.void LANGUAGE c VOLATILE AS 'MODULE_PATHNAME';
+
+CREATE FUNCTION test_hold_interrupts_hook()
+RETURNS pg_catalog.void LANGUAGE c VOLATILE AS 'MODULE_PATHNAME';
diff --git a/src/test/modules/test_hooks/test_hooks.c b/src/test/modules/test_hooks/test_hooks.c
new file mode 100644
index 0000000000..9d5422e2ac
--- /dev/null
+++ b/src/test/modules/test_hooks/test_hooks.c
@@ -0,0 +1,37 @@
+/*--------------------------------------------------------------------------
+ *
+ * test_hooks.c
+ * Code for testing RLS hooks.
+ *
+ * Copyright (c) 2015-2021, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ * src/test/modules/test_hooks/test_hooks.c
+ *
+ * -------------------------------------------------------------------------
+ */
+
+#include "postgres.h"
+
+#include "fmgr.h"
+#include "miscadmin.h"
+#include "test_hooks.h"
+
+PG_MODULE_MAGIC;
+
+void _PG_init(void);
+void _PG_fini(void);
+
+/* Install hooks */
+void
+_PG_init(void)
+{
+ install_test_process_interrupts_hook();
+}
+
+/* Uninstall hooks */
+void
+_PG_fini(void)
+{
+ uninstall_test_process_interrupts_hook();
+}
diff --git a/src/test/modules/test_hooks/test_hooks.control b/src/test/modules/test_hooks/test_hooks.control
new file mode 100644
index 0000000000..539f5f0fa3
--- /dev/null
+++ b/src/test/modules/test_hooks/test_hooks.control
@@ -0,0 +1,4 @@
+comment = 'Test code for various PostgreSQL hooks'
+default_version = '1.0'
+module_pathname = '$libdir/test_hooks'
+relocatable = true
diff --git a/src/test/modules/test_hooks/test_hooks.h b/src/test/modules/test_hooks/test_hooks.h
new file mode 100644
index 0000000000..6879a0e3bd
--- /dev/null
+++ b/src/test/modules/test_hooks/test_hooks.h
@@ -0,0 +1,20 @@
+/*--------------------------------------------------------------------------
+ *
+ * test_hooks.h
+ * Definitions for various PostgreSQL hooks
+ *
+ * Copyright (c) 2021, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ * src/test/modules/test_hooks/test_hooks.h
+ *
+ * -------------------------------------------------------------------------
+ */
+
+#ifndef TEST_HOOKS_H
+#define TEST_HOOKS_H
+
+extern void install_test_process_interrupts_hook(void);
+extern void uninstall_test_process_interrupts_hook(void);
+
+#endif
diff --git a/src/test/modules/test_hooks/test_process_interrupts_hook.c b/src/test/modules/test_hooks/test_process_interrupts_hook.c
new file mode 100644
index 0000000000..8f39ad5e91
--- /dev/null
+++ b/src/test/modules/test_hooks/test_process_interrupts_hook.c
@@ -0,0 +1,140 @@
+/*--------------------------------------------------------------------------
+ *
+ * test_process_interrupts_hook.c
+ * Code for testing RLS hooks.
+ *
+ * Copyright (c) 2021, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ * src/test/modules/test_hooks/test_process_interrupts_hook.c
+ *
+ * -------------------------------------------------------------------------
+ */
+
+#include "postgres.h"
+
+#include "fmgr.h"
+#include "storage/latch.h"
+#include "miscadmin.h"
+#include "pgstat.h"
+#include "test_hooks.h"
+
+/* Saved hook values in case of unload */
+static ProcessInterrupts_hook_type original_ProcessInterrupts_hook = NULL;
+
+/* Next hooks to call in the chain */
+static ProcessInterrupts_hook_type next_ProcessInterrupts_hook = NULL;
+
+static int times_processed_interrupts;
+static bool exit_deferred;
+static bool saved_ProcDiePending;
+
+static void
+test_ProcessInterrupts_hook(void)
+{
+ /* Hook never gets to fire if interrupts are held */
+ Assert(InterruptHoldoffCount == 0);
+
+ /*
+ * Typically you'd HOLD_INTERRUPTS() instead of this hack to defer
+ * interrupts. But just as an example, this extension will defer
+ * processing of SIGTERM via die() while acting normally on other
+ * signals.
+ *
+ * It'll react to the die() when exit_deferred=false is set and its
+ * latch is set or it's woken for some other reason, since it does not
+ * restore InterruptsPending=true.
+ *
+ * This relies on running in a normal user backend that uses die()
+ * as its SIGTERM handler.
+ */
+ if (exit_deferred && ProcDiePending)
+ {
+ saved_ProcDiePending = true;
+ ProcDiePending = false;
+ }
+ else if (!exit_deferred && saved_ProcDiePending)
+ {
+ ProcDiePending = true;
+ saved_ProcDiePending = false;
+ }
+
+ if (next_ProcessInterrupts_hook)
+ next_ProcessInterrupts_hook();
+ else
+ standard_ProcessInterrupts();
+
+ times_processed_interrupts ++;
+}
+
+void
+install_test_process_interrupts_hook(void)
+{
+ /* Save values for unload */
+ original_ProcessInterrupts_hook = ProcessInterrupts_hook;
+
+ /* Set our hooks */
+ next_ProcessInterrupts_hook = ProcessInterrupts_hook;
+ ProcessInterrupts_hook = test_ProcessInterrupts_hook;
+
+ times_processed_interrupts = 0;
+ exit_deferred = false;
+}
+
+void
+uninstall_test_process_interrupts_hook(void)
+{
+ if (ProcessInterrupts_hook != test_ProcessInterrupts_hook)
+ elog(ERROR, "cannot unload test_hooks: another hook has been registered on the ProcessInterrupts_hook");
+
+ ProcessInterrupts_hook = original_ProcessInterrupts_hook;
+}
+
+PG_FUNCTION_INFO_V1(get_times_processed_interrupts);
+
+Datum
+get_times_processed_interrupts(PG_FUNCTION_ARGS)
+{
+ PG_RETURN_INT32(times_processed_interrupts);
+}
+
+PG_FUNCTION_INFO_V1(set_exit_deferred);
+
+Datum
+set_exit_deferred(PG_FUNCTION_ARGS)
+{
+ if (PG_ARGISNULL(0))
+ elog(ERROR, "argument must be non-null");
+ exit_deferred = PG_GETARG_BOOL(0);
+ if (!exit_deferred)
+ InterruptPending = true;
+ PG_RETURN_VOID();
+}
+
+PG_FUNCTION_INFO_V1(test_hold_interrupts_hook);
+
+/*
+ * Make sure the hook doesn't fire when interrupts are held
+ */
+Datum
+test_hold_interrupts_hook(PG_FUNCTION_ARGS)
+{
+ int rc;
+
+ HOLD_INTERRUPTS();
+
+ SetLatch(MyLatch);
+
+ CHECK_FOR_INTERRUPTS();
+
+ rc = WaitLatch(MyLatch, WL_LATCH_SET|WL_TIMEOUT|WL_EXIT_ON_PM_DEATH, 1000, PG_WAIT_EXTENSION);
+
+ RESUME_INTERRUPTS();
+
+ if (rc & WL_LATCH_SET)
+ elog(ERROR, "received latch set event with interrupts held");
+ if (rc & WL_TIMEOUT)
+ elog(INFO, "timed out as expected");
+
+ PG_RETURN_VOID();
+}
--
2.29.2
