Skip to content

Commit 4826caa

Browse files
author
Oleg Tselebrovskiy
committed
Save pid and wait_event_info from PGPROC to avoid race condition
When using a pointer to PGPROC structure it can be changed somewhere else and cause a race condition, which leads to an incorrect entry in pg_wait_sampling. Saving pid and wait_event_info at the start guarantees consistent data Reviewed and amended by Sergey Shinderuk
1 parent 8f6f7e6 commit 4826caa

File tree

3 files changed

+16
-12
lines changed

3 files changed

+16
-12
lines changed

Diff for: collector.c

+1-5
Original file line numberDiff line numberDiff line change
@@ -164,13 +164,9 @@ probe_waits(History *observations, HTAB *profile_hash,
164164
*observation;
165165
PGPROC *proc = &ProcGlobal->allProcs[i];
166166

167-
if (!pgws_should_sample_proc(proc))
167+
if (!pgws_should_sample_proc(proc, &item.pid, &item.wait_event_info))
168168
continue;
169169

170-
/* Collect next wait event sample */
171-
item.pid = proc->pid;
172-
item.wait_event_info = proc->wait_event_info;
173-
174170
if (pgws_profileQueries)
175171
item.queryId = pgws_proc_queryids[i];
176172
else

Diff for: pg_wait_sampling.c

+12-4
Original file line numberDiff line numberDiff line change
@@ -451,9 +451,15 @@ search_proc(int pid)
451451
* views.
452452
*/
453453
bool
454-
pgws_should_sample_proc(PGPROC *proc)
454+
pgws_should_sample_proc(PGPROC *proc, int *pid_p, uint32 *wait_event_info_p)
455455
{
456-
if (proc->wait_event_info == 0 && !pgws_sampleCpu)
456+
int pid = proc->pid;
457+
uint32 wait_event_info = proc->wait_event_info;
458+
459+
*pid_p = pid;
460+
*wait_event_info_p = wait_event_info;
461+
462+
if (wait_event_info == 0 && !pgws_sampleCpu)
457463
return false;
458464

459465
/*
@@ -462,7 +468,7 @@ pgws_should_sample_proc(PGPROC *proc)
462468
* null wait events. So instead we make use of DisownLatch() resetting
463469
* owner_pid during ProcKill().
464470
*/
465-
if (proc->pid == 0 || proc->procLatch.owner_pid == 0 || proc->pid == MyProcPid)
471+
if (pid == 0 || proc->procLatch.owner_pid == 0 || pid == MyProcPid)
466472
return false;
467473

468474
return true;
@@ -533,7 +539,9 @@ pg_wait_sampling_get_current(PG_FUNCTION_ARGS)
533539
{
534540
PGPROC *proc = &ProcGlobal->allProcs[i];
535541

536-
if (!pgws_should_sample_proc(proc))
542+
if (!pgws_should_sample_proc(proc,
543+
&params->items[j].pid,
544+
&params->items[j].wait_event_info))
537545
continue;
538546

539547
params->items[j].pid = proc->pid;

Diff for: pg_wait_sampling.h

+3-3
Original file line numberDiff line numberDiff line change
@@ -24,15 +24,15 @@
2424

2525
typedef struct
2626
{
27-
uint32 pid;
27+
int pid;
2828
uint32 wait_event_info;
2929
uint64 queryId;
3030
uint64 count;
3131
} ProfileItem;
3232

3333
typedef struct
3434
{
35-
uint32 pid;
35+
int pid;
3636
uint32 wait_event_info;
3737
uint64 queryId;
3838
TimestampTz ts;
@@ -73,7 +73,7 @@ extern CollectorShmqHeader *pgws_collector_hdr;
7373
extern shm_mq *pgws_collector_mq;
7474
extern uint64 *pgws_proc_queryids;
7575
extern void pgws_init_lock_tag(LOCKTAG *tag, uint32 lock);
76-
extern bool pgws_should_sample_proc(PGPROC *proc);
76+
extern bool pgws_should_sample_proc(PGPROC *proc, int *pid_p, uint32 *wait_event_info_p);
7777

7878
/* collector.c */
7979
extern void pgws_register_wait_collector(void);

0 commit comments

Comments
 (0)