Author: Noah Misch Commit: Noah Misch 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);