From d9dbf585e51274da0a71e0d06460e2330822bd86 Mon Sep 17 00:00:00 2001 From: Sami Imseih Date: Wed, 8 Apr 2026 23:36:07 +0000 Subject: [PATCH v2 1/1] Fix double-free on relation_needs_vacanalyze relation_needs_vacanalyze() unconditionally calls pfree() on the tabentry returned by pgstat_fetch_stat_tabentry_ext(). This is correct for autovacuum, which uses PGSTAT_FETCH_CONSISTENCY_NONE where the caller owns the allocation, but causes a double-free for callers using PGSTAT_FETCH_CONSISTENCY_CACHE where the entry is managed by the snapshot cache. Fix this by adding a bool *will_free output parameter to pgstat_fetch_entry() and propagating it through pgstat_fetch_stat_tabentry_ext(). This lets callers determine whether they are responsible for freeing the returned entry. Callers that don't need this information pass NULL, keeping existing call sites minimal while allowing future callers to make use of it as needed. --- src/backend/postmaster/autovacuum.c | 6 ++++-- src/backend/utils/activity/pgstat.c | 9 ++++++++- src/backend/utils/activity/pgstat_backend.c | 2 +- src/backend/utils/activity/pgstat_database.c | 2 +- src/backend/utils/activity/pgstat_function.c | 2 +- src/backend/utils/activity/pgstat_relation.c | 8 ++++---- src/backend/utils/activity/pgstat_replslot.c | 2 +- src/backend/utils/activity/pgstat_subscription.c | 2 +- src/include/pgstat.h | 3 ++- src/include/utils/pgstat_internal.h | 3 ++- .../modules/test_custom_stats/test_custom_var_stats.c | 2 +- 11 files changed, 26 insertions(+), 15 deletions(-) diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index bd626a16363..3c1fed6264d 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -3080,6 +3080,7 @@ relation_needs_vacanalyze(Oid relid, PgStat_StatTabEntry *tabentry; bool force_vacuum; bool av_enabled; + bool will_free; /* constants from reloptions or GUC variables */ int vac_base_thresh, @@ -3246,7 +3247,7 @@ relation_needs_vacanalyze(Oid relid, * forced. */ tabentry = pgstat_fetch_stat_tabentry_ext(classForm->relisshared, - relid); + relid, &will_free); if (!tabentry) return; @@ -3327,7 +3328,8 @@ relation_needs_vacanalyze(Oid relid, anltuples, anlthresh, scores->anl, scores->xid, scores->mxid); - pfree(tabentry); + if (will_free) + pfree(tabentry); } /* diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c index eb8ccbaa628..a0561250710 100644 --- a/src/backend/utils/activity/pgstat.c +++ b/src/backend/utils/activity/pgstat.c @@ -960,7 +960,7 @@ pgstat_clear_snapshot(void) } void * -pgstat_fetch_entry(PgStat_Kind kind, Oid dboid, uint64 objid) +pgstat_fetch_entry(PgStat_Kind kind, Oid dboid, uint64 objid, bool *will_free) { PgStat_HashKey key = {0}; PgStat_EntryRef *entry_ref; @@ -971,6 +971,13 @@ pgstat_fetch_entry(PgStat_Kind kind, Oid dboid, uint64 objid) Assert(IsUnderPostmaster || !IsPostmasterEnvironment); Assert(!kind_info->fixed_amount); + /* + * When pgstat_fetch_consistency is PGSTAT_FETCH_CONSISTENCY_NONE, callers + * will be responsible for freeing the entry. + */ + if (will_free) + *will_free = (pgstat_fetch_consistency == PGSTAT_FETCH_CONSISTENCY_NONE); + pgstat_prep_snapshot(); key.kind = kind; diff --git a/src/backend/utils/activity/pgstat_backend.c b/src/backend/utils/activity/pgstat_backend.c index 04fe13e64c6..6ffef1040a2 100644 --- a/src/backend/utils/activity/pgstat_backend.c +++ b/src/backend/utils/activity/pgstat_backend.c @@ -95,7 +95,7 @@ pgstat_fetch_stat_backend(ProcNumber procNumber) PgStat_Backend *backend_entry; backend_entry = (PgStat_Backend *) pgstat_fetch_entry(PGSTAT_KIND_BACKEND, - InvalidOid, procNumber); + InvalidOid, procNumber, NULL); return backend_entry; } diff --git a/src/backend/utils/activity/pgstat_database.c b/src/backend/utils/activity/pgstat_database.c index 933dcb5cae5..f1846d3236c 100644 --- a/src/backend/utils/activity/pgstat_database.c +++ b/src/backend/utils/activity/pgstat_database.c @@ -288,7 +288,7 @@ PgStat_StatDBEntry * pgstat_fetch_stat_dbentry(Oid dboid) { return (PgStat_StatDBEntry *) - pgstat_fetch_entry(PGSTAT_KIND_DATABASE, dboid, InvalidOid); + pgstat_fetch_entry(PGSTAT_KIND_DATABASE, dboid, InvalidOid, NULL); } void diff --git a/src/backend/utils/activity/pgstat_function.c b/src/backend/utils/activity/pgstat_function.c index e6b84283c6c..d47d05e3d92 100644 --- a/src/backend/utils/activity/pgstat_function.c +++ b/src/backend/utils/activity/pgstat_function.c @@ -245,5 +245,5 @@ PgStat_StatFuncEntry * pgstat_fetch_stat_funcentry(Oid func_id) { return (PgStat_StatFuncEntry *) - pgstat_fetch_entry(PGSTAT_KIND_FUNCTION, MyDatabaseId, func_id); + pgstat_fetch_entry(PGSTAT_KIND_FUNCTION, MyDatabaseId, func_id, NULL); } diff --git a/src/backend/utils/activity/pgstat_relation.c b/src/backend/utils/activity/pgstat_relation.c index bc8c43b96aa..01239472f6b 100644 --- a/src/backend/utils/activity/pgstat_relation.c +++ b/src/backend/utils/activity/pgstat_relation.c @@ -61,7 +61,7 @@ pgstat_copy_relation_stats(Relation dst, Relation src) PgStat_EntryRef *dst_ref; srcstats = pgstat_fetch_stat_tabentry_ext(src->rd_rel->relisshared, - RelationGetRelid(src)); + RelationGetRelid(src), NULL); if (!srcstats) return; @@ -468,7 +468,7 @@ pgstat_update_heap_dead_tuples(Relation rel, int delta) PgStat_StatTabEntry * pgstat_fetch_stat_tabentry(Oid relid) { - return pgstat_fetch_stat_tabentry_ext(IsSharedRelation(relid), relid); + return pgstat_fetch_stat_tabentry_ext(IsSharedRelation(relid), relid, NULL); } /* @@ -476,12 +476,12 @@ pgstat_fetch_stat_tabentry(Oid relid) * whether the to-be-accessed table is a shared relation or not. */ PgStat_StatTabEntry * -pgstat_fetch_stat_tabentry_ext(bool shared, Oid reloid) +pgstat_fetch_stat_tabentry_ext(bool shared, Oid reloid, bool *will_free) { Oid dboid = (shared ? InvalidOid : MyDatabaseId); return (PgStat_StatTabEntry *) - pgstat_fetch_entry(PGSTAT_KIND_RELATION, dboid, reloid); + pgstat_fetch_entry(PGSTAT_KIND_RELATION, dboid, reloid, will_free); } /* diff --git a/src/backend/utils/activity/pgstat_replslot.c b/src/backend/utils/activity/pgstat_replslot.c index 168ef8f8f45..e09bb6b4355 100644 --- a/src/backend/utils/activity/pgstat_replslot.c +++ b/src/backend/utils/activity/pgstat_replslot.c @@ -208,7 +208,7 @@ pgstat_fetch_replslot(NameData slotname) if (idx != -1) slotentry = (PgStat_StatReplSlotEntry *) pgstat_fetch_entry(PGSTAT_KIND_REPLSLOT, - InvalidOid, idx); + InvalidOid, idx, NULL); LWLockRelease(ReplicationSlotControlLock); diff --git a/src/backend/utils/activity/pgstat_subscription.c b/src/backend/utils/activity/pgstat_subscription.c index 3277cf88a4e..3eaf3e0390f 100644 --- a/src/backend/utils/activity/pgstat_subscription.c +++ b/src/backend/utils/activity/pgstat_subscription.c @@ -107,7 +107,7 @@ PgStat_StatSubEntry * pgstat_fetch_stat_subscription(Oid subid) { return (PgStat_StatSubEntry *) - pgstat_fetch_entry(PGSTAT_KIND_SUBSCRIPTION, InvalidOid, subid); + pgstat_fetch_entry(PGSTAT_KIND_SUBSCRIPTION, InvalidOid, subid, NULL); } /* diff --git a/src/include/pgstat.h b/src/include/pgstat.h index 2786a7c5ffb..a42d2124080 100644 --- a/src/include/pgstat.h +++ b/src/include/pgstat.h @@ -763,7 +763,8 @@ extern void pgstat_twophase_postabort(FullTransactionId fxid, uint16 info, extern PgStat_StatTabEntry *pgstat_fetch_stat_tabentry(Oid relid); extern PgStat_StatTabEntry *pgstat_fetch_stat_tabentry_ext(bool shared, - Oid reloid); + Oid reloid, + bool *will_free); extern PgStat_TableStatus *find_tabstat_entry(Oid rel_id); diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h index c0496f2c69c..1669644fe8a 100644 --- a/src/include/utils/pgstat_internal.h +++ b/src/include/utils/pgstat_internal.h @@ -685,7 +685,8 @@ extern PgStat_EntryRef *pgstat_prep_pending_entry(PgStat_Kind kind, Oid dboid, extern PgStat_EntryRef *pgstat_fetch_pending_entry(PgStat_Kind kind, Oid dboid, uint64 objid); -extern void *pgstat_fetch_entry(PgStat_Kind kind, Oid dboid, uint64 objid); +extern void *pgstat_fetch_entry(PgStat_Kind kind, Oid dboid, uint64 objid, + bool *will_free); extern void pgstat_snapshot_fixed(PgStat_Kind kind); diff --git a/src/test/modules/test_custom_stats/test_custom_var_stats.c b/src/test/modules/test_custom_stats/test_custom_var_stats.c index 2ef0e903745..95ec7687c6c 100644 --- a/src/test/modules/test_custom_stats/test_custom_var_stats.c +++ b/src/test/modules/test_custom_stats/test_custom_var_stats.c @@ -494,7 +494,7 @@ test_custom_stats_var_fetch_entry(const char *stat_name) return (PgStat_StatCustomVarEntry *) pgstat_fetch_entry(PGSTAT_KIND_TEST_CUSTOM_VAR_STATS, InvalidOid, - PGSTAT_CUSTOM_VAR_STATS_IDX(stat_name)); + PGSTAT_CUSTOM_VAR_STATS_IDX(stat_name), NULL); } /*-------------------------------------------------------------------------- -- 2.50.1