Add support for runtime arguments in injection points

michaelpq · michaelpq · commit 371f2db8b05e · 2025-05-10T06:56:26.000+09:00
The macros INJECTION_POINT() and INJECTION_POINT_CACHED() are extended with an optional argument that can be passed down to the callback attached when an injection point is run, giving to callbacks the possibility to manipulate a stack state given by the caller. The existing callbacks in modules injection_points and test_aio have their declarations adjusted based on that. da72269 (core AIO infrastructure) and 93bc3d7 (test_aio) and been relying on a set of workarounds where a static variable called pgaio_inj_cur_handle is used as runtime argument in the injection point callbacks used by the AIO tests, in combination with a TRY/CATCH block to reset the argument value. The infrastructure introduced in this commit will be reused for the AIO tests, simplifying them. Reviewed-by: Greg Burd <greg@burd.me> Discussion: https://postgr.es/m/Z_y9TtnXubvYAApS@paquier.xyz
diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
@@ -3829,15 +3829,17 @@ uint32 WaitEventExtensionNew(const char *wait_event_name)
      An injection point with a given <literal>name</literal> is declared using
      macro:
 <programlisting>
-INJECTION_POINT(name);
+INJECTION_POINT(name, arg);
 </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. The injection point names should
-     use lower-case characters, with terms separated by dashes.
+     their own code using the same macro. The injection point names should use
+     lower-case characters, with terms separated by
+     dashes. <literal>arg</literal> is an optional argument value given to the
+     callback at run-time.
     </para>
 
     <para>
@@ -3847,7 +3849,7 @@ INJECTION_POINT(name);
      a two-step approach with the following macros:
 <programlisting>
 INJECTION_POINT_LOAD(name);
-INJECTION_POINT_CACHED(name);
+INJECTION_POINT_CACHED(name, arg);
 </programlisting>
 
      Before entering the critical section,
@@ -3880,7 +3882,9 @@ extern void InjectionPointAttach(const char *name,
      <literal>InjectionPointCallback</literal>:
 <programlisting>
 static void
-custom_injection_callback(const char *name, const void *private_data)
+custom_injection_callback(const char *name,
+                          const void *private_data,
+                          void *arg)
 {
     uint32 wait_event_info = WaitEventInjectionPointNew(name);
 
@@ -3909,7 +3913,7 @@ if (IS_INJECTION_POINT_ATTACHED("before-foobar"))
     local_var = 123;
 
     /* also execute the callback */
-    INJECTION_POINT_CACHED("before-foobar");
+    INJECTION_POINT_CACHED("before-foobar", NULL);
 }
 #endif
 </programlisting>
diff --git a/src/backend/access/gin/ginbtree.c b/src/backend/access/gin/ginbtree.c
@@ -685,9 +685,9 @@ ginFinishSplit(GinBtree btree, GinBtreeStack *stack, bool freestack,
 
 #ifdef USE_INJECTION_POINTS
 		if (GinPageIsLeaf(BufferGetPage(stack->buffer)))
-			INJECTION_POINT("gin-leave-leaf-split-incomplete");
+			INJECTION_POINT("gin-leave-leaf-split-incomplete", NULL);
 		else
-			INJECTION_POINT("gin-leave-internal-split-incomplete");
+			INJECTION_POINT("gin-leave-internal-split-incomplete", NULL);
 #endif
 
 		/* search parent to lock */
@@ -778,7 +778,7 @@ ginFinishSplit(GinBtree btree, GinBtreeStack *stack, bool freestack,
 static void
 ginFinishOldSplit(GinBtree btree, GinBtreeStack *stack, GinStatsData *buildStats, int access)
 {
-	INJECTION_POINT("gin-finish-incomplete-split");
+	INJECTION_POINT("gin-finish-incomplete-split", NULL);
 	elog(DEBUG1, "finishing incomplete split of block %u in gin index \"%s\"",
 		 stack->blkno, RelationGetRelationName(btree->index));
 
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
@@ -3304,7 +3304,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
 	interesting_attrs = bms_add_members(interesting_attrs, id_attrs);
 
 	block = ItemPointerGetBlockNumber(otid);
-	INJECTION_POINT("heap_update-before-pin");
+	INJECTION_POINT("heap_update-before-pin", NULL);
 	buffer = ReadBuffer(relation, block);
 	page = BufferGetPage(buffer);
 
diff --git a/src/backend/access/index/genam.c b/src/backend/access/index/genam.c
@@ -851,7 +851,7 @@ systable_inplace_update_begin(Relation relation,
 		if (retries++ > 10000)
 			elog(ERROR, "giving up after too many tries to overwrite row");
 
-		INJECTION_POINT("inplace-before-pin");
+		INJECTION_POINT("inplace-before-pin", NULL);
 		scan = systable_beginscan(relation, indexId, indexOK, snapshot,
 								  nkeys, unconstify(ScanKeyData *, key));
 		oldtup = systable_getnext(scan);
diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
@@ -872,7 +872,7 @@ MultiXactIdCreateFromMembers(int nmembers, MultiXactMember *members)
 	 */
 	multi = GetNewMultiXactId(nmembers, &offset);
 
-	INJECTION_POINT_CACHED("multixact-create-from-members");
+	INJECTION_POINT_CACHED("multixact-create-from-members", NULL);
 
 	/* Make an XLOG entry describing the new MXID. */
 	xlrec.mid = multi;
@@ -1486,7 +1486,7 @@ GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **members,
 			LWLockRelease(lock);
 			CHECK_FOR_INTERRUPTS();
 
-			INJECTION_POINT("multixact-get-members-cv-sleep");
+			INJECTION_POINT("multixact-get-members-cv-sleep", NULL);
 
 			ConditionVariableSleep(&MultiXactState->nextoff_cv,
 								   WAIT_EVENT_MULTIXACT_CREATION);
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
@@ -7882,7 +7882,7 @@ CreateRestartPoint(int flags)
 	 * This location needs to be after CheckPointGuts() to ensure that some
 	 * work has already happened during this checkpoint.
 	 */
-	INJECTION_POINT("create-restart-point");
+	INJECTION_POINT("create-restart-point", NULL);
 
 	/*
 	 * Remember the prior checkpoint's redo ptr for
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
@@ -3892,9 +3892,9 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein
 
 #ifdef USE_INJECTION_POINTS
 		if (idx->safe)
-			INJECTION_POINT("reindex-conc-index-safe");
+			INJECTION_POINT("reindex-conc-index-safe", NULL);
 		else
-			INJECTION_POINT("reindex-conc-index-not-safe");
+			INJECTION_POINT("reindex-conc-index-not-safe", NULL);
 #endif
 
 		idx->tableId = RelationGetRelid(heapRel);
diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
@@ -1492,7 +1492,7 @@ build_hash_tables(AggState *aggstate)
 		if (IS_INJECTION_POINT_ATTACHED("hash-aggregate-oversize-table"))
 		{
 			nbuckets = memory / TupleHashEntrySize();
-			INJECTION_POINT_CACHED("hash-aggregate-oversize-table");
+			INJECTION_POINT_CACHED("hash-aggregate-oversize-table", NULL);
 		}
 #endif
 
@@ -1882,7 +1882,7 @@ hash_agg_check_limits(AggState *aggstate)
 		if (IS_INJECTION_POINT_ATTACHED("hash-aggregate-spill-1000"))
 		{
 			do_spill = true;
-			INJECTION_POINT_CACHED("hash-aggregate-spill-1000");
+			INJECTION_POINT_CACHED("hash-aggregate-spill-1000", NULL);
 		}
 	}
 #endif
@@ -1910,7 +1910,7 @@ hash_agg_check_limits(AggState *aggstate)
 static void
 hash_agg_enter_spill_mode(AggState *aggstate)
 {
-	INJECTION_POINT("hash-aggregate-enter-spill-mode");
+	INJECTION_POINT("hash-aggregate-enter-spill-mode", NULL);
 	aggstate->hash_spill_mode = true;
 	hashagg_recompile_expressions(aggstate, aggstate->table_filled, true);
 
@@ -2739,7 +2739,7 @@ agg_refill_hash_table(AggState *aggstate)
 	 */
 	hashagg_recompile_expressions(aggstate, true, true);
 
-	INJECTION_POINT("hash-aggregate-process-batch");
+	INJECTION_POINT("hash-aggregate-process-batch", NULL);
 	for (;;)
 	{
 		TupleTableSlot *spillslot = aggstate->hash_spill_rslot;
@@ -2995,7 +2995,7 @@ hashagg_spill_init(HashAggSpill *spill, LogicalTapeSet *tapeset, int used_bits,
 	{
 		npartitions = 1;
 		partition_bits = 0;
-		INJECTION_POINT_CACHED("hash-aggregate-single-partition");
+		INJECTION_POINT_CACHED("hash-aggregate-single-partition", NULL);
 	}
 #endif
 
diff --git a/src/backend/libpq/be-secure-gssapi.c b/src/backend/libpq/be-secure-gssapi.c
@@ -500,7 +500,7 @@ secure_open_gssapi(Port *port)
 				minor;
 	gss_cred_id_t delegated_creds;
 
-	INJECTION_POINT("backend-gssapi-startup");
+	INJECTION_POINT("backend-gssapi-startup", NULL);
 
 	/*
 	 * Allocate subsidiary Port data for GSSAPI operations.
diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
@@ -131,7 +131,7 @@ secure_open_server(Port *port)
 	}
 	Assert(pq_buffer_remaining_data() == 0);
 
-	INJECTION_POINT("backend-ssl-startup");
+	INJECTION_POINT("backend-ssl-startup", NULL);
 
 	r = be_tls_open_server(port);
 
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
@@ -1922,7 +1922,7 @@ do_autovacuum(void)
 	 * This injection point is put in a transaction block to work with a wait
 	 * that uses a condition variable.
 	 */
-	INJECTION_POINT("autovacuum-worker-start");
+	INJECTION_POINT("autovacuum-worker-start", NULL);
 
 	/*
 	 * Compute the multixact age for which freezing is urgent.  This is
diff --git a/src/backend/storage/aio/aio.c b/src/backend/storage/aio/aio.c
@@ -1275,7 +1275,7 @@ pgaio_io_call_inj(PgAioHandle *ioh, const char *injection_point)
 
 	PG_TRY();
 	{
-		InjectionPointCached(injection_point);
+		InjectionPointCached(injection_point, NULL);
 	}
 	PG_FINALLY();
 	{
diff --git a/src/backend/tcop/backend_startup.c b/src/backend/tcop/backend_startup.c
@@ -234,7 +234,7 @@ BackendInitialize(ClientSocket *client_sock, CAC_state cac)
 
 	/* For testing client error handling */
 #ifdef USE_INJECTION_POINTS
-	INJECTION_POINT("backend-initialize");
+	INJECTION_POINT("backend-initialize", NULL);
 	if (IS_INJECTION_POINT_ATTACHED("backend-initialize-v2-error"))
 	{
 		/*
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
@@ -3482,7 +3482,7 @@ ProcessInterrupts(void)
 		IdleInTransactionSessionTimeoutPending = false;
 		if (IdleInTransactionSessionTimeout > 0)
 		{
-			INJECTION_POINT("idle-in-transaction-session-timeout");
+			INJECTION_POINT("idle-in-transaction-session-timeout", NULL);
 			ereport(FATAL,
 					(errcode(ERRCODE_IDLE_IN_TRANSACTION_SESSION_TIMEOUT),
 					 errmsg("terminating connection due to idle-in-transaction timeout")));
@@ -3495,7 +3495,7 @@ ProcessInterrupts(void)
 		TransactionTimeoutPending = false;
 		if (TransactionTimeout > 0)
 		{
-			INJECTION_POINT("transaction-timeout");
+			INJECTION_POINT("transaction-timeout", NULL);
 			ereport(FATAL,
 					(errcode(ERRCODE_TRANSACTION_TIMEOUT),
 					 errmsg("terminating connection due to transaction timeout")));
@@ -3508,7 +3508,7 @@ ProcessInterrupts(void)
 		IdleSessionTimeoutPending = false;
 		if (IdleSessionTimeout > 0)
 		{
-			INJECTION_POINT("idle-session-timeout");
+			INJECTION_POINT("idle-session-timeout", NULL);
 			ereport(FATAL,
 					(errcode(ERRCODE_IDLE_SESSION_TIMEOUT),
 					 errmsg("terminating connection due to idle-session timeout")));
diff --git a/src/backend/utils/cache/catcache.c b/src/backend/utils/cache/catcache.c
@@ -1926,7 +1926,7 @@ SearchCatCacheList(CatCache *cache,
 			/* Injection point to help testing the recursive invalidation case */
 			if (first_iter)
 			{
-				INJECTION_POINT("catcache-list-miss-systable-scan-started");
+				INJECTION_POINT("catcache-list-miss-systable-scan-started", NULL);
 				first_iter = false;
 			}
 
diff --git a/src/backend/utils/cache/inval.c b/src/backend/utils/cache/inval.c
@@ -1207,7 +1207,7 @@ AtEOXact_Inval(bool isCommit)
 	/* Must be at top of stack */
 	Assert(transInvalInfo->my_level == 1 && transInvalInfo->parent == NULL);
 
-	INJECTION_POINT("transaction-end-process-inval");
+	INJECTION_POINT("transaction-end-process-inval", NULL);
 
 	if (isCommit)
 	{
diff --git a/src/backend/utils/cache/typcache.c b/src/backend/utils/cache/typcache.c
@@ -952,7 +952,7 @@ lookup_type_cache(Oid type_id, int flags)
 		load_domaintype_info(typentry);
 	}
 
-	INJECTION_POINT("typecache-before-rel-type-cache-insert");
+	INJECTION_POINT("typecache-before-rel-type-cache-insert", NULL);
 
 	Assert(in_progress_offset + 1 == in_progress_list_len);
 	in_progress_list_len--;
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
@@ -747,7 +747,7 @@ InitPostgres(const char *in_dbname, Oid dboid,
 	if (!bootstrap)
 	{
 		pgstat_bestart_initial();
-		INJECTION_POINT("init-pre-auth");
+		INJECTION_POINT("init-pre-auth", NULL);
 	}
 
 	/*
diff --git a/src/backend/utils/misc/injection_point.c b/src/backend/utils/misc/injection_point.c
@@ -541,14 +541,14 @@ InjectionPointLoad(const char *name)
  * Execute an injection point, if defined.
  */
 void
-InjectionPointRun(const char *name)
+InjectionPointRun(const char *name, void *arg)
 {
 #ifdef USE_INJECTION_POINTS
 	InjectionPointCacheEntry *cache_entry;
 
 	cache_entry = InjectionPointCacheRefresh(name);
 	if (cache_entry)
-		cache_entry->callback(name, cache_entry->private_data);
+		cache_entry->callback(name, cache_entry->private_data, arg);
 #else
 	elog(ERROR, "Injection points are not supported by this build");
 #endif
@@ -558,14 +558,14 @@ InjectionPointRun(const char *name)
  * Execute an injection point directly from the cache, if defined.
  */
 void
-InjectionPointCached(const char *name)
+InjectionPointCached(const char *name, void *arg)
 {
 #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);
+		cache_entry->callback(name, cache_entry->private_data, arg);
 #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
@@ -16,21 +16,22 @@
  */
 #ifdef USE_INJECTION_POINTS
 #define INJECTION_POINT_LOAD(name) InjectionPointLoad(name)
-#define INJECTION_POINT(name) InjectionPointRun(name)
-#define INJECTION_POINT_CACHED(name) InjectionPointCached(name)
+#define INJECTION_POINT(name, arg) InjectionPointRun(name, arg)
+#define INJECTION_POINT_CACHED(name, arg) InjectionPointCached(name, arg)
 #define IS_INJECTION_POINT_ATTACHED(name) IsInjectionPointAttached(name)
 #else
 #define INJECTION_POINT_LOAD(name) ((void) name)
-#define INJECTION_POINT(name) ((void) name)
-#define INJECTION_POINT_CACHED(name) ((void) name)
+#define INJECTION_POINT(name, arg) ((void) name)
+#define INJECTION_POINT_CACHED(name, arg) ((void) name)
 #define IS_INJECTION_POINT_ATTACHED(name) (false)
 #endif
 
 /*
  * Typedef for callback function launched by an injection point.
  */
 typedef void (*InjectionPointCallback) (const char *name,
-										const void *private_data);
+										const void *private_data,
+										void *arg);
 
 extern Size InjectionPointShmemSize(void);
 extern void InjectionPointShmemInit(void);
@@ -41,8 +42,8 @@ extern void InjectionPointAttach(const char *name,
 								 const void *private_data,
 								 int private_data_size);
 extern void InjectionPointLoad(const char *name);
-extern void InjectionPointRun(const char *name);
-extern void InjectionPointCached(const char *name);
+extern void InjectionPointRun(const char *name, void *arg);
+extern void InjectionPointCached(const char *name, void *arg);
 extern bool IsInjectionPointAttached(const char *name);
 extern bool InjectionPointDetach(const char *name);
 
diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c
diff --git a/src/test/modules/test_aio/test_aio.c b/src/test/modules/test_aio/test_aio.c
diff --git a/src/test/modules/test_slru/test_multixact.c b/src/test/modules/test_slru/test_multixact.c

Original file line number	Diff line number	Diff line change
`@@ -1492,7 +1492,7 @@ build_hash_tables(AggState *aggstate)`
`1492`	`1492`	`if (IS_INJECTION_POINT_ATTACHED("hash-aggregate-oversize-table"))`
`1493`	`1493`	`{`
`1494`	`1494`	`nbuckets = memory / TupleHashEntrySize();`
`1495`		`- INJECTION_POINT_CACHED("hash-aggregate-oversize-table");`
	`1495`	`+ INJECTION_POINT_CACHED("hash-aggregate-oversize-table", NULL);`
`1496`	`1496`	`}`
`1497`	`1497`	`#endif`
`1498`	`1498`
`@@ -1882,7 +1882,7 @@ hash_agg_check_limits(AggState *aggstate)`
`1882`	`1882`	`if (IS_INJECTION_POINT_ATTACHED("hash-aggregate-spill-1000"))`
`1883`	`1883`	`{`
`1884`	`1884`	`do_spill = true;`
`1885`		`- INJECTION_POINT_CACHED("hash-aggregate-spill-1000");`
	`1885`	`+ INJECTION_POINT_CACHED("hash-aggregate-spill-1000", NULL);`
`1886`	`1886`	`}`
`1887`	`1887`	`}`
`1888`	`1888`	`#endif`
`@@ -1910,7 +1910,7 @@ hash_agg_check_limits(AggState *aggstate)`
`1910`	`1910`	`static void`
`1911`	`1911`	`hash_agg_enter_spill_mode(AggState *aggstate)`
`1912`	`1912`	`{`
`1913`		`- INJECTION_POINT("hash-aggregate-enter-spill-mode");`
	`1913`	`+ INJECTION_POINT("hash-aggregate-enter-spill-mode", NULL);`
`1914`	`1914`	`aggstate->hash_spill_mode = true;`
`1915`	`1915`	`hashagg_recompile_expressions(aggstate, aggstate->table_filled, true);`
`1916`	`1916`
`@@ -2739,7 +2739,7 @@ agg_refill_hash_table(AggState *aggstate)`
`2739`	`2739`	`*/`
`2740`	`2740`	`hashagg_recompile_expressions(aggstate, true, true);`
`2741`	`2741`
`2742`		`- INJECTION_POINT("hash-aggregate-process-batch");`
	`2742`	`+ INJECTION_POINT("hash-aggregate-process-batch", NULL);`
`2743`	`2743`	`for (;;)`
`2744`	`2744`	`{`
`2745`	`2745`	`TupleTableSlot *spillslot = aggstate->hash_spill_rslot;`
`@@ -2995,7 +2995,7 @@ hashagg_spill_init(HashAggSpill spill, LogicalTapeSet tapeset, int used_bits,`
`2995`	`2995`	`{`
`2996`	`2996`	`npartitions = 1;`
`2997`	`2997`	`partition_bits = 0;`
`2998`		`- INJECTION_POINT_CACHED("hash-aggregate-single-partition");`
	`2998`	`+ INJECTION_POINT_CACHED("hash-aggregate-single-partition", NULL);`
`2999`	`2999`	`}`
`3000`	`3000`	`#endif`
`3001`	`3001`
Original file line number	Diff line number	Diff line change
`@@ -131,7 +131,7 @@ secure_open_server(Port *port)`
`131`	`131`	`}`
`132`	`132`	`Assert(pq_buffer_remaining_data() == 0);`
`133`	`133`
`134`		`- INJECTION_POINT("backend-ssl-startup");`
	`134`	`+ INJECTION_POINT("backend-ssl-startup", NULL);`
`135`	`135`
`136`	`136`	`r = be_tls_open_server(port);`
`137`	`137`