v2-0001-Fix-race-condition-in-backend-exit-after-injectio.patch

From 9cf38b2f6dadd5b4bb81fe5ccea350d259e6a241 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Tue, 7 May 2024 10:15:28 +0900
Subject: [PATCH v2] Fix race condition in backend exit after
 injection_points_set_local().

Symptoms were like those prompting commit
f587338dec87d3c35b025e131c5977930ac69077.  That is, under installcheck,
backends from unrelated tests could run the injection points.

Reviewed by FIXME.

Discussion: https://postgr.es/m/FIXME
---
 .../injection_points/injection_points.c       | 27 ++++++++++++++-----
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c
index a74a4a28af..8c9a3ebd74 100644
--- a/src/test/modules/injection_points/injection_points.c
+++ b/src/test/modules/injection_points/injection_points.c
@@ -37,7 +37,13 @@ PG_MODULE_MAGIC;
 
 /*
  * Conditions related to injection points.  This tracks in shared memory the
- * runtime conditions under which an injection point is allowed to run.
+ * runtime conditions under which an injection point is allowed to run.  Once
+ * set, a name is never changed.  That avoids a race condition:
+ *
+ *	s1: local-attach to POINT
+ *	s2: yield CPU before InjectionPointRun(POINT) calls injection_callback()
+ *	s1: exit()
+ *	s2: run POINT as though it had been non-local
  *
  * If more types of runtime conditions need to be tracked, this structure
  * should be expanded.
@@ -49,6 +55,9 @@ typedef struct InjectionPointCondition
 
 	/* ID of the process where the injection point is allowed to run */
 	int			pid;
+
+	/* Should "pid" run this injection point, or not? */
+	bool		valid;
 } InjectionPointCondition;
 
 /* Shared state information for injection points. */
@@ -133,7 +142,7 @@ injection_point_allowed(const char *name)
 	{
 		InjectionPointCondition *condition = &inj_state->conditions[i];
 
-		if (strcmp(condition->name, name) == 0)
+		if (condition->valid && strcmp(condition->name, name) == 0)
 		{
 			/*
 			 * Check if this injection point is allowed to run in this
@@ -175,9 +184,11 @@ injection_points_cleanup(int code, Datum arg)
 	{
 		InjectionPointCondition *condition = &inj_state->conditions[i];
 
-		if (condition->name[0] == '\0')
+		if (!condition->valid)
 			continue;
 
+		Assert(condition->name[0] != '\0');
+
 		if (condition->pid != MyProcPid)
 			continue;
 
@@ -197,14 +208,14 @@ injection_points_cleanup(int code, Datum arg)
 	{
 		InjectionPointCondition *condition = &inj_state->conditions[i];
 
-		if (condition->name[0] == '\0')
+		if (condition->valid)
 			continue;
 
 		if (condition->pid != MyProcPid)
 			continue;
 
-		condition->name[0] = '\0';
 		condition->pid = 0;
+		condition->valid = false;
 	}
 	SpinLockRelease(&inj_state->lock);
 }
@@ -326,11 +337,13 @@ injection_points_attach(PG_FUNCTION_ARGS)
 		{
 			InjectionPointCondition *condition = &inj_state->conditions[i];
 
-			if (condition->name[0] == '\0')
+			if (strcmp(condition->name, name) == 0 ||
+				condition->name[0] == '\0')
 			{
 				index = i;
 				strlcpy(condition->name, name, INJ_NAME_MAXLEN);
 				condition->pid = MyProcPid;
+				condition->valid = true;
 				break;
 			}
 		}
@@ -444,7 +457,7 @@ injection_points_detach(PG_FUNCTION_ARGS)
 		if (strcmp(condition->name, name) == 0)
 		{
 			condition->pid = 0;
-			condition->name[0] = '\0';
+			condition->valid = false;
 		}
 	}
 	SpinLockRelease(&inj_state->lock);
-- 
2.43.0