Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit267d41d

Browse files
committed
injection_points: Store runtime conditions in private area
This commit fixes a race condition between injection point run anddetach, where a point detached by a backend and concurrently running ina second backend could cause the second backend to do an incorrectcondition check. This issue happens because the second backendretrieves the callback information in a first step in the shmem hashtable for injection points, and the condition in a second step withinthe callback. If the point is detached between these two steps, thecondition would be removed, causing the point to run while it shouldnot. Storing the condition in the new private_data area introduced in33181b4 ensures that the condition retrieved is consistent with itscallback.This commit leads to a lot of simplifications in the moduleinjection_points, as there is no need to handle the runtime conditionsinside it anymore. Runtime conditions have no more a maximum number.Per discussion with Noah Misch.Reviewed-by: Noah MischDiscussion:https://postgr.es/m/20240509031553.47@rfd.leadboat.com
1 parent33181b4 commit267d41d

File tree

2 files changed

+61
-113
lines changed

2 files changed

+61
-113
lines changed

‎src/test/modules/injection_points/injection_points.c

Lines changed: 60 additions & 113 deletions
Original file line numberDiff line numberDiff line change
@@ -19,38 +19,54 @@
1919

2020
#include"fmgr.h"
2121
#include"miscadmin.h"
22+
#include"nodes/pg_list.h"
23+
#include"nodes/value.h"
2224
#include"storage/condition_variable.h"
2325
#include"storage/dsm_registry.h"
2426
#include"storage/ipc.h"
2527
#include"storage/lwlock.h"
2628
#include"storage/shmem.h"
2729
#include"utils/builtins.h"
2830
#include"utils/injection_point.h"
31+
#include"utils/memutils.h"
2932
#include"utils/wait_event.h"
3033

3134
PG_MODULE_MAGIC;
3235

3336
/* Maximum number of waits usable in injection points at once */
3437
#defineINJ_MAX_WAIT8
3538
#defineINJ_NAME_MAXLEN64
36-
#defineINJ_MAX_CONDITION4
3739

3840
/*
3941
* Conditions related to injection points. This tracks in shared memory the
40-
* runtime conditions under which an injection point is allowed to run.
42+
* runtime conditions under which an injection point is allowed to run,
43+
* stored as private_data when an injection point is attached, and passed as
44+
* argument to the callback.
4145
*
4246
* If more types of runtime conditions need to be tracked, this structure
4347
* should be expanded.
4448
*/
49+
typedefenumInjectionPointConditionType
50+
{
51+
INJ_CONDITION_ALWAYS=0,/* always run */
52+
INJ_CONDITION_PID,/* PID restriction */
53+
}InjectionPointConditionType;
54+
4555
typedefstructInjectionPointCondition
4656
{
47-
/*Name of the injection point related to this condition */
48-
charname[INJ_NAME_MAXLEN];
57+
/*Type of the condition */
58+
InjectionPointConditionTypetype;
4959

5060
/* ID of the process where the injection point is allowed to run */
5161
intpid;
5262
}InjectionPointCondition;
5363

64+
/*
65+
* List of injection points stored in TopMemoryContext attached
66+
* locally to this process.
67+
*/
68+
staticList*inj_list_local=NIL;
69+
5470
/* Shared state information for injection points. */
5571
typedefstructInjectionPointSharedState
5672
{
@@ -65,9 +81,6 @@ typedef struct InjectionPointSharedState
6581

6682
/* Condition variable used for waits and wakeups */
6783
ConditionVariablewait_point;
68-
69-
/* Conditions to run an injection point */
70-
InjectionPointConditionconditions[INJ_MAX_CONDITION];
7184
}InjectionPointSharedState;
7285

7386
/* Pointer to shared-memory state. */
@@ -94,7 +107,6 @@ injection_point_init_state(void *ptr)
94107
SpinLockInit(&state->lock);
95108
memset(state->wait_counts,0,sizeof(state->wait_counts));
96109
memset(state->name,0,sizeof(state->name));
97-
memset(state->conditions,0,sizeof(state->conditions));
98110
ConditionVariableInit(&state->wait_point);
99111
}
100112

@@ -119,39 +131,23 @@ injection_init_shmem(void)
119131
* Check runtime conditions associated to an injection point.
120132
*
121133
* Returns true if the named injection point is allowed to run, and false
122-
* otherwise. Multiple conditions can be associated to a single injection
123-
* point, so check them all.
134+
* otherwise.
124135
*/
125136
staticbool
126-
injection_point_allowed(constchar*name)
137+
injection_point_allowed(InjectionPointCondition*condition)
127138
{
128139
boolresult= true;
129140

130-
if (inj_state==NULL)
131-
injection_init_shmem();
132-
133-
SpinLockAcquire(&inj_state->lock);
134-
135-
for (inti=0;i<INJ_MAX_CONDITION;i++)
141+
switch (condition->type)
136142
{
137-
InjectionPointCondition*condition=&inj_state->conditions[i];
138-
139-
if (strcmp(condition->name,name)==0)
140-
{
141-
/*
142-
* Check if this injection point is allowed to run in this
143-
* process.
144-
*/
143+
caseINJ_CONDITION_PID:
145144
if (MyProcPid!=condition->pid)
146-
{
147145
result= false;
148-
break;
149-
}
150-
}
146+
break;
147+
caseINJ_CONDITION_ALWAYS:
148+
break;
151149
}
152150

153-
SpinLockRelease(&inj_state->lock);
154-
155151
returnresult;
156152
}
157153

@@ -162,61 +158,28 @@ injection_point_allowed(const char *name)
162158
staticvoid
163159
injection_points_cleanup(intcode,Datumarg)
164160
{
165-
charnames[INJ_MAX_CONDITION][INJ_NAME_MAXLEN]= {0};
166-
intcount=0;
161+
ListCell*lc;
167162

168163
/* Leave if nothing is tracked locally */
169164
if (!injection_point_local)
170165
return;
171166

172-
/*
173-
* This is done in three steps: detect the points to detach, detach them
174-
* and release their conditions.
175-
*/
176-
SpinLockAcquire(&inj_state->lock);
177-
for (inti=0;i<INJ_MAX_CONDITION;i++)
178-
{
179-
InjectionPointCondition*condition=&inj_state->conditions[i];
180-
181-
if (condition->name[0]=='\0')
182-
continue;
183-
184-
if (condition->pid!=MyProcPid)
185-
continue;
186-
187-
/* Extract the point name to detach */
188-
strlcpy(names[count],condition->name,INJ_NAME_MAXLEN);
189-
count++;
190-
}
191-
SpinLockRelease(&inj_state->lock);
192-
193-
/* Detach, without holding the spinlock */
194-
for (inti=0;i<count;i++)
195-
(void)InjectionPointDetach(names[i]);
196-
197-
/* Clear all the conditions */
198-
SpinLockAcquire(&inj_state->lock);
199-
for (inti=0;i<INJ_MAX_CONDITION;i++)
167+
/* Detach all the local points */
168+
foreach(lc,inj_list_local)
200169
{
201-
InjectionPointCondition*condition=&inj_state->conditions[i];
170+
char*name=strVal(lfirst(lc));
202171

203-
if (condition->name[0]=='\0')
204-
continue;
205-
206-
if (condition->pid!=MyProcPid)
207-
continue;
208-
209-
condition->name[0]='\0';
210-
condition->pid=0;
172+
(void)InjectionPointDetach(name);
211173
}
212-
SpinLockRelease(&inj_state->lock);
213174
}
214175

215176
/* Set of callbacks available to be attached to an injection point. */
216177
void
217178
injection_error(constchar*name,constvoid*private_data)
218179
{
219-
if (!injection_point_allowed(name))
180+
InjectionPointCondition*condition= (InjectionPointCondition*)private_data;
181+
182+
if (!injection_point_allowed(condition))
220183
return;
221184

222185
elog(ERROR,"error triggered for injection point %s",name);
@@ -225,7 +188,9 @@ injection_error(const char *name, const void *private_data)
225188
void
226189
injection_notice(constchar*name,constvoid*private_data)
227190
{
228-
if (!injection_point_allowed(name))
191+
InjectionPointCondition*condition= (InjectionPointCondition*)private_data;
192+
193+
if (!injection_point_allowed(condition))
229194
return;
230195

231196
elog(NOTICE,"notice triggered for injection point %s",name);
@@ -238,11 +203,12 @@ injection_wait(const char *name, const void *private_data)
238203
uint32old_wait_counts=0;
239204
intindex=-1;
240205
uint32injection_wait_event=0;
206+
InjectionPointCondition*condition= (InjectionPointCondition*)private_data;
241207

242208
if (inj_state==NULL)
243209
injection_init_shmem();
244210

245-
if (!injection_point_allowed(name))
211+
if (!injection_point_allowed(condition))
246212
return;
247213

248214
/*
@@ -304,6 +270,7 @@ injection_points_attach(PG_FUNCTION_ARGS)
304270
char*name=text_to_cstring(PG_GETARG_TEXT_PP(0));
305271
char*action=text_to_cstring(PG_GETARG_TEXT_PP(1));
306272
char*function;
273+
InjectionPointConditioncondition= {0};
307274

308275
if (strcmp(action,"error")==0)
309276
function="injection_error";
@@ -314,37 +281,24 @@ injection_points_attach(PG_FUNCTION_ARGS)
314281
else
315282
elog(ERROR,"incorrect action \"%s\" for injection point creation",action);
316283

317-
InjectionPointAttach(name,"injection_points",function,NULL,0);
318-
319284
if (injection_point_local)
320285
{
321-
intindex=-1;
286+
condition.type=INJ_CONDITION_PID;
287+
condition.pid=MyProcPid;
288+
}
322289

323-
/*
324-
* Register runtime condition to link this injection point to the
325-
* current process.
326-
*/
327-
SpinLockAcquire(&inj_state->lock);
328-
for (inti=0;i<INJ_MAX_CONDITION;i++)
329-
{
330-
InjectionPointCondition*condition=&inj_state->conditions[i];
331-
332-
if (condition->name[0]=='\0')
333-
{
334-
index=i;
335-
strlcpy(condition->name,name,INJ_NAME_MAXLEN);
336-
condition->pid=MyProcPid;
337-
break;
338-
}
339-
}
340-
SpinLockRelease(&inj_state->lock);
290+
InjectionPointAttach(name,"injection_points",function,&condition,
291+
sizeof(InjectionPointCondition));
341292

342-
if (index<0)
343-
elog(FATAL,
344-
"could not find free slot for condition of injection point %s",
345-
name);
346-
}
293+
if (injection_point_local)
294+
{
295+
MemoryContextoldctx;
347296

297+
/* Local injection point, so track it for automated cleanup */
298+
oldctx=MemoryContextSwitchTo(TopMemoryContext);
299+
inj_list_local=lappend(inj_list_local,makeString(pstrdup(name)));
300+
MemoryContextSwitchTo(oldctx);
301+
}
348302
PG_RETURN_VOID();
349303
}
350304

@@ -436,22 +390,15 @@ injection_points_detach(PG_FUNCTION_ARGS)
436390
if (!InjectionPointDetach(name))
437391
elog(ERROR,"could not detach injection point \"%s\"",name);
438392

439-
if (inj_state==NULL)
440-
injection_init_shmem();
441-
442-
/* Clean up any conditions associated to this injection point */
443-
SpinLockAcquire(&inj_state->lock);
444-
for (inti=0;i<INJ_MAX_CONDITION;i++)
393+
/* Remove point from local list, if required */
394+
if (inj_list_local!=NIL)
445395
{
446-
InjectionPointCondition*condition=&inj_state->conditions[i];
396+
MemoryContextoldctx;
447397

448-
if (strcmp(condition->name,name)==0)
449-
{
450-
condition->pid=0;
451-
condition->name[0]='\0';
452-
}
398+
oldctx=MemoryContextSwitchTo(TopMemoryContext);
399+
inj_list_local=list_delete(inj_list_local,makeString(name));
400+
MemoryContextSwitchTo(oldctx);
453401
}
454-
SpinLockRelease(&inj_state->lock);
455402

456403
PG_RETURN_VOID();
457404
}

‎src/tools/pgindent/typedefs.list

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1219,6 +1219,7 @@ InitializeDSMForeignScan_function
12191219
InitializeWorkerForeignScan_function
12201220
InjectionPointCacheEntry
12211221
InjectionPointCondition
1222+
InjectionPointConditionType
12221223
InjectionPointEntry
12231224
InjectionPointSharedState
12241225
InlineCodeBlock

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp