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

Commit cd06334

Browse files
committed
pgstat: Acquire lock when reading variable-numbered stats
Somewhere during the development of the patch acquiring a lock during read access to variable-numbered stats got lost. The missing lock acquisition won't cause corruption, but can lead to reading torn values when accessing stats. Add the missing lock acquisitions. Reported-by: Greg Stark <stark@mit.edu> Reviewed-by: "Drouvot, Bertrand" <bdrouvot@amazon.com> Reviewed-by: Andres Freund <andres@anarazel.de> Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Discussion: https://postgr.es/m/CAM-w4HMYkM_DkYhWtUGV+qE_rrBxKOzOF0+5faozxO3vXrc9wA@mail.gmail.com Backpatch: 15-
1 parent ba83213 commit cd06334

File tree

3 files changed

+26
-0
lines changed

3 files changed

+26
-0
lines changed

src/backend/utils/activity/pgstat.c

+9
Original file line numberDiff line numberDiff line change
@@ -844,9 +844,12 @@ pgstat_fetch_entry(PgStat_Kind kind, Oid dboid, Oid objoid)
844844
else
845845
stats_data = MemoryContextAlloc(pgStatLocal.snapshot.context,
846846
kind_info->shared_data_len);
847+
848+
pgstat_lock_entry_shared(entry_ref, false);
847849
memcpy(stats_data,
848850
pgstat_get_entry_data(kind, entry_ref->shared_stats),
849851
kind_info->shared_data_len);
852+
pgstat_unlock_entry(entry_ref);
850853

851854
if (pgstat_fetch_consistency > PGSTAT_FETCH_CONSISTENCY_NONE)
852855
{
@@ -983,9 +986,15 @@ pgstat_build_snapshot(void)
983986

984987
entry->data = MemoryContextAlloc(pgStatLocal.snapshot.context,
985988
kind_info->shared_size);
989+
/*
990+
* Acquire the LWLock directly instead of using
991+
* pg_stat_lock_entry_shared() which requires a reference.
992+
*/
993+
LWLockAcquire(&stats_data->lock, LW_SHARED);
986994
memcpy(entry->data,
987995
pgstat_get_entry_data(kind, stats_data),
988996
kind_info->shared_size);
997+
LWLockRelease(&stats_data->lock);
989998
}
990999
dshash_seq_term(&hstat);
9911000

src/backend/utils/activity/pgstat_shmem.c

+16
Original file line numberDiff line numberDiff line change
@@ -579,6 +579,22 @@ pgstat_lock_entry(PgStat_EntryRef *entry_ref, bool nowait)
579579
return true;
580580
}
581581

582+
/*
583+
* Separate from pgstat_lock_entry() as most callers will need to lock
584+
* exclusively.
585+
*/
586+
bool
587+
pgstat_lock_entry_shared(PgStat_EntryRef *entry_ref, bool nowait)
588+
{
589+
LWLock *lock = &entry_ref->shared_stats->lock;
590+
591+
if (nowait)
592+
return LWLockConditionalAcquire(lock, LW_SHARED);
593+
594+
LWLockAcquire(lock, LW_SHARED);
595+
return true;
596+
}
597+
582598
void
583599
pgstat_unlock_entry(PgStat_EntryRef *entry_ref)
584600
{

src/include/utils/pgstat_internal.h

+1
Original file line numberDiff line numberDiff line change
@@ -581,6 +581,7 @@ extern void pgstat_detach_shmem(void);
581581
extern PgStat_EntryRef *pgstat_get_entry_ref(PgStat_Kind kind, Oid dboid, Oid objoid,
582582
bool create, bool *found);
583583
extern bool pgstat_lock_entry(PgStat_EntryRef *entry_ref, bool nowait);
584+
extern bool pgstat_lock_entry_shared(PgStat_EntryRef *entry_ref, bool nowait);
584585
extern void pgstat_unlock_entry(PgStat_EntryRef *entry_ref);
585586
extern bool pgstat_drop_entry(PgStat_Kind kind, Oid dboid, Oid objoid);
586587
extern void pgstat_drop_all_entries(void);

0 commit comments

Comments
 (0)