diff options
| author | Andres Freund <andres@anarazel.de> | 2022-08-22 20:16:50 -0700 |
|---|---|---|
| committer | Andres Freund <andres@anarazel.de> | 2022-08-22 20:16:50 -0700 |
| commit | cd063344fb801a90a40923a5b8aefe4eb8d80762 (patch) | |
| tree | a9248822b82059ddb2cd7b9f36856d6a1b180f81 | |
| parent | ba8321349bc02423aa4d49ebb5d579ec313cf930 (diff) | |
| download | postgresql-cd063344fb801a90a40923a5b8aefe4eb8d80762.tar.gz postgresql-cd063344fb801a90a40923a5b8aefe4eb8d80762.zip | |
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-
| -rw-r--r-- | src/backend/utils/activity/pgstat.c | 9 | ||||
| -rw-r--r-- | src/backend/utils/activity/pgstat_shmem.c | 16 | ||||
| -rw-r--r-- | src/include/utils/pgstat_internal.h | 1 |
3 files changed, 26 insertions, 0 deletions
diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c index 88e5dd1b2b7..6224c498c21 100644 --- a/src/backend/utils/activity/pgstat.c +++ b/src/backend/utils/activity/pgstat.c @@ -844,9 +844,12 @@ pgstat_fetch_entry(PgStat_Kind kind, Oid dboid, Oid objoid) else stats_data = MemoryContextAlloc(pgStatLocal.snapshot.context, kind_info->shared_data_len); + + pgstat_lock_entry_shared(entry_ref, false); memcpy(stats_data, pgstat_get_entry_data(kind, entry_ref->shared_stats), kind_info->shared_data_len); + pgstat_unlock_entry(entry_ref); if (pgstat_fetch_consistency > PGSTAT_FETCH_CONSISTENCY_NONE) { @@ -983,9 +986,15 @@ pgstat_build_snapshot(void) entry->data = MemoryContextAlloc(pgStatLocal.snapshot.context, kind_info->shared_size); + /* + * Acquire the LWLock directly instead of using + * pg_stat_lock_entry_shared() which requires a reference. + */ + LWLockAcquire(&stats_data->lock, LW_SHARED); memcpy(entry->data, pgstat_get_entry_data(kind, stats_data), kind_info->shared_size); + LWLockRelease(&stats_data->lock); } dshash_seq_term(&hstat); diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/activity/pgstat_shmem.c index 89060ef29a0..ac989186884 100644 --- a/src/backend/utils/activity/pgstat_shmem.c +++ b/src/backend/utils/activity/pgstat_shmem.c @@ -579,6 +579,22 @@ pgstat_lock_entry(PgStat_EntryRef *entry_ref, bool nowait) return true; } +/* + * Separate from pgstat_lock_entry() as most callers will need to lock + * exclusively. + */ +bool +pgstat_lock_entry_shared(PgStat_EntryRef *entry_ref, bool nowait) +{ + LWLock *lock = &entry_ref->shared_stats->lock; + + if (nowait) + return LWLockConditionalAcquire(lock, LW_SHARED); + + LWLockAcquire(lock, LW_SHARED); + return true; +} + void pgstat_unlock_entry(PgStat_EntryRef *entry_ref) { diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h index 9303d05427f..901d2041d66 100644 --- a/src/include/utils/pgstat_internal.h +++ b/src/include/utils/pgstat_internal.h @@ -581,6 +581,7 @@ extern void pgstat_detach_shmem(void); extern PgStat_EntryRef *pgstat_get_entry_ref(PgStat_Kind kind, Oid dboid, Oid objoid, bool create, bool *found); extern bool pgstat_lock_entry(PgStat_EntryRef *entry_ref, bool nowait); +extern bool pgstat_lock_entry_shared(PgStat_EntryRef *entry_ref, bool nowait); extern void pgstat_unlock_entry(PgStat_EntryRef *entry_ref); extern bool pgstat_drop_entry(PgStat_Kind kind, Oid dboid, Oid objoid); extern void pgstat_drop_all_entries(void); |