postgres.git / summary / log / commit / refs

commit    597f66942d0b18f2f15b478346bc74ae6fdb793b
Author:   Michael Paquier <michael@paquier.xyz>
Date:     Mon May 06 00:45:46 2024 +0000
First released in: 17.0

    injection_points: Fix incorrect spinlock acquisition
    
    Injection points created under injection_points_set_local() are cleaned
    up by a shmem_exit() callback.  The spinlock used by the module would
    be hold while calling InjectionPointDetach(), which is incorrect as
    spinlocks should avoid external calls while hold.
    
    This commit changes the shmem_exit() callback to detach the points in
    three steps with the spinlock acquired twice, knowing that the
    injection points should be around with the conditions related to them:
    - Scans for the points to detach in a first loop, while holding the
    spinlock.
    - Detach them.
    - Remove the registered conditions.
    
    It is still possible for other processes to detach local points
    concurrently of the callback.  I have wanted to restrict the detach, but
    Noah has mentioned that he has in mind some cases that may require this
    capability.  No tests in the tree based on injection points need that
    currently.
    
    Thinko in f587338dec87.
    
    Reported-by: Noah Misch
    Reviewed-by: Noah Misch
    Discussion: https://postgr.es/m/20240501231214.40@rfd.leadboat.com


 .../modules/injection_points/injection_points.c    | 31 ++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c
index ace386da50b..a74a4a28afd 100644
--- a/src/test/modules/injection_points/injection_points.c
+++ b/src/test/modules/injection_points/injection_points.c
@@ -159,10 +159,39 @@ injection_point_allowed(const char *name)
 static void
 injection_points_cleanup(int code, Datum arg)
 {
+	char		names[INJ_MAX_CONDITION][INJ_NAME_MAXLEN] = {0};
+	int			count = 0;
+
 	/* Leave if nothing is tracked locally */
 	if (!injection_point_local)
 		return;
 
+	/*
+	 * This is done in three steps: detect the points to detach, detach them
+	 * and release their conditions.
+	 */
+	SpinLockAcquire(&inj_state->lock);
+	for (int i = 0; i < INJ_MAX_CONDITION; i++)
+	{
+		InjectionPointCondition *condition = &inj_state->conditions[i];
+
+		if (condition->name[0] == '\0')
+			continue;
+
+		if (condition->pid != MyProcPid)
+			continue;
+
+		/* Extract the point name to detach */
+		strlcpy(names[count], condition->name, INJ_NAME_MAXLEN);
+		count++;
+	}
+	SpinLockRelease(&inj_state->lock);
+
+	/* Detach, without holding the spinlock */
+	for (int i = 0; i < count; i++)
+		InjectionPointDetach(names[i]);
+
+	/* Clear all the conditions */
 	SpinLockAcquire(&inj_state->lock);
 	for (int i = 0; i < INJ_MAX_CONDITION; i++)
 	{
@@ -174,8 +203,6 @@ injection_points_cleanup(int code, Datum arg)
 		if (condition->pid != MyProcPid)
 			continue;
 
-		/* Detach the injection point and unregister condition */
-		InjectionPointDetach(condition->name);
 		condition->name[0] = '\0';
 		condition->pid = 0;
 	}


[parent: 713cfaf2a576]