From 135c1363d0ff2aaa1886d4ed59016c3b610c375a Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathandbossart@gmail.com>
Date: Mon, 18 Apr 2022 15:25:37 -0700
Subject: [PATCH v4 2/2] Add a new shmem_request_hook hook.

Currently, preloaded libraries are expected to request additional
shared memory in _PG_init().  However, it is not unusal for such
requests to depend on MaxBackends, which won't be initialized at
that time.  Such requests could also depend on GUCs that other
modules might change.  This introduces a new hook where modules can
safely use MaxBackends and GUCs to request additional shared
memory.

Furthermore, this change restricts shared memory requests by
preloaded libraries to this hook.  Previously, libraries could
request additional shared memory until the size of the main shared
memory segment was calculated.  Besides decoupling a common library
task from _PG_init(), this ensures that shared memory requests are
only allowed when MaxBackends is initialized and GUCs should not be
changed.  Unlike before, we no longer silently ignore requests
received at invalid times.  Instead, we ERROR if someone tries to
request additional shared memory outside of the hook.

Authors: Julien Rouhaud, Nathan Bossart
Discussion: https://postgr.es/m/20220412210112.GA2065815%40nathanxps13
---
 contrib/pg_prewarm/autoprewarm.c              | 27 ++++++++++++-
 .../pg_stat_statements/pg_stat_statements.c   | 27 +++++++++----
 src/backend/postmaster/postmaster.c           |  5 +++
 src/backend/storage/ipc/ipci.c                | 39 +++++++++++++------
 src/include/storage/ipc.h                     |  2 +
 src/include/storage/shmem.h                   |  1 +
 src/tools/pgindent/typedefs.list              |  1 +
 7 files changed, 81 insertions(+), 21 deletions(-)

diff --git a/contrib/pg_prewarm/autoprewarm.c b/contrib/pg_prewarm/autoprewarm.c
index 45e012a63a..14345d060a 100644
--- a/contrib/pg_prewarm/autoprewarm.c
+++ b/contrib/pg_prewarm/autoprewarm.c
@@ -83,6 +83,7 @@ typedef struct AutoPrewarmSharedState
 } AutoPrewarmSharedState;
 
 void		_PG_init(void);
+void		_PG_fini(void);
 void		autoprewarm_main(Datum main_arg);
 void		autoprewarm_database_main(Datum main_arg);
 
@@ -96,6 +97,8 @@ static void apw_start_database_worker(void);
 static bool apw_init_shmem(void);
 static void apw_detach_shmem(int code, Datum arg);
 static int	apw_compare_blockinfo(const void *p, const void *q);
+static void autoprewarm_shmem_request(void);
+static shmem_request_hook_type prev_shmem_request_hook = NULL;
 
 /* Pointer to shared-memory state. */
 static AutoPrewarmSharedState *apw_state = NULL;
@@ -139,13 +142,35 @@ _PG_init(void)
 
 	MarkGUCPrefixReserved("pg_prewarm");
 
-	RequestAddinShmemSpace(MAXALIGN(sizeof(AutoPrewarmSharedState)));
+	prev_shmem_request_hook = shmem_request_hook;
+	shmem_request_hook = autoprewarm_shmem_request;
 
 	/* Register autoprewarm worker, if enabled. */
 	if (autoprewarm)
 		apw_start_leader_worker();
 }
 
+/*
+ * Module unload callback.
+ */
+void
+_PG_fini(void)
+{
+	shmem_request_hook = prev_shmem_request_hook;
+}
+
+/*
+ * Requests any additional shared memory required for autoprewarm.
+ */
+static void
+autoprewarm_shmem_request(void)
+{
+	if (prev_shmem_request_hook)
+		prev_shmem_request_hook();
+
+	RequestAddinShmemSpace(MAXALIGN(sizeof(AutoPrewarmSharedState)));
+}
+
 /*
  * Main entry point for the leader autoprewarm process.  Per-database workers
  * have a separate entry point.
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index df2ce63790..87b75d779e 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -252,6 +252,7 @@ static int	exec_nested_level = 0;
 static int	plan_nested_level = 0;
 
 /* Saved hook values in case of unload */
+static shmem_request_hook_type prev_shmem_request_hook = NULL;
 static shmem_startup_hook_type prev_shmem_startup_hook = NULL;
 static post_parse_analyze_hook_type prev_post_parse_analyze_hook = NULL;
 static planner_hook_type prev_planner_hook = NULL;
@@ -317,6 +318,7 @@ PG_FUNCTION_INFO_V1(pg_stat_statements_1_10);
 PG_FUNCTION_INFO_V1(pg_stat_statements);
 PG_FUNCTION_INFO_V1(pg_stat_statements_info);
 
+static void pgss_shmem_request(void);
 static void pgss_shmem_startup(void);
 static void pgss_shmem_shutdown(int code, Datum arg);
 static void pgss_post_parse_analyze(ParseState *pstate, Query *query,
@@ -452,17 +454,11 @@ _PG_init(void)
 
 	MarkGUCPrefixReserved("pg_stat_statements");
 
-	/*
-	 * Request additional shared resources.  (These are no-ops if we're not in
-	 * the postmaster process.)  We'll allocate or attach to the shared
-	 * resources in pgss_shmem_startup().
-	 */
-	RequestAddinShmemSpace(pgss_memsize());
-	RequestNamedLWLockTranche("pg_stat_statements", 1);
-
 	/*
 	 * Install hooks.
 	 */
+	prev_shmem_request_hook = shmem_request_hook;
+	shmem_request_hook = pgss_shmem_request;
 	prev_shmem_startup_hook = shmem_startup_hook;
 	shmem_startup_hook = pgss_shmem_startup;
 	prev_post_parse_analyze_hook = post_parse_analyze_hook;
@@ -488,6 +484,7 @@ void
 _PG_fini(void)
 {
 	/* Uninstall hooks. */
+	shmem_request_hook = prev_shmem_request_hook;
 	shmem_startup_hook = prev_shmem_startup_hook;
 	post_parse_analyze_hook = prev_post_parse_analyze_hook;
 	planner_hook = prev_planner_hook;
@@ -498,6 +495,20 @@ _PG_fini(void)
 	ProcessUtility_hook = prev_ProcessUtility;
 }
 
+/*
+ * shmem_request hook: request additional shared resources.  We'll allocate or
+ * attach to the shared resources in pgss_shmem_startup().
+ */
+static void
+pgss_shmem_request(void)
+{
+	if (prev_shmem_request_hook)
+		prev_shmem_request_hook();
+
+	RequestAddinShmemSpace(pgss_memsize());
+	RequestNamedLWLockTranche("pg_stat_statements", 1);
+}
+
 /*
  * shmem_startup hook: allocate or attach to shared memory,
  * then load any pre-existing statistics from file.
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index ce4007bb2c..57663ddc6a 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -1032,6 +1032,11 @@ PostmasterMain(int argc, char *argv[])
 	 */
 	InitializeMaxBackends();
 
+	/*
+	 * Give preloaded libraries a chance to request additional shared memory.
+	 */
+	ProcessShmemRequests();
+
 	/*
 	 * Now that loadable modules have had their chance to request additional
 	 * shared memory, determine the value of any runtime-computed GUCs that
diff --git a/src/backend/storage/ipc/ipci.c b/src/backend/storage/ipc/ipci.c
index 75e456360b..117aacba7d 100644
--- a/src/backend/storage/ipc/ipci.c
+++ b/src/backend/storage/ipc/ipci.c
@@ -52,28 +52,45 @@
 /* GUCs */
 int			shared_memory_type = DEFAULT_SHARED_MEMORY_TYPE;
 
+shmem_request_hook_type shmem_request_hook = NULL;
 shmem_startup_hook_type shmem_startup_hook = NULL;
 
 static Size total_addin_request = 0;
-static bool addin_request_allowed = true;
+static bool addin_request_allowed = false;
 
+/*
+ * ProcessShmemRequests
+ *
+ * Calls to RequestAddinShmemSpace() by preloaded libraries are only allowed in
+ * the shmem_request_hook.
+ */
+void
+ProcessShmemRequests(void)
+{
+	Assert(MaxBackends > 0);
+
+	addin_request_allowed = true;
+
+	if (shmem_request_hook)
+		shmem_request_hook();
+
+	addin_request_allowed = false;
+}
 
 /*
  * RequestAddinShmemSpace
  *		Request that extra shmem space be allocated for use by
  *		a loadable module.
  *
- * This is only useful if called from the _PG_init hook of a library that
- * is loaded into the postmaster via shared_preload_libraries.  Once
- * shared memory has been allocated, calls will be ignored.  (We could
- * raise an error, but it seems better to make it a no-op, so that
- * libraries containing such calls can be reloaded if needed.)
+ * This may only be called via the shmem_request_hook of a library that is
+ * loaded into the postmaster via shared_preload_libraries.  Calls from
+ * elsewhere will ERROR.
  */
 void
 RequestAddinShmemSpace(Size size)
 {
 	if (IsUnderPostmaster || !addin_request_allowed)
-		return;					/* too late */
+		elog(ERROR, "cannot request additional shared memory outside shmem_request_hook");
 	total_addin_request = add_size(total_addin_request, size);
 }
 
@@ -83,9 +100,6 @@ RequestAddinShmemSpace(Size size)
  *
  * If num_semaphores is not NULL, it will be set to the number of semaphores
  * required.
- *
- * Note that this function freezes the additional shared memory request size
- * from loadable modules.
  */
 Size
 CalculateShmemSize(int *num_semaphores)
@@ -93,6 +107,8 @@ CalculateShmemSize(int *num_semaphores)
 	Size		size;
 	int			numSemas;
 
+	Assert(!addin_request_allowed);
+
 	/* Compute number of semaphores we'll need */
 	numSemas = ProcGlobalSemas();
 	numSemas += SpinlockSemas();
@@ -152,8 +168,7 @@ CalculateShmemSize(int *num_semaphores)
 	size = add_size(size, ShmemBackendArraySize());
 #endif
 
-	/* freeze the addin request size and include it */
-	addin_request_allowed = false;
+	/* include additional requested shmem from preload libraries */
 	size = add_size(size, total_addin_request);
 
 	/* might as well round it off to a multiple of a typical page size */
diff --git a/src/include/storage/ipc.h b/src/include/storage/ipc.h
index fade4dbe63..5f2c6683db 100644
--- a/src/include/storage/ipc.h
+++ b/src/include/storage/ipc.h
@@ -19,6 +19,7 @@
 #define IPC_H
 
 typedef void (*pg_on_exit_callback) (int code, Datum arg);
+typedef void (*shmem_request_hook_type) (void);
 typedef void (*shmem_startup_hook_type) (void);
 
 /*----------
@@ -75,6 +76,7 @@ extern void on_exit_reset(void);
 extern void check_on_shmem_exit_lists_are_empty(void);
 
 /* ipci.c */
+extern PGDLLIMPORT shmem_request_hook_type shmem_request_hook;
 extern PGDLLIMPORT shmem_startup_hook_type shmem_startup_hook;
 
 extern Size CalculateShmemSize(int *num_semaphores);
diff --git a/src/include/storage/shmem.h b/src/include/storage/shmem.h
index de9e7c6e73..bdb0acfedf 100644
--- a/src/include/storage/shmem.h
+++ b/src/include/storage/shmem.h
@@ -46,6 +46,7 @@ extern Size add_size(Size s1, Size s2);
 extern Size mul_size(Size s1, Size s2);
 
 /* ipci.c */
+extern void ProcessShmemRequests(void);
 extern void RequestAddinShmemSpace(Size size);
 
 /* size constants for the shmem index table */
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 87ee7bf866..71a97654e0 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -3549,6 +3549,7 @@ shm_mq_result
 shm_toc
 shm_toc_entry
 shm_toc_estimator
+shmem_request_hook_type
 shmem_startup_hook_type
 sig_atomic_t
 sigjmp_buf
-- 
2.25.1

