Injection points: preloading and runtime arguments

From 61e698159acd8d0a374292a75bbbcd3ad2bf8a48 Mon Sep 17 00:00:00 2001
From: Andrey Borodin <amborodin@acm.org>
Date: Mon, 20 May 2024 15:20:31 +0500
Subject: [PATCH] Add multixact CV sleep

---
 src/backend/access/transam/multixact.c        |  5 ++
 .../injection_points--1.0.sql                 |  2 +-
 .../injection_points/injection_points.c       |  2 +-
 src/test/modules/test_slru/Makefile           |  3 +
 src/test/modules/test_slru/meson.build        |  8 +++
 src/test/modules/test_slru/t/001_multixact.pl | 66 +++++++++++++++++++
 src/test/modules/test_slru/test_slru--1.0.sql |  6 ++
 src/test/modules/test_slru/test_slru.c        | 29 ++++++++
 8 files changed, 119 insertions(+), 2 deletions(-)
 create mode 100644 src/test/modules/test_slru/t/001_multixact.pl

diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 54c916e0347..58ec847cf02 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -88,6 +88,7 @@
 #include "storage/proc.h"
 #include "storage/procarray.h"
 #include "utils/fmgrprotos.h"
+#include "utils/injection_point.h"
 #include "utils/guc_hooks.h"
 #include "utils/memutils.h"
 
@@ -825,8 +826,12 @@ MultiXactIdCreateFromMembers(int nmembers, MultiXactMember *members)
 	 * in vacuum.  During vacuum, in particular, it would be unacceptable to
 	 * keep OldestMulti set, in case it runs for long.
 	 */
+	INJECTION_POINT_PRELOAD("GetNewMultiXactId-done");
+
 	multi = GetNewMultiXactId(nmembers, &offset);
 
+	INJECTION_POINT("GetNewMultiXactId-done");
+
 	/* Make an XLOG entry describing the new MXID. */
 	xlrec.mid = multi;
 	xlrec.moff = offset;
diff --git a/src/test/modules/injection_points/injection_points--1.0.sql b/src/test/modules/injection_points/injection_points--1.0.sql
index 29d43cc5566..998a513bffc 100644
--- a/src/test/modules/injection_points/injection_points--1.0.sql
+++ b/src/test/modules/injection_points/injection_points--1.0.sql
@@ -73,4 +73,4 @@ LANGUAGE C STRICT PARALLEL UNSAFE;
 CREATE FUNCTION injection_points_detach(IN point_name TEXT)
 RETURNS void
 AS 'MODULE_PATHNAME', 'injection_points_detach'
-LANGUAGE C STRICT PARALLEL UNSAFE;
+LANGUAGE C STRICT PARALLEL UNSAFE;
\ No newline at end of file
diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c
index 086b8a6a5e4..a819aebe5b3 100644
--- a/src/test/modules/injection_points/injection_points.c
+++ b/src/test/modules/injection_points/injection_points.c
@@ -452,4 +452,4 @@ injection_points_detach(PG_FUNCTION_ARGS)
 	}
 
 	PG_RETURN_VOID();
-}
+}
\ No newline at end of file
diff --git a/src/test/modules/test_slru/Makefile b/src/test/modules/test_slru/Makefile
index 936886753b7..a0f49129079 100644
--- a/src/test/modules/test_slru/Makefile
+++ b/src/test/modules/test_slru/Makefile
@@ -6,6 +6,9 @@ OBJS = \
 	test_slru.o
 PGFILEDESC = "test_slru - test module for SLRUs"
 
+EXTRA_INSTALL=src/test/modules/injection_points
+TAP_TESTS = 1
+
 EXTENSION = test_slru
 DATA = test_slru--1.0.sql
 
diff --git a/src/test/modules/test_slru/meson.build b/src/test/modules/test_slru/meson.build
index ce91e606313..4a5bc6349a4 100644
--- a/src/test/modules/test_slru/meson.build
+++ b/src/test/modules/test_slru/meson.build
@@ -32,4 +32,12 @@ tests += {
     'regress_args': ['--temp-config', files('test_slru.conf')],
     'runningcheck': false,
   },
+  'tap': {
+    'env': {
+       'enable_injection_points': get_option('injection_points') ? 'yes' : 'no',
+    },
+    'tests': [
+      't/001_multixact.pl'
+    ],
+  },
 }
diff --git a/src/test/modules/test_slru/t/001_multixact.pl b/src/test/modules/test_slru/t/001_multixact.pl
new file mode 100644
index 00000000000..b46e0994351
--- /dev/null
+++ b/src/test/modules/test_slru/t/001_multixact.pl
@@ -0,0 +1,66 @@
+
+# Copyright (c) 2024, PostgreSQL Global Development Group
+
+use strict;
+use warnings FATAL => 'all';
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+
+use Test::More;
+
+my ($node, $result);
+
+$node = PostgreSQL::Test::Cluster->new('multixact_CV_sleep');
+$node->init;
+$node->append_conf('postgresql.conf',
+	"shared_preload_libraries = 'test_slru'");
+$node->start;
+$node->safe_psql('postgres', q(CREATE EXTENSION injection_points));
+$node->safe_psql('postgres', q(CREATE EXTENSION test_slru));
+
+# Test for Multixact generation edge case
+$node->safe_psql('postgres', q(select injection_points_attach('test_read_multixact','wait')));
+$node->safe_psql('postgres', q(select injection_points_attach('GetMultiXactIdMembers-CV-sleep','notice')));
+
+# This session must observe sleep on CV when generating multixact.
+# To achive this it first will create a multixact, then pause before reading it.
+my $observer = $node->background_psql('postgres');
+
+$observer->query_until(qr/start/,
+q(
+	\echo start
+	select test_read_multixact(test_create_multixact());
+));
+
+# This session will create next Multixact, it's necessary to avoid edge case 1 (see multixact.c)
+my $creator = $node->background_psql('postgres');
+$node->safe_psql('postgres', q(select injection_points_attach('GetNewMultiXactId-done','wait')));
+
+# We expect this query to hand in critical section after generating new multixact,
+# but before filling it's offset into SLRU
+$creator->query_until(qr/start/, q(
+	\echo start
+	select test_create_multixact();
+));
+
+# Now we are sure we can reach edge case 2. Proceed session that is reading that multixact.
+$node->safe_psql('postgres', q(select injection_points_detach('test_read_multixact')));
+
+# Release critical section. We have to do this so everyon can proceed.
+# But this is inherent race condition, I hope the tast will not be unstable here.
+# The only way to stabilize it will be adding some sleep here.
+$node->safe_psql('postgres', q(select injection_points_detach('GetNewMultiXactId-done')));
+
+# Here goes the whole purpose of this test: see that sleep in fact occured.
+ok( pump_until(
+		$observer->{run}, $observer->{timeout},
+		\$observer->{stderr}, qr/notice triggered for injection point GetMultiXactIdMembers-CV-sleep/),
+	"sleep observed");
+
+$observer->quit;
+
+$creator->quit;
+
+$node->stop;
+done_testing();
diff --git a/src/test/modules/test_slru/test_slru--1.0.sql b/src/test/modules/test_slru/test_slru--1.0.sql
index 202e8da3fde..58300c59a77 100644
--- a/src/test/modules/test_slru/test_slru--1.0.sql
+++ b/src/test/modules/test_slru/test_slru--1.0.sql
@@ -19,3 +19,9 @@ CREATE OR REPLACE FUNCTION test_slru_page_truncate(bigint) RETURNS VOID
   AS 'MODULE_PATHNAME', 'test_slru_page_truncate' LANGUAGE C;
 CREATE OR REPLACE FUNCTION test_slru_delete_all() RETURNS VOID
   AS 'MODULE_PATHNAME', 'test_slru_delete_all' LANGUAGE C;
+
+
+CREATE OR REPLACE FUNCTION test_create_multixact() RETURNS xid
+  AS 'MODULE_PATHNAME', 'test_create_multixact' LANGUAGE C;
+CREATE OR REPLACE FUNCTION test_read_multixact(xid) RETURNS VOID
+  AS 'MODULE_PATHNAME', 'test_read_multixact'LANGUAGE C;
\ No newline at end of file
diff --git a/src/test/modules/test_slru/test_slru.c b/src/test/modules/test_slru/test_slru.c
index d227b067034..b3fabc3b6ed 100644
--- a/src/test/modules/test_slru/test_slru.c
+++ b/src/test/modules/test_slru/test_slru.c
@@ -14,13 +14,16 @@
 
 #include "postgres.h"
 
+#include "access/multixact.h"
 #include "access/slru.h"
+#include "access/xact.h"
 #include "access/transam.h"
 #include "miscadmin.h"
 #include "storage/fd.h"
 #include "storage/ipc.h"
 #include "storage/shmem.h"
 #include "utils/builtins.h"
+#include "utils/injection_point.h"
 
 PG_MODULE_MAGIC;
 
@@ -36,6 +39,8 @@ PG_FUNCTION_INFO_V1(test_slru_page_sync);
 PG_FUNCTION_INFO_V1(test_slru_page_delete);
 PG_FUNCTION_INFO_V1(test_slru_page_truncate);
 PG_FUNCTION_INFO_V1(test_slru_delete_all);
+PG_FUNCTION_INFO_V1(test_create_multixact);
+PG_FUNCTION_INFO_V1(test_read_multixact);
 
 /* Number of SLRU page slots */
 #define NUM_TEST_BUFFERS		16
@@ -260,3 +265,27 @@ _PG_init(void)
 	prev_shmem_startup_hook = shmem_startup_hook;
 	shmem_startup_hook = test_slru_shmem_startup;
 }
+
+Datum
+test_create_multixact(PG_FUNCTION_ARGS)
+{
+	MultiXactId id;
+	MultiXactIdSetOldestMember();
+	id = MultiXactIdCreate(GetCurrentTransactionId(), MultiXactStatusUpdate,
+						GetCurrentTransactionId(), MultiXactStatusForShare);
+	PG_RETURN_TRANSACTIONID(id);
+}
+
+Datum
+test_read_multixact(PG_FUNCTION_ARGS)
+{
+	MultiXactId id = PG_GETARG_TRANSACTIONID(0);
+	MultiXactMember *members;
+	INJECTION_POINT("test_read_multixact");
+	/* discard caches */
+	AtEOXact_MultiXact();
+
+	if (GetMultiXactIdMembers(id,&members,false, false) == -1)
+		elog(ERROR, "MultiXactId not found");
+	PG_RETURN_VOID();
+}
\ No newline at end of file
-- 
2.37.1 (Apple Git-137.1)

From fe58c08881c7fea3be94e6a166d621e8acc78a92 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Wed, 5 Jun 2024 07:35:29 +0900
Subject: [PATCH v2 1/2] Support loading of injection points

This can be used to load an injection point in the backend-level cache
before running it, to avoid issues if the point cannot be loaded due to
restrictions in the code path where it would be run, like a critical
section.
---
 src/include/utils/injection_point.h           |   3 +
 src/backend/utils/misc/injection_point.c      | 116 ++++++++++++------
 .../expected/injection_points.out             |  32 +++++
 .../injection_points--1.0.sql                 |  10 ++
 .../injection_points/injection_points.c       |  17 +++
 .../injection_points/sql/injection_points.sql |   7 ++
 doc/src/sgml/xfunc.sgml                       |  14 +++
 7 files changed, 163 insertions(+), 36 deletions(-)

diff --git a/src/include/utils/injection_point.h b/src/include/utils/injection_point.h
index a61d5d4439..bd3a62425c 100644
--- a/src/include/utils/injection_point.h
+++ b/src/include/utils/injection_point.h
@@ -15,8 +15,10 @@
  * Injections points require --enable-injection-points.
  */
 #ifdef USE_INJECTION_POINTS
+#define INJECTION_POINT_LOAD(name) InjectionPointLoad(name)
 #define INJECTION_POINT(name) InjectionPointRun(name)
 #else
+#define INJECTION_POINT_LOAD(name) ((void) name)
 #define INJECTION_POINT(name) ((void) name)
 #endif
 
@@ -34,6 +36,7 @@ extern void InjectionPointAttach(const char *name,
 								 const char *function,
 								 const void *private_data,
 								 int private_data_size);
+extern void InjectionPointLoad(const char *name);
 extern void InjectionPointRun(const char *name);
 extern bool InjectionPointDetach(const char *name);
 
diff --git a/src/backend/utils/misc/injection_point.c b/src/backend/utils/misc/injection_point.c
index 5c2a0d2297..f02ed6c08b 100644
--- a/src/backend/utils/misc/injection_point.c
+++ b/src/backend/utils/misc/injection_point.c
@@ -129,20 +129,47 @@ injection_point_cache_remove(const char *name)
 	(void) hash_search(InjectionPointCache, name, HASH_REMOVE, NULL);
 }
 
+/*
+ * injection_point_cache_load
+ *
+ * Load an injection point into the local cache.
+ */
+static void
+injection_point_cache_load(InjectionPointEntry *entry_by_name)
+{
+	char		path[MAXPGPATH];
+	void	   *injection_callback_local;
+
+	snprintf(path, MAXPGPATH, "%s/%s%s", pkglib_path,
+			 entry_by_name->library, DLSUFFIX);
+
+	if (!pg_file_exists(path))
+		elog(ERROR, "could not find library \"%s\" for injection point \"%s\"",
+			 path, entry_by_name->name);
+
+	injection_callback_local = (void *)
+		load_external_function(path, entry_by_name->function, false, NULL);
+
+	if (injection_callback_local == NULL)
+		elog(ERROR, "could not find function \"%s\" in library \"%s\" for injection point \"%s\"",
+			 entry_by_name->function, path, entry_by_name->name);
+
+	/* add it to the local cache when found */
+	injection_point_cache_add(entry_by_name->name, injection_callback_local,
+							  entry_by_name->private_data);
+}
+
 /*
  * injection_point_cache_get
  *
  * Retrieve an injection point from the local cache, if any.
  */
-static InjectionPointCallback
-injection_point_cache_get(const char *name, const void **private_data)
+static InjectionPointCacheEntry *
+injection_point_cache_get(const char *name)
 {
 	bool		found;
 	InjectionPointCacheEntry *entry;
 
-	if (private_data)
-		*private_data = NULL;
-
 	/* no callback if no cache yet */
 	if (InjectionPointCache == NULL)
 		return NULL;
@@ -151,11 +178,7 @@ injection_point_cache_get(const char *name, const void **private_data)
 		hash_search(InjectionPointCache, name, HASH_FIND, &found);
 
 	if (found)
-	{
-		if (private_data)
-			*private_data = entry->private_data;
-		return entry->callback;
-	}
+		return entry;
 
 	return NULL;
 }
@@ -281,6 +304,47 @@ InjectionPointDetach(const char *name)
 #endif
 }
 
+/*
+ * Load an injection point into the local cache.
+ *
+ * This is useful to be able to load a function pointer at a stage earlier
+ * than it would be run, especially if the injection point is called in a code
+ * path where memory allocations cannot happen, like critical sections.
+ */
+void
+InjectionPointLoad(const char *name)
+{
+#ifdef USE_INJECTION_POINTS
+	InjectionPointEntry *entry_by_name;
+	bool		found;
+
+	LWLockAcquire(InjectionPointLock, LW_SHARED);
+	entry_by_name = (InjectionPointEntry *)
+		hash_search(InjectionPointHash, name,
+					HASH_FIND, &found);
+	LWLockRelease(InjectionPointLock);
+
+	/*
+	 * If not found, do nothing and remove it from the local cache if it
+	 * existed there.
+	 */
+	if (!found)
+	{
+		injection_point_cache_remove(name);
+		return;
+	}
+
+	/* Check first the local cache, and leave if this entry exists. */
+	if (injection_point_cache_get(name) != NULL)
+		return;
+
+	/* Nothing?  Then load it and leave */
+	injection_point_cache_load(entry_by_name);
+#else
+	elog(ERROR, "Injection points are not supported by this build");
+#endif
+}
+
 /*
  * Execute an injection point, if defined.
  *
@@ -293,8 +357,7 @@ InjectionPointRun(const char *name)
 #ifdef USE_INJECTION_POINTS
 	InjectionPointEntry *entry_by_name;
 	bool		found;
-	InjectionPointCallback injection_callback;
-	const void *private_data;
+	InjectionPointCacheEntry *cache_entry;
 
 	LWLockAcquire(InjectionPointLock, LW_SHARED);
 	entry_by_name = (InjectionPointEntry *)
@@ -316,34 +379,15 @@ InjectionPointRun(const char *name)
 	 * Check if the callback exists in the local cache, to avoid unnecessary
 	 * external loads.
 	 */
-	if (injection_point_cache_get(name, NULL) == NULL)
+	if (injection_point_cache_get(name) == NULL)
 	{
-		char		path[MAXPGPATH];
-		InjectionPointCallback injection_callback_local;
-
-		/* not found in local cache, so load and register */
-		snprintf(path, MAXPGPATH, "%s/%s%s", pkglib_path,
-				 entry_by_name->library, DLSUFFIX);
-
-		if (!pg_file_exists(path))
-			elog(ERROR, "could not find library \"%s\" for injection point \"%s\"",
-				 path, name);
-
-		injection_callback_local = (InjectionPointCallback)
-			load_external_function(path, entry_by_name->function, false, NULL);
-
-		if (injection_callback_local == NULL)
-			elog(ERROR, "could not find function \"%s\" in library \"%s\" for injection point \"%s\"",
-				 entry_by_name->function, path, name);
-
-		/* add it to the local cache when found */
-		injection_point_cache_add(name, injection_callback_local,
-								  entry_by_name->private_data);
+		/* not found in local cache, so load and register it */
+		injection_point_cache_load(entry_by_name);
 	}
 
 	/* Now loaded, so get it. */
-	injection_callback = injection_point_cache_get(name, &private_data);
-	injection_callback(name, private_data);
+	cache_entry = injection_point_cache_get(name);
+	cache_entry->callback(name, cache_entry->private_data);
 #else
 	elog(ERROR, "Injection points are not supported by this build");
 #endif
diff --git a/src/test/modules/injection_points/expected/injection_points.out b/src/test/modules/injection_points/expected/injection_points.out
index dd9db06e10..2f60da900b 100644
--- a/src/test/modules/injection_points/expected/injection_points.out
+++ b/src/test/modules/injection_points/expected/injection_points.out
@@ -128,6 +128,38 @@ SELECT injection_points_detach('TestInjectionLog2');
  
 (1 row)
 
+-- Loading
+SELECT injection_points_load('TestInjectionLogLoad'); -- nothing
+ injection_points_load 
+-----------------------
+ 
+(1 row)
+
+SELECT injection_points_attach('TestInjectionLogLoad', 'notice');
+ injection_points_attach 
+-------------------------
+ 
+(1 row)
+
+SELECT injection_points_load('TestInjectionLogLoad'); -- nothing happens
+ injection_points_load 
+-----------------------
+ 
+(1 row)
+
+SELECT injection_points_run('TestInjectionLogLoad'); -- runs from cache
+NOTICE:  notice triggered for injection point TestInjectionLogLoad
+ injection_points_run 
+----------------------
+ 
+(1 row)
+
+SELECT injection_points_detach('TestInjectionLogLoad');
+ injection_points_detach 
+-------------------------
+ 
+(1 row)
+
 -- Runtime conditions
 SELECT injection_points_attach('TestConditionError', 'error');
  injection_points_attach 
diff --git a/src/test/modules/injection_points/injection_points--1.0.sql b/src/test/modules/injection_points/injection_points--1.0.sql
index c16a33b08d..e275c2cf5b 100644
--- a/src/test/modules/injection_points/injection_points--1.0.sql
+++ b/src/test/modules/injection_points/injection_points--1.0.sql
@@ -14,6 +14,16 @@ RETURNS void
 AS 'MODULE_PATHNAME', 'injection_points_attach'
 LANGUAGE C STRICT PARALLEL UNSAFE;
 
+--
+-- injection_points_load()
+--
+-- Load an injection point already attached.
+--
+CREATE FUNCTION injection_points_load(IN point_name TEXT)
+RETURNS void
+AS 'MODULE_PATHNAME', 'injection_points_load'
+LANGUAGE C STRICT PARALLEL UNSAFE;
+
 --
 -- injection_points_run()
 --
diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c
index 5c44625d1d..2e3da997b1 100644
--- a/src/test/modules/injection_points/injection_points.c
+++ b/src/test/modules/injection_points/injection_points.c
@@ -302,6 +302,23 @@ injection_points_attach(PG_FUNCTION_ARGS)
 	PG_RETURN_VOID();
 }
 
+/*
+ * SQL function for loading an injection point.
+ */
+PG_FUNCTION_INFO_V1(injection_points_load);
+Datum
+injection_points_load(PG_FUNCTION_ARGS)
+{
+	char	   *name = text_to_cstring(PG_GETARG_TEXT_PP(0));
+
+	if (inj_state == NULL)
+		injection_init_shmem();
+
+	INJECTION_POINT_LOAD(name);
+
+	PG_RETURN_VOID();
+}
+
 /*
  * SQL function for triggering an injection point.
  */
diff --git a/src/test/modules/injection_points/sql/injection_points.sql b/src/test/modules/injection_points/sql/injection_points.sql
index 71e2972a7e..fabf0a8823 100644
--- a/src/test/modules/injection_points/sql/injection_points.sql
+++ b/src/test/modules/injection_points/sql/injection_points.sql
@@ -41,6 +41,13 @@ SELECT injection_points_detach('TestInjectionLog'); -- fails
 SELECT injection_points_run('TestInjectionLog2'); -- notice
 SELECT injection_points_detach('TestInjectionLog2');
 
+-- Loading
+SELECT injection_points_load('TestInjectionLogLoad'); -- nothing
+SELECT injection_points_attach('TestInjectionLogLoad', 'notice');
+SELECT injection_points_load('TestInjectionLogLoad'); -- nothing happens
+SELECT injection_points_run('TestInjectionLogLoad'); -- runs from cache
+SELECT injection_points_detach('TestInjectionLogLoad');
+
 -- Runtime conditions
 SELECT injection_points_attach('TestConditionError', 'error');
 -- Any follow-up injection point attached will be local to this process.
diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
index a7c170476a..f9f46e1835 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -3618,6 +3618,20 @@ INJECTION_POINT(name);
      their own code using the same macro.
     </para>
 
+    <para>
+     An injection point with a given <literal>name</literal> can be loaded
+     using macro:
+<programlisting>
+INJECTION_POINT_LOAD(name);
+</programlisting>
+
+     This will load the injection point callback into the process cache,
+     doing all memory allocations at this stage without running the callback.
+     This is useful when an injection point is defined in a critical section
+     where no memory can be allocated, loading the injection point outside the
+     critical section with a two-step logic.
+    </para>
+
     <para>
      Add-ins can attach callbacks to an already-declared injection point by
      calling:
-- 
2.43.0

From 53cb84a661717cae179f09e8a16755202259c5b9 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Wed, 5 Jun 2024 07:41:20 +0900
Subject: [PATCH v2 2/2] Extend injection points with optional runtime
 arguments

This extends injections points with a 1-argument flavor, so as it
becomes possible for callbacks to pass down values coming from the code
paths where an injection point is defined.  The primary use case that
can be covered in this function is runtime manipulation of a given
stack, where it would be possible to corrupt a running state, based on a
runtime set of conditions.

For example, imagine a class of failures in the solar flare category
causing bits to flip on a page.
---
 src/include/utils/injection_point.h           |  9 ++-
 src/backend/utils/misc/injection_point.c      | 69 ++++++++++++++++---
 .../expected/injection_points.out             | 31 +++++++++
 .../injection_points--1.0.sql                 | 10 +++
 .../injection_points/injection_points.c       | 39 ++++++++++-
 .../injection_points/sql/injection_points.sql |  9 +++
 doc/src/sgml/xfunc.sgml                       |  9 ++-
 7 files changed, 162 insertions(+), 14 deletions(-)

diff --git a/src/include/utils/injection_point.h b/src/include/utils/injection_point.h
index bd3a62425c..08c4811e89 100644
--- a/src/include/utils/injection_point.h
+++ b/src/include/utils/injection_point.h
@@ -17,9 +17,11 @@
 #ifdef USE_INJECTION_POINTS
 #define INJECTION_POINT_LOAD(name) InjectionPointLoad(name)
 #define INJECTION_POINT(name) InjectionPointRun(name)
+#define INJECTION_POINT_1ARG(name, arg1) InjectionPointRun1Arg(name, arg1)
 #else
 #define INJECTION_POINT_LOAD(name) ((void) name)
 #define INJECTION_POINT(name) ((void) name)
+#define INJECTION_POINT_1ARG(name) ((void) name, (void) arg1)
 #endif
 
 /*
@@ -27,6 +29,9 @@
  */
 typedef void (*InjectionPointCallback) (const char *name,
 										const void *private_data);
+typedef void (*InjectionPointCallback1Arg) (const char *name,
+											const void *private_data,
+											const void *arg1);
 
 extern Size InjectionPointShmemSize(void);
 extern void InjectionPointShmemInit(void);
@@ -35,9 +40,11 @@ extern void InjectionPointAttach(const char *name,
 								 const char *library,
 								 const char *function,
 								 const void *private_data,
-								 int private_data_size);
+								 int private_data_size,
+								 int num_args);
 extern void InjectionPointLoad(const char *name);
 extern void InjectionPointRun(const char *name);
+extern void InjectionPointRun1Arg(const char *name, void *arg1);
 extern bool InjectionPointDetach(const char *name);
 
 #endif							/* INJECTION_POINT_H */
diff --git a/src/backend/utils/misc/injection_point.c b/src/backend/utils/misc/injection_point.c
index f02ed6c08b..af76642a5f 100644
--- a/src/backend/utils/misc/injection_point.c
+++ b/src/backend/utils/misc/injection_point.c
@@ -56,6 +56,9 @@ typedef struct InjectionPointEntry
 	 * callbacks, registered when attached.
 	 */
 	char		private_data[INJ_PRIVATE_MAXLEN];
+
+	/* Number of arguments used by the callback */
+	int			num_args;
 } InjectionPointEntry;
 
 #define INJECTION_POINT_HASH_INIT_SIZE	16
@@ -69,7 +72,12 @@ typedef struct InjectionPointCacheEntry
 {
 	char		name[INJ_NAME_MAXLEN];
 	char		private_data[INJ_PRIVATE_MAXLEN];
-	InjectionPointCallback callback;
+	int			num_args;
+	union
+	{
+		InjectionPointCallback callback;
+		InjectionPointCallback1Arg callback_1arg;
+	}			data;
 } InjectionPointCacheEntry;
 
 static HTAB *InjectionPointCache = NULL;
@@ -81,8 +89,9 @@ static HTAB *InjectionPointCache = NULL;
  */
 static void
 injection_point_cache_add(const char *name,
-						  InjectionPointCallback callback,
-						  const void *private_data)
+						  void *callback,
+						  const void *private_data,
+						  int num_args)
 {
 	InjectionPointCacheEntry *entry;
 	bool		found;
@@ -107,7 +116,14 @@ injection_point_cache_add(const char *name,
 
 	Assert(!found);
 	strlcpy(entry->name, name, sizeof(entry->name));
-	entry->callback = callback;
+	entry->num_args = num_args;
+	if (num_args == 0)
+		entry->data.callback = (InjectionPointCallback) callback;
+	else if (num_args == 1)
+		entry->data.callback_1arg = (InjectionPointCallback1Arg) callback;
+	else
+		elog(ERROR, "unsupported number of arguments for injection point \"%s\"",
+			 name);
 	if (private_data != NULL)
 		memcpy(entry->private_data, private_data, INJ_PRIVATE_MAXLEN);
 }
@@ -156,7 +172,8 @@ injection_point_cache_load(InjectionPointEntry *entry_by_name)
 
 	/* add it to the local cache when found */
 	injection_point_cache_add(entry_by_name->name, injection_callback_local,
-							  entry_by_name->private_data);
+							  entry_by_name->private_data,
+							  entry_by_name->num_args);
 }
 
 /*
@@ -229,7 +246,8 @@ InjectionPointAttach(const char *name,
 					 const char *library,
 					 const char *function,
 					 const void *private_data,
-					 int private_data_size)
+					 int private_data_size,
+					 int num_args)
 {
 #ifdef USE_INJECTION_POINTS
 	InjectionPointEntry *entry_by_name;
@@ -271,6 +289,7 @@ InjectionPointAttach(const char *name,
 	entry_by_name->function[INJ_FUNC_MAXLEN - 1] = '\0';
 	if (private_data != NULL)
 		memcpy(entry_by_name->private_data, private_data, private_data_size);
+	entry_by_name->num_args = num_args;
 
 	LWLockRelease(InjectionPointLock);
 
@@ -346,13 +365,13 @@ InjectionPointLoad(const char *name)
 }
 
 /*
- * Execute an injection point, if defined.
+ * Workhorse for execution of an injection point, if defined.
  *
  * Check first the shared hash table, and adapt the local cache depending
  * on that as it could be possible that an entry to run has been removed.
  */
-void
-InjectionPointRun(const char *name)
+static inline void
+InjectionPointRunInternal(const char *name, int num_args, void *arg1)
 {
 #ifdef USE_INJECTION_POINTS
 	InjectionPointEntry *entry_by_name;
@@ -387,8 +406,38 @@ InjectionPointRun(const char *name)
 
 	/* Now loaded, so get it. */
 	cache_entry = injection_point_cache_get(name);
-	cache_entry->callback(name, cache_entry->private_data);
+
+	/* Check the number of arguments with the cache. */
+	if (cache_entry->num_args != num_args)
+		elog(ERROR, "incorrect number of arguments for injection point \"%s\": defined %d but expected %d",
+			 name, cache_entry->num_args, num_args);
+
+	if (cache_entry->num_args == 0)
+		cache_entry->data.callback(name, cache_entry->private_data);
+	else if (cache_entry->num_args == 1)
+		cache_entry->data.callback_1arg(name, cache_entry->private_data, arg1);
+	else
+	{
+		/* cannot be reached */
+		Assert(false);
+	}
 #else
 	elog(ERROR, "Injection points are not supported by this build");
 #endif
 }
+
+/*
+ * Execute an injection point, with no arguments.
+ */
+void
+InjectionPointRun(const char *name)
+{
+	InjectionPointRunInternal(name, 0, NULL);
+}
+
+/* 1-argument version */
+void
+InjectionPointRun1Arg(const char *name, void *arg1)
+{
+	InjectionPointRunInternal(name, 1, arg1);
+}
diff --git a/src/test/modules/injection_points/expected/injection_points.out b/src/test/modules/injection_points/expected/injection_points.out
index 2f60da900b..86043ce1c6 100644
--- a/src/test/modules/injection_points/expected/injection_points.out
+++ b/src/test/modules/injection_points/expected/injection_points.out
@@ -122,12 +122,43 @@ NOTICE:  notice triggered for injection point TestInjectionLog2
  
 (1 row)
 
+-- 1-argument runs
+SELECT injection_points_run_1arg('TestInjectionLog2', 'blah'); -- error
+ERROR:  incorrect number of arguments for injection point "TestInjectionLog2": defined 0 but expected 1
+SELECT injection_points_attach('TestInjectionLogArg1', 'notice_1arg');
+ injection_points_attach 
+-------------------------
+ 
+(1 row)
+
+SELECT injection_points_run('TestInjectionLogArg1'); -- error
+ERROR:  incorrect number of arguments for injection point "TestInjectionLogArg1": defined 1 but expected 0
+SELECT injection_points_run_1arg('TestInjectionLogArg1', 'blah'); -- notice
+NOTICE:  notice triggered for injection point TestInjectionLogArg1: blah
+ injection_points_run_1arg 
+---------------------------
+ 
+(1 row)
+
+SELECT injection_points_run_1arg('TestInjectionLogArg1', 'foobar'); -- notice
+NOTICE:  notice triggered for injection point TestInjectionLogArg1: foobar
+ injection_points_run_1arg 
+---------------------------
+ 
+(1 row)
+
 SELECT injection_points_detach('TestInjectionLog2');
  injection_points_detach 
 -------------------------
  
 (1 row)
 
+SELECT injection_points_detach('TestInjectionLogArg1');
+ injection_points_detach 
+-------------------------
+ 
+(1 row)
+
 -- Loading
 SELECT injection_points_load('TestInjectionLogLoad'); -- nothing
  injection_points_load 
diff --git a/src/test/modules/injection_points/injection_points--1.0.sql b/src/test/modules/injection_points/injection_points--1.0.sql
index e275c2cf5b..5bd7dad8e2 100644
--- a/src/test/modules/injection_points/injection_points--1.0.sql
+++ b/src/test/modules/injection_points/injection_points--1.0.sql
@@ -55,6 +55,16 @@ RETURNS void
 AS 'MODULE_PATHNAME', 'injection_points_set_local'
 LANGUAGE C STRICT PARALLEL UNSAFE;
 
+--
+-- injection_points_run_1arg()
+--
+-- Executes the action attached to the injection point.
+--
+CREATE FUNCTION injection_points_run_1arg(IN point_name TEXT, IN arg1 TEXT)
+RETURNS void
+AS 'MODULE_PATHNAME', 'injection_points_run_1arg'
+LANGUAGE C STRICT PARALLEL UNSAFE;
+
 --
 -- injection_points_detach()
 --
diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c
index 2e3da997b1..3640ee6f93 100644
--- a/src/test/modules/injection_points/injection_points.c
+++ b/src/test/modules/injection_points/injection_points.c
@@ -90,6 +90,9 @@ extern PGDLLEXPORT void injection_error(const char *name,
 										const void *private_data);
 extern PGDLLEXPORT void injection_notice(const char *name,
 										 const void *private_data);
+extern PGDLLEXPORT void injection_notice_1arg(const char *name,
+											  const void *private_data,
+											  const void *arg1);
 extern PGDLLEXPORT void injection_wait(const char *name,
 									   const void *private_data);
 
@@ -260,6 +263,19 @@ injection_wait(const char *name, const void *private_data)
 	SpinLockRelease(&inj_state->lock);
 }
 
+void
+injection_notice_1arg(const char *name, const void *private_data,
+					  const void *arg1)
+{
+	InjectionPointCondition *condition = (InjectionPointCondition *) private_data;
+	const char *str = (const char *) arg1;
+
+	if (!injection_point_allowed(condition))
+		return;
+
+	elog(NOTICE, "notice triggered for injection point %s: %s", name, str);
+}
+
 /*
  * SQL function for creating an injection point.
  */
@@ -271,6 +287,7 @@ injection_points_attach(PG_FUNCTION_ARGS)
 	char	   *action = text_to_cstring(PG_GETARG_TEXT_PP(1));
 	char	   *function;
 	InjectionPointCondition condition = {0};
+	int			num_args = 0;
 
 	if (strcmp(action, "error") == 0)
 		function = "injection_error";
@@ -278,6 +295,11 @@ injection_points_attach(PG_FUNCTION_ARGS)
 		function = "injection_notice";
 	else if (strcmp(action, "wait") == 0)
 		function = "injection_wait";
+	else if (strcmp(action, "notice_1arg") == 0)
+	{
+		function = "injection_notice_1arg";
+		num_args = 1;
+	}
 	else
 		elog(ERROR, "incorrect action \"%s\" for injection point creation", action);
 
@@ -288,7 +310,7 @@ injection_points_attach(PG_FUNCTION_ARGS)
 	}
 
 	InjectionPointAttach(name, "injection_points", function, &condition,
-						 sizeof(InjectionPointCondition));
+						 sizeof(InjectionPointCondition), num_args);
 
 	if (injection_point_local)
 	{
@@ -395,6 +417,21 @@ injection_points_set_local(PG_FUNCTION_ARGS)
 	PG_RETURN_VOID();
 }
 
+/*
+ * SQL function for triggering an injection point, 1-argument flavor.
+ */
+PG_FUNCTION_INFO_V1(injection_points_run_1arg);
+Datum
+injection_points_run_1arg(PG_FUNCTION_ARGS)
+{
+	char	   *name = text_to_cstring(PG_GETARG_TEXT_PP(0));
+	char	   *arg1 = text_to_cstring(PG_GETARG_TEXT_PP(1));
+
+	INJECTION_POINT_1ARG(name, (void *) arg1);
+
+	PG_RETURN_VOID();
+}
+
 /*
  * SQL function for dropping an injection point.
  */
diff --git a/src/test/modules/injection_points/sql/injection_points.sql b/src/test/modules/injection_points/sql/injection_points.sql
index fabf0a8823..4e78a29154 100644
--- a/src/test/modules/injection_points/sql/injection_points.sql
+++ b/src/test/modules/injection_points/sql/injection_points.sql
@@ -39,7 +39,16 @@ SELECT injection_points_run('TestInjectionLog2'); -- notice
 SELECT injection_points_detach('TestInjectionLog'); -- fails
 
 SELECT injection_points_run('TestInjectionLog2'); -- notice
+
+-- 1-argument runs
+SELECT injection_points_run_1arg('TestInjectionLog2', 'blah'); -- error
+SELECT injection_points_attach('TestInjectionLogArg1', 'notice_1arg');
+SELECT injection_points_run('TestInjectionLogArg1'); -- error
+SELECT injection_points_run_1arg('TestInjectionLogArg1', 'blah'); -- notice
+SELECT injection_points_run_1arg('TestInjectionLogArg1', 'foobar'); -- notice
+
 SELECT injection_points_detach('TestInjectionLog2');
+SELECT injection_points_detach('TestInjectionLogArg1');
 
 -- Loading
 SELECT injection_points_load('TestInjectionLogLoad'); -- nothing
diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
index f9f46e1835..934d355d32 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -3609,13 +3609,15 @@ uint32 WaitEventExtensionNew(const char *wait_event_name)
      macro:
 <programlisting>
 INJECTION_POINT(name);
+INJECTION_POINT_1ARG(name, arg1);
 </programlisting>
 
      There are a few injection points already declared at strategic points
      within the server code. After adding a new injection point the code needs
      to be compiled in order for that injection point to be available in the
      binary. Add-ins written in C-language can declare injection points in
-     their own code using the same macro.
+     their own code using the same macro. It is possible to pass down arguments
+     to callbacks for runtime checks.
     </para>
 
     <para>
@@ -3640,7 +3642,8 @@ extern void InjectionPointAttach(const char *name,
                                  const char *library,
                                  const char *function,
                                  const void *private_data,
-                                 int private_data_size);
+                                 int private_data_size,
+                                 int num_args);
 </programlisting>
 
      <literal>name</literal> is the name of the injection point, which when
@@ -3648,6 +3651,8 @@ extern void InjectionPointAttach(const char *name,
      loaded from <literal>library</literal>. <literal>private_data</literal>
      is a private area of data of size <literal>private_data_size</literal>
      given as argument to the callback when executed.
+     <literal>num_args</literal> is the number of arguments used by the
+     callback.
     </para>
 
     <para>
-- 
2.43.0

From b12c5c2b713ec7f79b86ac5f9bb8c0b6d8581228 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Wed, 5 Jun 2024 07:35:29 +0900
Subject: [PATCH vAB1 1/2] Support loading of injection points

This can be used to load an injection point in the backend-level cache
before running it, to avoid issues if the point cannot be loaded due to
restrictions in the code path where it would be run, like a critical
section.
---
 doc/src/sgml/xfunc.sgml                       |  14 +++
 src/backend/utils/misc/injection_point.c      | 116 ++++++++++++------
 src/include/utils/injection_point.h           |   3 +
 .../expected/injection_points.out             |  32 +++++
 .../injection_points--1.0.sql                 |  10 ++
 .../injection_points/injection_points.c       |  17 +++
 .../injection_points/sql/injection_points.sql |   7 ++
 7 files changed, 163 insertions(+), 36 deletions(-)

diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
index a7c170476a..f9f46e1835 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -3618,6 +3618,20 @@ INJECTION_POINT(name);
      their own code using the same macro.
     </para>
 
+    <para>
+     An injection point with a given <literal>name</literal> can be loaded
+     using macro:
+<programlisting>
+INJECTION_POINT_LOAD(name);
+</programlisting>
+
+     This will load the injection point callback into the process cache,
+     doing all memory allocations at this stage without running the callback.
+     This is useful when an injection point is defined in a critical section
+     where no memory can be allocated, loading the injection point outside the
+     critical section with a two-step logic.
+    </para>
+
     <para>
      Add-ins can attach callbacks to an already-declared injection point by
      calling:
diff --git a/src/backend/utils/misc/injection_point.c b/src/backend/utils/misc/injection_point.c
index 5c2a0d2297..f02ed6c08b 100644
--- a/src/backend/utils/misc/injection_point.c
+++ b/src/backend/utils/misc/injection_point.c
@@ -129,20 +129,47 @@ injection_point_cache_remove(const char *name)
 	(void) hash_search(InjectionPointCache, name, HASH_REMOVE, NULL);
 }
 
+/*
+ * injection_point_cache_load
+ *
+ * Load an injection point into the local cache.
+ */
+static void
+injection_point_cache_load(InjectionPointEntry *entry_by_name)
+{
+	char		path[MAXPGPATH];
+	void	   *injection_callback_local;
+
+	snprintf(path, MAXPGPATH, "%s/%s%s", pkglib_path,
+			 entry_by_name->library, DLSUFFIX);
+
+	if (!pg_file_exists(path))
+		elog(ERROR, "could not find library \"%s\" for injection point \"%s\"",
+			 path, entry_by_name->name);
+
+	injection_callback_local = (void *)
+		load_external_function(path, entry_by_name->function, false, NULL);
+
+	if (injection_callback_local == NULL)
+		elog(ERROR, "could not find function \"%s\" in library \"%s\" for injection point \"%s\"",
+			 entry_by_name->function, path, entry_by_name->name);
+
+	/* add it to the local cache when found */
+	injection_point_cache_add(entry_by_name->name, injection_callback_local,
+							  entry_by_name->private_data);
+}
+
 /*
  * injection_point_cache_get
  *
  * Retrieve an injection point from the local cache, if any.
  */
-static InjectionPointCallback
-injection_point_cache_get(const char *name, const void **private_data)
+static InjectionPointCacheEntry *
+injection_point_cache_get(const char *name)
 {
 	bool		found;
 	InjectionPointCacheEntry *entry;
 
-	if (private_data)
-		*private_data = NULL;
-
 	/* no callback if no cache yet */
 	if (InjectionPointCache == NULL)
 		return NULL;
@@ -151,11 +178,7 @@ injection_point_cache_get(const char *name, const void **private_data)
 		hash_search(InjectionPointCache, name, HASH_FIND, &found);
 
 	if (found)
-	{
-		if (private_data)
-			*private_data = entry->private_data;
-		return entry->callback;
-	}
+		return entry;
 
 	return NULL;
 }
@@ -281,6 +304,47 @@ InjectionPointDetach(const char *name)
 #endif
 }
 
+/*
+ * Load an injection point into the local cache.
+ *
+ * This is useful to be able to load a function pointer at a stage earlier
+ * than it would be run, especially if the injection point is called in a code
+ * path where memory allocations cannot happen, like critical sections.
+ */
+void
+InjectionPointLoad(const char *name)
+{
+#ifdef USE_INJECTION_POINTS
+	InjectionPointEntry *entry_by_name;
+	bool		found;
+
+	LWLockAcquire(InjectionPointLock, LW_SHARED);
+	entry_by_name = (InjectionPointEntry *)
+		hash_search(InjectionPointHash, name,
+					HASH_FIND, &found);
+	LWLockRelease(InjectionPointLock);
+
+	/*
+	 * If not found, do nothing and remove it from the local cache if it
+	 * existed there.
+	 */
+	if (!found)
+	{
+		injection_point_cache_remove(name);
+		return;
+	}
+
+	/* Check first the local cache, and leave if this entry exists. */
+	if (injection_point_cache_get(name) != NULL)
+		return;
+
+	/* Nothing?  Then load it and leave */
+	injection_point_cache_load(entry_by_name);
+#else
+	elog(ERROR, "Injection points are not supported by this build");
+#endif
+}
+
 /*
  * Execute an injection point, if defined.
  *
@@ -293,8 +357,7 @@ InjectionPointRun(const char *name)
 #ifdef USE_INJECTION_POINTS
 	InjectionPointEntry *entry_by_name;
 	bool		found;
-	InjectionPointCallback injection_callback;
-	const void *private_data;
+	InjectionPointCacheEntry *cache_entry;
 
 	LWLockAcquire(InjectionPointLock, LW_SHARED);
 	entry_by_name = (InjectionPointEntry *)
@@ -316,34 +379,15 @@ InjectionPointRun(const char *name)
 	 * Check if the callback exists in the local cache, to avoid unnecessary
 	 * external loads.
 	 */
-	if (injection_point_cache_get(name, NULL) == NULL)
+	if (injection_point_cache_get(name) == NULL)
 	{
-		char		path[MAXPGPATH];
-		InjectionPointCallback injection_callback_local;
-
-		/* not found in local cache, so load and register */
-		snprintf(path, MAXPGPATH, "%s/%s%s", pkglib_path,
-				 entry_by_name->library, DLSUFFIX);
-
-		if (!pg_file_exists(path))
-			elog(ERROR, "could not find library \"%s\" for injection point \"%s\"",
-				 path, name);
-
-		injection_callback_local = (InjectionPointCallback)
-			load_external_function(path, entry_by_name->function, false, NULL);
-
-		if (injection_callback_local == NULL)
-			elog(ERROR, "could not find function \"%s\" in library \"%s\" for injection point \"%s\"",
-				 entry_by_name->function, path, name);
-
-		/* add it to the local cache when found */
-		injection_point_cache_add(name, injection_callback_local,
-								  entry_by_name->private_data);
+		/* not found in local cache, so load and register it */
+		injection_point_cache_load(entry_by_name);
 	}
 
 	/* Now loaded, so get it. */
-	injection_callback = injection_point_cache_get(name, &private_data);
-	injection_callback(name, private_data);
+	cache_entry = injection_point_cache_get(name);
+	cache_entry->callback(name, cache_entry->private_data);
 #else
 	elog(ERROR, "Injection points are not supported by this build");
 #endif
diff --git a/src/include/utils/injection_point.h b/src/include/utils/injection_point.h
index a61d5d4439..bd3a62425c 100644
--- a/src/include/utils/injection_point.h
+++ b/src/include/utils/injection_point.h
@@ -15,8 +15,10 @@
  * Injections points require --enable-injection-points.
  */
 #ifdef USE_INJECTION_POINTS
+#define INJECTION_POINT_LOAD(name) InjectionPointLoad(name)
 #define INJECTION_POINT(name) InjectionPointRun(name)
 #else
+#define INJECTION_POINT_LOAD(name) ((void) name)
 #define INJECTION_POINT(name) ((void) name)
 #endif
 
@@ -34,6 +36,7 @@ extern void InjectionPointAttach(const char *name,
 								 const char *function,
 								 const void *private_data,
 								 int private_data_size);
+extern void InjectionPointLoad(const char *name);
 extern void InjectionPointRun(const char *name);
 extern bool InjectionPointDetach(const char *name);
 
diff --git a/src/test/modules/injection_points/expected/injection_points.out b/src/test/modules/injection_points/expected/injection_points.out
index dd9db06e10..2f60da900b 100644
--- a/src/test/modules/injection_points/expected/injection_points.out
+++ b/src/test/modules/injection_points/expected/injection_points.out
@@ -128,6 +128,38 @@ SELECT injection_points_detach('TestInjectionLog2');
  
 (1 row)
 
+-- Loading
+SELECT injection_points_load('TestInjectionLogLoad'); -- nothing
+ injection_points_load 
+-----------------------
+ 
+(1 row)
+
+SELECT injection_points_attach('TestInjectionLogLoad', 'notice');
+ injection_points_attach 
+-------------------------
+ 
+(1 row)
+
+SELECT injection_points_load('TestInjectionLogLoad'); -- nothing happens
+ injection_points_load 
+-----------------------
+ 
+(1 row)
+
+SELECT injection_points_run('TestInjectionLogLoad'); -- runs from cache
+NOTICE:  notice triggered for injection point TestInjectionLogLoad
+ injection_points_run 
+----------------------
+ 
+(1 row)
+
+SELECT injection_points_detach('TestInjectionLogLoad');
+ injection_points_detach 
+-------------------------
+ 
+(1 row)
+
 -- Runtime conditions
 SELECT injection_points_attach('TestConditionError', 'error');
  injection_points_attach 
diff --git a/src/test/modules/injection_points/injection_points--1.0.sql b/src/test/modules/injection_points/injection_points--1.0.sql
index c16a33b08d..e275c2cf5b 100644
--- a/src/test/modules/injection_points/injection_points--1.0.sql
+++ b/src/test/modules/injection_points/injection_points--1.0.sql
@@ -14,6 +14,16 @@ RETURNS void
 AS 'MODULE_PATHNAME', 'injection_points_attach'
 LANGUAGE C STRICT PARALLEL UNSAFE;
 
+--
+-- injection_points_load()
+--
+-- Load an injection point already attached.
+--
+CREATE FUNCTION injection_points_load(IN point_name TEXT)
+RETURNS void
+AS 'MODULE_PATHNAME', 'injection_points_load'
+LANGUAGE C STRICT PARALLEL UNSAFE;
+
 --
 -- injection_points_run()
 --
diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c
index 5c44625d1d..2e3da997b1 100644
--- a/src/test/modules/injection_points/injection_points.c
+++ b/src/test/modules/injection_points/injection_points.c
@@ -302,6 +302,23 @@ injection_points_attach(PG_FUNCTION_ARGS)
 	PG_RETURN_VOID();
 }
 
+/*
+ * SQL function for loading an injection point.
+ */
+PG_FUNCTION_INFO_V1(injection_points_load);
+Datum
+injection_points_load(PG_FUNCTION_ARGS)
+{
+	char	   *name = text_to_cstring(PG_GETARG_TEXT_PP(0));
+
+	if (inj_state == NULL)
+		injection_init_shmem();
+
+	INJECTION_POINT_LOAD(name);
+
+	PG_RETURN_VOID();
+}
+
 /*
  * SQL function for triggering an injection point.
  */
diff --git a/src/test/modules/injection_points/sql/injection_points.sql b/src/test/modules/injection_points/sql/injection_points.sql
index 71e2972a7e..fabf0a8823 100644
--- a/src/test/modules/injection_points/sql/injection_points.sql
+++ b/src/test/modules/injection_points/sql/injection_points.sql
@@ -41,6 +41,13 @@ SELECT injection_points_detach('TestInjectionLog'); -- fails
 SELECT injection_points_run('TestInjectionLog2'); -- notice
 SELECT injection_points_detach('TestInjectionLog2');
 
+-- Loading
+SELECT injection_points_load('TestInjectionLogLoad'); -- nothing
+SELECT injection_points_attach('TestInjectionLogLoad', 'notice');
+SELECT injection_points_load('TestInjectionLogLoad'); -- nothing happens
+SELECT injection_points_run('TestInjectionLogLoad'); -- runs from cache
+SELECT injection_points_detach('TestInjectionLogLoad');
+
 -- Runtime conditions
 SELECT injection_points_attach('TestConditionError', 'error');
 -- Any follow-up injection point attached will be local to this process.
-- 
2.39.3 (Apple Git-146)

From 121f2da707ad4a71f0b3a6f70002190bae066cc5 Mon Sep 17 00:00:00 2001
From: Andrey Borodin <amborodin@acm.org>
Date: Mon, 20 May 2024 15:20:31 +0500
Subject: [PATCH vAB1 2/2] Add multixact CV sleep test

Previosuly we used 1ms sleep when we are reading multixact before
another multixact withouth offset yet. a0e0fb1ba changed this behaviour
to CV sleep. This commit addes test case for this.
To test such race condition we have to use waiting injection point under
critical section. This requires special pre-loaded injection point.
---
 src/backend/access/transam/multixact.c        |  7 ++
 src/test/modules/test_slru/Makefile           |  4 +
 src/test/modules/test_slru/meson.build        |  8 ++
 src/test/modules/test_slru/t/001_multixact.pl | 91 +++++++++++++++++++
 src/test/modules/test_slru/test_slru--1.0.sql |  6 ++
 src/test/modules/test_slru/test_slru.c        | 38 ++++++++
 6 files changed, 154 insertions(+)
 create mode 100644 src/test/modules/test_slru/t/001_multixact.pl

diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 54c916e034..8c68bdbe0b 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -88,6 +88,7 @@
 #include "storage/proc.h"
 #include "storage/procarray.h"
 #include "utils/fmgrprotos.h"
+#include "utils/injection_point.h"
 #include "utils/guc_hooks.h"
 #include "utils/memutils.h"
 
@@ -825,8 +826,12 @@ MultiXactIdCreateFromMembers(int nmembers, MultiXactMember *members)
 	 * in vacuum.  During vacuum, in particular, it would be unacceptable to
 	 * keep OldestMulti set, in case it runs for long.
 	 */
+	INJECTION_POINT_LOAD("GetNewMultiXactId-done");
+
 	multi = GetNewMultiXactId(nmembers, &offset);
 
+	INJECTION_POINT("GetNewMultiXactId-done");
+
 	/* Make an XLOG entry describing the new MXID. */
 	xlrec.mid = multi;
 	xlrec.moff = offset;
@@ -1439,6 +1444,8 @@ retry:
 			LWLockRelease(lock);
 			CHECK_FOR_INTERRUPTS();
 
+			INJECTION_POINT("GetMultiXactIdMembers-CV-sleep");
+
 			ConditionVariableSleep(&MultiXactState->nextoff_cv,
 								   WAIT_EVENT_MULTIXACT_CREATION);
 			slept = true;
diff --git a/src/test/modules/test_slru/Makefile b/src/test/modules/test_slru/Makefile
index 936886753b..8f9623b128 100644
--- a/src/test/modules/test_slru/Makefile
+++ b/src/test/modules/test_slru/Makefile
@@ -6,6 +6,10 @@ OBJS = \
 	test_slru.o
 PGFILEDESC = "test_slru - test module for SLRUs"
 
+EXTRA_INSTALL=src/test/modules/injection_points
+export enable_injection_points enable_injection_points
+TAP_TESTS = 1
+
 EXTENSION = test_slru
 DATA = test_slru--1.0.sql
 
diff --git a/src/test/modules/test_slru/meson.build b/src/test/modules/test_slru/meson.build
index ce91e60631..4a5bc6349a 100644
--- a/src/test/modules/test_slru/meson.build
+++ b/src/test/modules/test_slru/meson.build
@@ -32,4 +32,12 @@ tests += {
     'regress_args': ['--temp-config', files('test_slru.conf')],
     'runningcheck': false,
   },
+  'tap': {
+    'env': {
+       'enable_injection_points': get_option('injection_points') ? 'yes' : 'no',
+    },
+    'tests': [
+      't/001_multixact.pl'
+    ],
+  },
 }
diff --git a/src/test/modules/test_slru/t/001_multixact.pl b/src/test/modules/test_slru/t/001_multixact.pl
new file mode 100644
index 0000000000..5c24bf2736
--- /dev/null
+++ b/src/test/modules/test_slru/t/001_multixact.pl
@@ -0,0 +1,91 @@
+# Copyright (c) 2024, PostgreSQL Global Development Group
+
+# This test verifies edge case of reading a multixact:
+# when we have multixact that is followed by exactly one another multixact,
+# and another multixact have no offest yet, we must wait until this offset
+# becomes observable. Previously we used to wait for 1ms in a loop in this
+# case, but now we use CV for this. This test is exercising such a sleep.
+
+use strict;
+use warnings FATAL => 'all';
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+
+use Test::More;
+
+if ($ENV{enable_injection_points} ne 'yes')
+{
+	plan skip_all => 'Injection points not supported by this build';
+}
+
+my ($node, $result);
+
+$node = PostgreSQL::Test::Cluster->new('multixact_CV_sleep');
+$node->init;
+$node->append_conf('postgresql.conf',
+	"shared_preload_libraries = 'test_slru'");
+$node->start;
+$node->safe_psql('postgres', q(CREATE EXTENSION injection_points));
+$node->safe_psql('postgres', q(CREATE EXTENSION test_slru));
+
+# Test for Multixact generation edge case
+$node->safe_psql('postgres', q(select injection_points_attach('test_read_multixact','wait')));
+$node->safe_psql('postgres', q(select injection_points_attach('GetMultiXactIdMembers-CV-sleep','wait')));
+
+# This session must observe sleep on CV when generating multixact.
+# To achive this it first will create a multixact, then pause before reading it.
+my $observer = $node->background_psql('postgres');
+
+# This query will create multixact, and hand just before reading it.
+$observer->query_until(qr/start/,
+q(
+	\echo start
+	select test_read_multixact(test_create_multixact());
+));
+$node->wait_for_event('client backend', 'test_read_multixact');
+
+# This session will create next Multixact, it's necessary to avoid edge case 1
+# (see multixact.c)
+my $creator = $node->background_psql('postgres');
+$node->safe_psql('postgres', q(select injection_points_attach('GetNewMultiXactId-done','wait');));
+
+# We expect this query to hand in critical section after generating new multixact,
+# but before filling it's offset into SLRU
+$creator->query_until(qr/start/, q(
+	\echo start
+	select injection_points_load('GetNewMultiXactId-done');
+	select test_create_multixact();
+));
+
+$node->wait_for_event('client backend', 'GetNewMultiXactId-done');
+
+# Now we are sure we can reach edge case 2.
+# Observer is going to read multixact, which has next, but next lacks offset.
+$node->safe_psql('postgres', q(select injection_points_wakeup('test_read_multixact')));
+
+
+$node->wait_for_event('client backend', 'GetMultiXactIdMembers-CV-sleep');
+
+# Now we have two backends waiting in GetNewMultiXactId-done and
+# GetMultiXactIdMembers-CV-sleep. Also we have 3 injections points set to wait.
+# If we wakeup GetMultiXactIdMembers-CV-sleep it will happend again, so we must
+# detach it first. So let's detach all injection points, then wake up all
+# backends.
+
+$node->safe_psql('postgres', q(select injection_points_detach('test_read_multixact')));
+$node->safe_psql('postgres', q(select injection_points_detach('GetNewMultiXactId-done')));
+$node->safe_psql('postgres', q(select injection_points_detach('GetMultiXactIdMembers-CV-sleep')));
+
+$node->safe_psql('postgres', q(select injection_points_wakeup('GetNewMultiXactId-done')));
+$node->safe_psql('postgres', q(select injection_points_wakeup('GetMultiXactIdMembers-CV-sleep')));
+
+# Background psql will now be able to read the result and disconnect.
+$observer->quit;
+$creator->quit;
+
+$node->stop;
+
+# If we reached this point - everything is OK.
+ok(1);
+done_testing();
diff --git a/src/test/modules/test_slru/test_slru--1.0.sql b/src/test/modules/test_slru/test_slru--1.0.sql
index 202e8da3fd..58300c59a7 100644
--- a/src/test/modules/test_slru/test_slru--1.0.sql
+++ b/src/test/modules/test_slru/test_slru--1.0.sql
@@ -19,3 +19,9 @@ CREATE OR REPLACE FUNCTION test_slru_page_truncate(bigint) RETURNS VOID
   AS 'MODULE_PATHNAME', 'test_slru_page_truncate' LANGUAGE C;
 CREATE OR REPLACE FUNCTION test_slru_delete_all() RETURNS VOID
   AS 'MODULE_PATHNAME', 'test_slru_delete_all' LANGUAGE C;
+
+
+CREATE OR REPLACE FUNCTION test_create_multixact() RETURNS xid
+  AS 'MODULE_PATHNAME', 'test_create_multixact' LANGUAGE C;
+CREATE OR REPLACE FUNCTION test_read_multixact(xid) RETURNS VOID
+  AS 'MODULE_PATHNAME', 'test_read_multixact'LANGUAGE C;
\ No newline at end of file
diff --git a/src/test/modules/test_slru/test_slru.c b/src/test/modules/test_slru/test_slru.c
index d227b06703..699a9cd64b 100644
--- a/src/test/modules/test_slru/test_slru.c
+++ b/src/test/modules/test_slru/test_slru.c
@@ -14,13 +14,16 @@
 
 #include "postgres.h"
 
+#include "access/multixact.h"
 #include "access/slru.h"
+#include "access/xact.h"
 #include "access/transam.h"
 #include "miscadmin.h"
 #include "storage/fd.h"
 #include "storage/ipc.h"
 #include "storage/shmem.h"
 #include "utils/builtins.h"
+#include "utils/injection_point.h"
 
 PG_MODULE_MAGIC;
 
@@ -36,6 +39,8 @@ PG_FUNCTION_INFO_V1(test_slru_page_sync);
 PG_FUNCTION_INFO_V1(test_slru_page_delete);
 PG_FUNCTION_INFO_V1(test_slru_page_truncate);
 PG_FUNCTION_INFO_V1(test_slru_delete_all);
+PG_FUNCTION_INFO_V1(test_create_multixact);
+PG_FUNCTION_INFO_V1(test_read_multixact);
 
 /* Number of SLRU page slots */
 #define NUM_TEST_BUFFERS		16
@@ -260,3 +265,36 @@ _PG_init(void)
 	prev_shmem_startup_hook = shmem_startup_hook;
 	shmem_startup_hook = test_slru_shmem_startup;
 }
+
+/*
+ * Produces multixact with 2 current xids
+ */
+Datum
+test_create_multixact(PG_FUNCTION_ARGS)
+{
+	MultiXactId id;
+	MultiXactIdSetOldestMember();
+	id = MultiXactIdCreate(GetCurrentTransactionId(), MultiXactStatusUpdate,
+						GetCurrentTransactionId(), MultiXactStatusForShare);
+	PG_RETURN_TRANSACTIONID(id);
+}
+
+/*
+ * Reads given multixact after running an injection point. Discards local cache
+ * to make real read.
+ * This function is expected to be used in conjunction with test_create_multixact
+ * to test CV sleep when reading recent multixact.
+ */
+Datum
+test_read_multixact(PG_FUNCTION_ARGS)
+{
+	MultiXactId id = PG_GETARG_TRANSACTIONID(0);
+	MultiXactMember *members;
+	INJECTION_POINT("test_read_multixact");
+	/* discard caches */
+	AtEOXact_MultiXact();
+
+	if (GetMultiXactIdMembers(id,&members,false, false) == -1)
+		elog(ERROR, "MultiXactId not found");
+	PG_RETURN_VOID();
+}
-- 
2.39.3 (Apple Git-146)

From e33ab4202cf5e6ee79eef1927978d45e1cfa6034 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Tue, 16 Jul 2024 13:07:58 +0900
Subject: [PATCH] Add INJECTION_POINT_CACHED()

This works in combination with INJECTION_POINT_LOAD(), ensuring that an
injection point can be run without touching the shared memory area
related to injection points.
---
 src/include/utils/injection_point.h           |  3 ++
 src/backend/utils/misc/injection_point.c      | 33 ++++++++++++++++---
 .../expected/injection_points.out             | 13 ++++++++
 .../injection_points--1.0.sql                 | 10 ++++++
 .../injection_points/injection_points.c       | 14 ++++++++
 .../injection_points/sql/injection_points.sql |  2 ++
 doc/src/sgml/xfunc.sgml                       |  6 +++-
 7 files changed, 76 insertions(+), 5 deletions(-)

diff --git a/src/include/utils/injection_point.h b/src/include/utils/injection_point.h
index bd3a62425c..a385e3df64 100644
--- a/src/include/utils/injection_point.h
+++ b/src/include/utils/injection_point.h
@@ -17,9 +17,11 @@
 #ifdef USE_INJECTION_POINTS
 #define INJECTION_POINT_LOAD(name) InjectionPointLoad(name)
 #define INJECTION_POINT(name) InjectionPointRun(name)
+#define INJECTION_POINT_CACHED(name) InjectionPointCached(name)
 #else
 #define INJECTION_POINT_LOAD(name) ((void) name)
 #define INJECTION_POINT(name) ((void) name)
+#define INJECTION_POINT_CACHED(name) ((void) name)
 #endif
 
 /*
@@ -38,6 +40,7 @@ extern void InjectionPointAttach(const char *name,
 								 int private_data_size);
 extern void InjectionPointLoad(const char *name);
 extern void InjectionPointRun(const char *name);
+extern void InjectionPointCached(const char *name);
 extern bool InjectionPointDetach(const char *name);
 
 #endif							/* INJECTION_POINT_H */
diff --git a/src/backend/utils/misc/injection_point.c b/src/backend/utils/misc/injection_point.c
index 84ad5e470d..9dd6575a9a 100644
--- a/src/backend/utils/misc/injection_point.c
+++ b/src/backend/utils/misc/injection_point.c
@@ -415,10 +415,11 @@ InjectionPointDetach(const char *name)
  * Common workhorse of InjectionPointRun() and InjectionPointLoad()
  *
  * Checks if an injection point exists in shared memory, and update
- * the local cache entry accordingly.
+ * the local cache entry accordingly.  If "direct" is true, return the
+ * point after looking up for it only in the cache.
  */
 static InjectionPointCacheEntry *
-InjectionPointCacheRefresh(const char *name)
+InjectionPointCacheRefresh(const char *name, bool direct)
 {
 	uint32		max_inuse;
 	int			namelen;
@@ -461,6 +462,13 @@ InjectionPointCacheRefresh(const char *name)
 		cached = NULL;
 	}
 
+	/*
+	 * If using a direct lookup, forget about the shared memory array
+	 * and return immediately with the data found in the cache.
+	 */
+	if (direct)
+		return cached;
+
 	/*
 	 * Search the shared memory array.
 	 *
@@ -531,7 +539,7 @@ void
 InjectionPointLoad(const char *name)
 {
 #ifdef USE_INJECTION_POINTS
-	InjectionPointCacheRefresh(name);
+	InjectionPointCacheRefresh(name, false);
 #else
 	elog(ERROR, "Injection points are not supported by this build");
 #endif
@@ -546,7 +554,24 @@ InjectionPointRun(const char *name)
 #ifdef USE_INJECTION_POINTS
 	InjectionPointCacheEntry *cache_entry;
 
-	cache_entry = InjectionPointCacheRefresh(name);
+	cache_entry = InjectionPointCacheRefresh(name, false);
+	if (cache_entry)
+		cache_entry->callback(name, cache_entry->private_data);
+#else
+	elog(ERROR, "Injection points are not supported by this build");
+#endif
+}
+
+/*
+ * Execute an injection point directly from the cache, if defined.
+ */
+void
+InjectionPointCached(const char *name)
+{
+#ifdef USE_INJECTION_POINTS
+	InjectionPointCacheEntry *cache_entry;
+
+	cache_entry = InjectionPointCacheRefresh(name, true);
 	if (cache_entry)
 		cache_entry->callback(name, cache_entry->private_data);
 #else
diff --git a/src/test/modules/injection_points/expected/injection_points.out b/src/test/modules/injection_points/expected/injection_points.out
index 2f60da900b..f25bbe4966 100644
--- a/src/test/modules/injection_points/expected/injection_points.out
+++ b/src/test/modules/injection_points/expected/injection_points.out
@@ -129,6 +129,12 @@ SELECT injection_points_detach('TestInjectionLog2');
 (1 row)
 
 -- Loading
+SELECT injection_points_cached('TestInjectionLogLoad'); -- nothing in cache
+ injection_points_cached 
+-------------------------
+ 
+(1 row)
+
 SELECT injection_points_load('TestInjectionLogLoad'); -- nothing
  injection_points_load 
 -----------------------
@@ -147,6 +153,13 @@ SELECT injection_points_load('TestInjectionLogLoad'); -- nothing happens
  
 (1 row)
 
+SELECT injection_points_cached('TestInjectionLogLoad'); -- runs from cache
+NOTICE:  notice triggered for injection point TestInjectionLogLoad
+ injection_points_cached 
+-------------------------
+ 
+(1 row)
+
 SELECT injection_points_run('TestInjectionLogLoad'); -- runs from cache
 NOTICE:  notice triggered for injection point TestInjectionLogLoad
  injection_points_run 
diff --git a/src/test/modules/injection_points/injection_points--1.0.sql b/src/test/modules/injection_points/injection_points--1.0.sql
index e275c2cf5b..0f280419a5 100644
--- a/src/test/modules/injection_points/injection_points--1.0.sql
+++ b/src/test/modules/injection_points/injection_points--1.0.sql
@@ -34,6 +34,16 @@ RETURNS void
 AS 'MODULE_PATHNAME', 'injection_points_run'
 LANGUAGE C STRICT PARALLEL UNSAFE;
 
+--
+-- injection_points_cached()
+--
+-- Executes the action attached to the injection point, from local cache.
+--
+CREATE FUNCTION injection_points_cached(IN point_name TEXT)
+RETURNS void
+AS 'MODULE_PATHNAME', 'injection_points_cached'
+LANGUAGE C STRICT PARALLEL UNSAFE;
+
 --
 -- injection_points_wakeup()
 --
diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c
index b6c8e89324..15f9d0233c 100644
--- a/src/test/modules/injection_points/injection_points.c
+++ b/src/test/modules/injection_points/injection_points.c
@@ -333,6 +333,20 @@ injection_points_run(PG_FUNCTION_ARGS)
 	PG_RETURN_VOID();
 }
 
+/*
+ * SQL function for triggering an injection point from cache.
+ */
+PG_FUNCTION_INFO_V1(injection_points_cached);
+Datum
+injection_points_cached(PG_FUNCTION_ARGS)
+{
+	char	   *name = text_to_cstring(PG_GETARG_TEXT_PP(0));
+
+	INJECTION_POINT_CACHED(name);
+
+	PG_RETURN_VOID();
+}
+
 /*
  * SQL function for waking up an injection point waiting in injection_wait().
  */
diff --git a/src/test/modules/injection_points/sql/injection_points.sql b/src/test/modules/injection_points/sql/injection_points.sql
index fabf0a8823..e3a481d604 100644
--- a/src/test/modules/injection_points/sql/injection_points.sql
+++ b/src/test/modules/injection_points/sql/injection_points.sql
@@ -42,9 +42,11 @@ SELECT injection_points_run('TestInjectionLog2'); -- notice
 SELECT injection_points_detach('TestInjectionLog2');
 
 -- Loading
+SELECT injection_points_cached('TestInjectionLogLoad'); -- nothing in cache
 SELECT injection_points_load('TestInjectionLogLoad'); -- nothing
 SELECT injection_points_attach('TestInjectionLogLoad', 'notice');
 SELECT injection_points_load('TestInjectionLogLoad'); -- nothing happens
+SELECT injection_points_cached('TestInjectionLogLoad'); -- runs from cache
 SELECT injection_points_run('TestInjectionLogLoad'); -- runs from cache
 SELECT injection_points_detach('TestInjectionLogLoad');
 
diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
index 756a9d07fb..f02a48ead4 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -3629,7 +3629,11 @@ INJECTION_POINT_LOAD(name);
      doing all memory allocations at this stage without running the callback.
      This is useful when an injection point is attached in a critical section
      where no memory can be allocated: load the injection point outside the
-     critical section, then run it in the critical section.
+     critical section, then run it in the critical section directly from
+     the process cache:
+<programlisting>
+INJECTION_POINT_CACHED(name);
+</programlisting>
     </para>
 
     <para>
-- 
2.45.2

From 03a91db4fc7af15651f735eef4295c7d5883def7 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Tue, 16 Jul 2024 13:07:58 +0900
Subject: [PATCH v2 1/2] Add INJECTION_POINT_CACHED()

This works in combination with INJECTION_POINT_LOAD(), ensuring that an
injection point can be run without touching the shared memory area
related to injection points.
---
 doc/src/sgml/xfunc.sgml                       |  6 +++++-
 src/backend/utils/misc/injection_point.c      | 19 ++++++++++++++++++-
 src/include/utils/injection_point.h           |  3 +++
 .../expected/injection_points.out             | 13 +++++++++++++
 .../injection_points--1.0.sql                 | 10 ++++++++++
 .../injection_points/injection_points.c       | 14 ++++++++++++++
 .../injection_points/sql/injection_points.sql |  2 ++
 7 files changed, 65 insertions(+), 2 deletions(-)

diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
index 756a9d07fb..f02a48ead4 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -3629,7 +3629,11 @@ INJECTION_POINT_LOAD(name);
      doing all memory allocations at this stage without running the callback.
      This is useful when an injection point is attached in a critical section
      where no memory can be allocated: load the injection point outside the
-     critical section, then run it in the critical section.
+     critical section, then run it in the critical section directly from
+     the process cache:
+<programlisting>
+INJECTION_POINT_CACHED(name);
+</programlisting>
     </para>
 
     <para>
diff --git a/src/backend/utils/misc/injection_point.c b/src/backend/utils/misc/injection_point.c
index 84ad5e470d..1fa7d95686 100644
--- a/src/backend/utils/misc/injection_point.c
+++ b/src/backend/utils/misc/injection_point.c
@@ -531,7 +531,7 @@ void
 InjectionPointLoad(const char *name)
 {
 #ifdef USE_INJECTION_POINTS
-	InjectionPointCacheRefresh(name);
+	InjectionPointCacheRefresh(name, false);
 #else
 	elog(ERROR, "Injection points are not supported by this build");
 #endif
@@ -553,3 +553,20 @@ InjectionPointRun(const char *name)
 	elog(ERROR, "Injection points are not supported by this build");
 #endif
 }
+
+/*
+ * Execute an injection point directly from the cache, if defined.
+ */
+void
+InjectionPointCached(const char *name)
+{
+#ifdef USE_INJECTION_POINTS
+	InjectionPointCacheEntry *cache_entry;
+
+	cache_entry = injection_point_cache_get(name);
+	if (cache_entry)
+		cache_entry->callback(name, cache_entry->private_data);
+#else
+	elog(ERROR, "Injection points are not supported by this build");
+#endif
+}
diff --git a/src/include/utils/injection_point.h b/src/include/utils/injection_point.h
index bd3a62425c..a385e3df64 100644
--- a/src/include/utils/injection_point.h
+++ b/src/include/utils/injection_point.h
@@ -17,9 +17,11 @@
 #ifdef USE_INJECTION_POINTS
 #define INJECTION_POINT_LOAD(name) InjectionPointLoad(name)
 #define INJECTION_POINT(name) InjectionPointRun(name)
+#define INJECTION_POINT_CACHED(name) InjectionPointCached(name)
 #else
 #define INJECTION_POINT_LOAD(name) ((void) name)
 #define INJECTION_POINT(name) ((void) name)
+#define INJECTION_POINT_CACHED(name) ((void) name)
 #endif
 
 /*
@@ -38,6 +40,7 @@ extern void InjectionPointAttach(const char *name,
 								 int private_data_size);
 extern void InjectionPointLoad(const char *name);
 extern void InjectionPointRun(const char *name);
+extern void InjectionPointCached(const char *name);
 extern bool InjectionPointDetach(const char *name);
 
 #endif							/* INJECTION_POINT_H */
diff --git a/src/test/modules/injection_points/expected/injection_points.out b/src/test/modules/injection_points/expected/injection_points.out
index 2f60da900b..f25bbe4966 100644
--- a/src/test/modules/injection_points/expected/injection_points.out
+++ b/src/test/modules/injection_points/expected/injection_points.out
@@ -129,6 +129,12 @@ SELECT injection_points_detach('TestInjectionLog2');
 (1 row)
 
 -- Loading
+SELECT injection_points_cached('TestInjectionLogLoad'); -- nothing in cache
+ injection_points_cached 
+-------------------------
+ 
+(1 row)
+
 SELECT injection_points_load('TestInjectionLogLoad'); -- nothing
  injection_points_load 
 -----------------------
@@ -147,6 +153,13 @@ SELECT injection_points_load('TestInjectionLogLoad'); -- nothing happens
  
 (1 row)
 
+SELECT injection_points_cached('TestInjectionLogLoad'); -- runs from cache
+NOTICE:  notice triggered for injection point TestInjectionLogLoad
+ injection_points_cached 
+-------------------------
+ 
+(1 row)
+
 SELECT injection_points_run('TestInjectionLogLoad'); -- runs from cache
 NOTICE:  notice triggered for injection point TestInjectionLogLoad
  injection_points_run 
diff --git a/src/test/modules/injection_points/injection_points--1.0.sql b/src/test/modules/injection_points/injection_points--1.0.sql
index e275c2cf5b..0f280419a5 100644
--- a/src/test/modules/injection_points/injection_points--1.0.sql
+++ b/src/test/modules/injection_points/injection_points--1.0.sql
@@ -34,6 +34,16 @@ RETURNS void
 AS 'MODULE_PATHNAME', 'injection_points_run'
 LANGUAGE C STRICT PARALLEL UNSAFE;
 
+--
+-- injection_points_cached()
+--
+-- Executes the action attached to the injection point, from local cache.
+--
+CREATE FUNCTION injection_points_cached(IN point_name TEXT)
+RETURNS void
+AS 'MODULE_PATHNAME', 'injection_points_cached'
+LANGUAGE C STRICT PARALLEL UNSAFE;
+
 --
 -- injection_points_wakeup()
 --
diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c
index b6c8e89324..15f9d0233c 100644
--- a/src/test/modules/injection_points/injection_points.c
+++ b/src/test/modules/injection_points/injection_points.c
@@ -333,6 +333,20 @@ injection_points_run(PG_FUNCTION_ARGS)
 	PG_RETURN_VOID();
 }
 
+/*
+ * SQL function for triggering an injection point from cache.
+ */
+PG_FUNCTION_INFO_V1(injection_points_cached);
+Datum
+injection_points_cached(PG_FUNCTION_ARGS)
+{
+	char	   *name = text_to_cstring(PG_GETARG_TEXT_PP(0));
+
+	INJECTION_POINT_CACHED(name);
+
+	PG_RETURN_VOID();
+}
+
 /*
  * SQL function for waking up an injection point waiting in injection_wait().
  */
diff --git a/src/test/modules/injection_points/sql/injection_points.sql b/src/test/modules/injection_points/sql/injection_points.sql
index fabf0a8823..e3a481d604 100644
--- a/src/test/modules/injection_points/sql/injection_points.sql
+++ b/src/test/modules/injection_points/sql/injection_points.sql
@@ -42,9 +42,11 @@ SELECT injection_points_run('TestInjectionLog2'); -- notice
 SELECT injection_points_detach('TestInjectionLog2');
 
 -- Loading
+SELECT injection_points_cached('TestInjectionLogLoad'); -- nothing in cache
 SELECT injection_points_load('TestInjectionLogLoad'); -- nothing
 SELECT injection_points_attach('TestInjectionLogLoad', 'notice');
 SELECT injection_points_load('TestInjectionLogLoad'); -- nothing happens
+SELECT injection_points_cached('TestInjectionLogLoad'); -- runs from cache
 SELECT injection_points_run('TestInjectionLogLoad'); -- runs from cache
 SELECT injection_points_detach('TestInjectionLogLoad');
 
-- 
2.39.2

From e12026251ae4817e73a41c00abb1bb6884a2b248 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Tue, 16 Jul 2024 11:15:39 +0300
Subject: [PATCH v2 2/2] rephrase docs paragraph

---
 doc/src/sgml/xfunc.sgml | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
index f02a48ead4..7e92e89846 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -3619,21 +3619,20 @@ INJECTION_POINT(name);
     </para>
 
     <para>
-     An injection point with a given <literal>name</literal> can be loaded
-     using macro:
+     Executing an injection point can require allocating a small amount of
+     memory, which can fail. If you need to have an injection point in a
+     critical section where dynamic allocations are not allowed, you can use
+     a two-step approach with the following macros:
 <programlisting>
 INJECTION_POINT_LOAD(name);
-</programlisting>
-
-     This will load the injection point callback into the process cache,
-     doing all memory allocations at this stage without running the callback.
-     This is useful when an injection point is attached in a critical section
-     where no memory can be allocated: load the injection point outside the
-     critical section, then run it in the critical section directly from
-     the process cache:
-<programlisting>
 INJECTION_POINT_CACHED(name);
 </programlisting>
+
+     Before entering the critical section,
+     call <function>INJECTION_POINT_LOAD</function>. It checks the shared
+     memory state, and loads the callback into backend-private memory if it is
+     active. Inside the critical section, use
+     <function>INJECTION_POINT_CACHED</function> to execute the callback.
     </para>
 
     <para>
-- 
2.39.2

From 651bdc4cab36e5294318e5a601688308c086984e Mon Sep 17 00:00:00 2001
From: Andrey Borodin <amborodin@acm.org>
Date: Mon, 20 May 2024 15:20:31 +0500
Subject: [PATCH vAug2] Add multixact CV sleep test

Previously we used 1ms sleep when we are reading multixact before
another multixact without offset yet. a0e0fb1ba changed this behavior
to CV sleep. This commit adds test for this edge case.
To test such race condition we have to use waiting injection point under
critical section. Such point requires special injection point loading.

Author: Andrey Borodin
Reviewed-by: Michael Paquier
---
 src/backend/access/transam/multixact.c        |  7 ++
 src/test/modules/test_slru/Makefile           |  4 +
 src/test/modules/test_slru/meson.build        |  8 ++
 src/test/modules/test_slru/t/001_multixact.pl | 91 +++++++++++++++++++
 src/test/modules/test_slru/test_slru--1.0.sql |  6 ++
 src/test/modules/test_slru/test_slru.c        | 38 ++++++++
 6 files changed, 154 insertions(+)
 create mode 100644 src/test/modules/test_slru/t/001_multixact.pl

diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index c601ff98a1..898ea8f4c9 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -88,6 +88,7 @@
 #include "storage/proc.h"
 #include "storage/procarray.h"
 #include "utils/fmgrprotos.h"
+#include "utils/injection_point.h"
 #include "utils/guc_hooks.h"
 #include "utils/memutils.h"
 
@@ -866,8 +867,12 @@ MultiXactIdCreateFromMembers(int nmembers, MultiXactMember *members)
 	 * in vacuum.  During vacuum, in particular, it would be unacceptable to
 	 * keep OldestMulti set, in case it runs for long.
 	 */
+	INJECTION_POINT_LOAD("get-new-multixact-id");
+
 	multi = GetNewMultiXactId(nmembers, &offset);
 
+	INJECTION_POINT_CACHED("get-new-multixact-id");
+
 	/* Make an XLOG entry describing the new MXID. */
 	xlrec.mid = multi;
 	xlrec.moff = offset;
@@ -1480,6 +1485,8 @@ retry:
 			LWLockRelease(lock);
 			CHECK_FOR_INTERRUPTS();
 
+			INJECTION_POINT("get-multixact-member-cv-sleep");
+
 			ConditionVariableSleep(&MultiXactState->nextoff_cv,
 								   WAIT_EVENT_MULTIXACT_CREATION);
 			slept = true;
diff --git a/src/test/modules/test_slru/Makefile b/src/test/modules/test_slru/Makefile
index 936886753b..8f9623b128 100644
--- a/src/test/modules/test_slru/Makefile
+++ b/src/test/modules/test_slru/Makefile
@@ -6,6 +6,10 @@ OBJS = \
 	test_slru.o
 PGFILEDESC = "test_slru - test module for SLRUs"
 
+EXTRA_INSTALL=src/test/modules/injection_points
+export enable_injection_points enable_injection_points
+TAP_TESTS = 1
+
 EXTENSION = test_slru
 DATA = test_slru--1.0.sql
 
diff --git a/src/test/modules/test_slru/meson.build b/src/test/modules/test_slru/meson.build
index ce91e60631..4a5bc6349a 100644
--- a/src/test/modules/test_slru/meson.build
+++ b/src/test/modules/test_slru/meson.build
@@ -32,4 +32,12 @@ tests += {
     'regress_args': ['--temp-config', files('test_slru.conf')],
     'runningcheck': false,
   },
+  'tap': {
+    'env': {
+       'enable_injection_points': get_option('injection_points') ? 'yes' : 'no',
+    },
+    'tests': [
+      't/001_multixact.pl'
+    ],
+  },
 }
diff --git a/src/test/modules/test_slru/t/001_multixact.pl b/src/test/modules/test_slru/t/001_multixact.pl
new file mode 100644
index 0000000000..b3d262e3c1
--- /dev/null
+++ b/src/test/modules/test_slru/t/001_multixact.pl
@@ -0,0 +1,91 @@
+# Copyright (c) 2024, PostgreSQL Global Development Group
+
+# This test verifies edge case of reading a multixact:
+# when we have multixact that is followed by exactly one another multixact,
+# and another multixact have no offset yet, we must wait until this offset
+# becomes observable. Previously we used to wait for 1ms in a loop in this
+# case, but now we use CV for this. This test is exercising such a sleep.
+
+use strict;
+use warnings FATAL => 'all';
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+
+use Test::More;
+
+if ($ENV{enable_injection_points} ne 'yes')
+{
+	plan skip_all => 'Injection points not supported by this build';
+}
+
+my ($node, $result);
+
+$node = PostgreSQL::Test::Cluster->new('multixact_CV_sleep');
+$node->init;
+$node->append_conf('postgresql.conf',
+	"shared_preload_libraries = 'test_slru'");
+$node->start;
+$node->safe_psql('postgres', q(CREATE EXTENSION injection_points));
+$node->safe_psql('postgres', q(CREATE EXTENSION test_slru));
+
+# Test for Multixact generation edge case
+$node->safe_psql('postgres', q(select injection_points_attach('test-read-multixact','wait')));
+$node->safe_psql('postgres', q(select injection_points_attach('get-multixact-member-cv-sleep','wait')));
+
+# This session must observe sleep on CV when generating multixact.
+# To achieve this it first will create a multixact, then pause before reading it.
+my $observer = $node->background_psql('postgres');
+
+# This query will create multixact, and hand just before reading it.
+$observer->query_until(qr/start/,
+q(
+	\echo start
+	select test_read_multixact(test_create_multixact());
+));
+$node->wait_for_event('client backend', 'test-read-multixact');
+
+# This session will create next Multixact, it's necessary to avoid edge case 1
+# (see multixact.c)
+my $creator = $node->background_psql('postgres');
+$node->safe_psql('postgres', q(select injection_points_attach('get-new-multixact-id','wait');));
+
+# We expect this query to hand in critical section after generating new multixact,
+# but before filling it's offset into SLRU.
+# Running injection point under SLRU requires special loading of cache.
+$creator->query_until(qr/start/, q(
+	\echo start
+	select test_create_multixact();
+));
+
+$node->wait_for_event('client backend', 'get-new-multixact-id');
+
+# Now we are sure we can reach edge case 2.
+# Observer is going to read multixact, which has next, but next lacks offset.
+$node->safe_psql('postgres', q(select injection_points_wakeup('test-read-multixact')));
+
+
+$node->wait_for_event('client backend', 'get-multixact-member-cv-sleep');
+
+# Now we have two backends waiting in get-new-multixact-id and
+# get-multixact-member-cv-sleep. Also we have 3 injections points set to wait.
+# If we wakeup get-multixact-member-cv-sleep it will happen again, so we must
+# detach it first. So let's detach all injection points, then wake up all
+# backends.
+
+$node->safe_psql('postgres', q(select injection_points_detach('test-read-multixact')));
+$node->safe_psql('postgres', q(select injection_points_detach('get-new-multixact-id')));
+$node->safe_psql('postgres', q(select injection_points_detach('get-multixact-member-cv-sleep')));
+
+$node->safe_psql('postgres', q(select injection_points_wakeup('get-new-multixact-id')));
+$node->safe_psql('postgres', q(select injection_points_wakeup('get-multixact-member-cv-sleep')));
+
+# Background psql will now be able to read the result and disconnect.
+$observer->quit;
+$creator->quit;
+
+$node->stop;
+
+# If we reached this point - everything is OK.
+ok(1);
+done_testing();
diff --git a/src/test/modules/test_slru/test_slru--1.0.sql b/src/test/modules/test_slru/test_slru--1.0.sql
index 202e8da3fd..58300c59a7 100644
--- a/src/test/modules/test_slru/test_slru--1.0.sql
+++ b/src/test/modules/test_slru/test_slru--1.0.sql
@@ -19,3 +19,9 @@ CREATE OR REPLACE FUNCTION test_slru_page_truncate(bigint) RETURNS VOID
   AS 'MODULE_PATHNAME', 'test_slru_page_truncate' LANGUAGE C;
 CREATE OR REPLACE FUNCTION test_slru_delete_all() RETURNS VOID
   AS 'MODULE_PATHNAME', 'test_slru_delete_all' LANGUAGE C;
+
+
+CREATE OR REPLACE FUNCTION test_create_multixact() RETURNS xid
+  AS 'MODULE_PATHNAME', 'test_create_multixact' LANGUAGE C;
+CREATE OR REPLACE FUNCTION test_read_multixact(xid) RETURNS VOID
+  AS 'MODULE_PATHNAME', 'test_read_multixact'LANGUAGE C;
\ No newline at end of file
diff --git a/src/test/modules/test_slru/test_slru.c b/src/test/modules/test_slru/test_slru.c
index d227b06703..56136bc117 100644
--- a/src/test/modules/test_slru/test_slru.c
+++ b/src/test/modules/test_slru/test_slru.c
@@ -14,13 +14,16 @@
 
 #include "postgres.h"
 
+#include "access/multixact.h"
 #include "access/slru.h"
+#include "access/xact.h"
 #include "access/transam.h"
 #include "miscadmin.h"
 #include "storage/fd.h"
 #include "storage/ipc.h"
 #include "storage/shmem.h"
 #include "utils/builtins.h"
+#include "utils/injection_point.h"
 
 PG_MODULE_MAGIC;
 
@@ -36,6 +39,8 @@ PG_FUNCTION_INFO_V1(test_slru_page_sync);
 PG_FUNCTION_INFO_V1(test_slru_page_delete);
 PG_FUNCTION_INFO_V1(test_slru_page_truncate);
 PG_FUNCTION_INFO_V1(test_slru_delete_all);
+PG_FUNCTION_INFO_V1(test_create_multixact);
+PG_FUNCTION_INFO_V1(test_read_multixact);
 
 /* Number of SLRU page slots */
 #define NUM_TEST_BUFFERS		16
@@ -260,3 +265,36 @@ _PG_init(void)
 	prev_shmem_startup_hook = shmem_startup_hook;
 	shmem_startup_hook = test_slru_shmem_startup;
 }
+
+/*
+ * Produces multixact with 2 current xids
+ */
+Datum
+test_create_multixact(PG_FUNCTION_ARGS)
+{
+	MultiXactId id;
+	MultiXactIdSetOldestMember();
+	id = MultiXactIdCreate(GetCurrentTransactionId(), MultiXactStatusUpdate,
+						GetCurrentTransactionId(), MultiXactStatusForShare);
+	PG_RETURN_TRANSACTIONID(id);
+}
+
+/*
+ * Reads given multixact after running an injection point. Discards local cache
+ * to make real read.
+ * This function is expected to be used in conjunction with test_create_multixact
+ * to test CV sleep when reading recent multixact.
+ */
+Datum
+test_read_multixact(PG_FUNCTION_ARGS)
+{
+	MultiXactId id = PG_GETARG_TRANSACTIONID(0);
+	MultiXactMember *members;
+	INJECTION_POINT("test-read-multixact");
+	/* discard caches */
+	AtEOXact_MultiXact();
+
+	if (GetMultiXactIdMembers(id,&members,false, false) == -1)
+		elog(ERROR, "MultiXactId not found");
+	PG_RETURN_VOID();
+}
-- 
2.39.3 (Apple Git-146)