Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 597f669

Browse files
committed
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 f587338. Reported-by: Noah Misch Reviewed-by: Noah Misch Discussion: https://postgr.es/m/20240501231214.40@rfd.leadboat.com
1 parent 713cfaf commit 597f669

File tree

1 file changed

+29
-2
lines changed

1 file changed

+29
-2
lines changed

src/test/modules/injection_points/injection_points.c

+29-2
Original file line numberDiff line numberDiff line change
@@ -159,10 +159,39 @@ injection_point_allowed(const char *name)
159159
static void
160160
injection_points_cleanup(int code, Datum arg)
161161
{
162+
char names[INJ_MAX_CONDITION][INJ_NAME_MAXLEN] = {0};
163+
int count = 0;
164+
162165
/* Leave if nothing is tracked locally */
163166
if (!injection_point_local)
164167
return;
165168

169+
/*
170+
* This is done in three steps: detect the points to detach, detach them
171+
* and release their conditions.
172+
*/
173+
SpinLockAcquire(&inj_state->lock);
174+
for (int i = 0; i < INJ_MAX_CONDITION; i++)
175+
{
176+
InjectionPointCondition *condition = &inj_state->conditions[i];
177+
178+
if (condition->name[0] == '\0')
179+
continue;
180+
181+
if (condition->pid != MyProcPid)
182+
continue;
183+
184+
/* Extract the point name to detach */
185+
strlcpy(names[count], condition->name, INJ_NAME_MAXLEN);
186+
count++;
187+
}
188+
SpinLockRelease(&inj_state->lock);
189+
190+
/* Detach, without holding the spinlock */
191+
for (int i = 0; i < count; i++)
192+
InjectionPointDetach(names[i]);
193+
194+
/* Clear all the conditions */
166195
SpinLockAcquire(&inj_state->lock);
167196
for (int i = 0; i < INJ_MAX_CONDITION; i++)
168197
{
@@ -174,8 +203,6 @@ injection_points_cleanup(int code, Datum arg)
174203
if (condition->pid != MyProcPid)
175204
continue;
176205

177-
/* Detach the injection point and unregister condition */
178-
InjectionPointDetach(condition->name);
179206
condition->name[0] = '\0';
180207
condition->pid = 0;
181208
}

0 commit comments

Comments
 (0)