inplace030-inj-exit-race-v1.patch
text/plain
Patch
Format: unified
Series: patch v1
| File | + | − |
|---|---|---|
| src/test/modules/injection_points/injection_points.c | 18 | 6 |
Author: Noah Misch <noah@leadboat.com>
Commit: Noah Misch <noah@leadboat.com>
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
diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c
index ace386d..75466d3 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
@@ -168,15 +177,16 @@ 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;
/* Detach the injection point and unregister condition */
InjectionPointDetach(condition->name);
- condition->name[0] = '\0';
+ condition->valid = false;
condition->pid = 0;
}
SpinLockRelease(&inj_state->lock);
@@ -299,11 +309,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;
}
}
@@ -416,8 +428,8 @@ injection_points_detach(PG_FUNCTION_ARGS)
if (strcmp(condition->name, name) == 0)
{
+ condition->valid = false;
condition->pid = 0;
- condition->name[0] = '\0';
}
}
SpinLockRelease(&inj_state->lock);