From 2e0c61aed6241bb66547bfab7467d43f889f78ba Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Thu, 18 Jun 2026 11:49:36 +0900 Subject: [PATCH] Fix PANIC with track_functions due to concurrent drop of pgstats entries pgstat_drop_entry_internal() generates an ERROR if facing a pgstats entry already marked as dropped. With a workload doing a lot of concurrent CALL and DROP/CREATE PROCEDURE, it could be possible for AtEOXact_PgStat_DroppedStats(), that wants to do transactional drops, to find entries that are already dropped, after a commit record has been written. In this case, ERRORs are upgraded to PANIC, taking down the server. This issue is fixed by making pgstat_drop_entry() optionally more tolerant to concurrent drops, adding to the routine a missing_ok option to make some of its callers more tolerant (spoiler: some of the callers want a strict behavior, like replication slots and backend stats). pgstat_drop_entry_internal() cannot be called anymore for an entry marked as dropped, hence its error is replaced by an assertion. Functions are handled as a special case in core; this problem could also apply to custom stats kinds depending on what an extension does. track_functions is costly when enabled (disabled by default), which is perhaps the main reason why this has not be found yet. A similar version of this patch has been proposed by Sami Imseih on a different thread for a feature in development. This version has tweaked here by me for the sake of fixing this issue. Reported-by: zhanglihui Author: Sami Imseih Author: Michael Paquier Reviewed-by: Ayush Tiwari Discussion: https://postgr.es/m/19520-73873648d44793cf@postgresql.org Backpatch-through: 15 --- src/backend/utils/activity/pgstat_function.c | 2 +- src/backend/utils/activity/pgstat_replslot.c | 2 +- src/backend/utils/activity/pgstat_shmem.c | 27 ++++++++++++++------ src/backend/utils/activity/pgstat_xact.c | 8 +++--- src/include/utils/pgstat_internal.h | 3 ++- 5 files changed, 27 insertions(+), 15 deletions(-) diff --git a/src/backend/utils/activity/pgstat_function.c b/src/backend/utils/activity/pgstat_function.c index 709b8e6726e..675ff13dc5c 100644 --- a/src/backend/utils/activity/pgstat_function.c +++ b/src/backend/utils/activity/pgstat_function.c @@ -113,7 +113,7 @@ pgstat_init_function_usage(FunctionCallInfo fcinfo, if (!SearchSysCacheExists1(PROCOID, ObjectIdGetDatum(fcinfo->flinfo->fn_oid))) { pgstat_drop_entry(PGSTAT_KIND_FUNCTION, MyDatabaseId, - fcinfo->flinfo->fn_oid); + fcinfo->flinfo->fn_oid, true); ereport(ERROR, errcode(ERRCODE_UNDEFINED_FUNCTION), errmsg("function call to dropped function")); } diff --git a/src/backend/utils/activity/pgstat_replslot.c b/src/backend/utils/activity/pgstat_replslot.c index da11b867445..8d5433db362 100644 --- a/src/backend/utils/activity/pgstat_replslot.c +++ b/src/backend/utils/activity/pgstat_replslot.c @@ -158,7 +158,7 @@ pgstat_drop_replslot(ReplicationSlot *slot) Assert(LWLockHeldByMeInMode(ReplicationSlotAllocationLock, LW_EXCLUSIVE)); if (!pgstat_drop_entry(PGSTAT_KIND_REPLSLOT, InvalidOid, - ReplicationSlotIndex(slot))) + ReplicationSlotIndex(slot), false)) pgstat_request_entry_refs_gc(); } diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/activity/pgstat_shmem.c index 324cd9dc5b4..7e03e9f8484 100644 --- a/src/backend/utils/activity/pgstat_shmem.c +++ b/src/backend/utils/activity/pgstat_shmem.c @@ -862,13 +862,7 @@ pgstat_drop_entry_internal(PgStatShared_HashEntry *shent, * Signal that the entry is dropped - this will eventually cause other * backends to release their references. */ - if (shent->dropped) - elog(ERROR, - "trying to drop stats entry already dropped: kind=%s dboid=%u objoid=%u refcount=%u generation=%u", - pgstat_get_kind_info(shent->key.kind)->name, - shent->key.dboid, shent->key.objoid, - pg_atomic_read_u32(&shent->refcount), - pg_atomic_read_u32(&shent->generation)); + Assert(!shent->dropped); shent->dropped = true; /* release refcount marking entry as not dropped */ @@ -944,13 +938,16 @@ pgstat_drop_database_and_contents(Oid dboid) * This routine returns false if the stats entry of the dropped object could * not be freed, true otherwise. * + * If missing_ok is true, skip entries that have been concurrently dropped. + * * The callers of this function should call pgstat_request_entry_refs_gc() * if the stats entry could not be freed, to ensure that this entry's memory * can be reclaimed later by a different backend calling * pgstat_gc_entry_refs(). */ bool -pgstat_drop_entry(PgStat_Kind kind, Oid dboid, Oid objoid) +pgstat_drop_entry(PgStat_Kind kind, Oid dboid, Oid objoid, + bool missing_ok) { PgStat_HashKey key; PgStatShared_HashEntry *shent; @@ -978,6 +975,20 @@ pgstat_drop_entry(PgStat_Kind kind, Oid dboid, Oid objoid) shent = dshash_find(pgStatLocal.shared_hash, &key, true); if (shent) { + if (shent->dropped) + { + if (!missing_ok) + elog(ERROR, + "trying to drop stats entry already dropped: kind=%s dboid=%u objoid=%u refcount=%u generation=%u", + pgstat_get_kind_info(shent->key.kind)->name, + shent->key.dboid, + shent->key.objoid, + pg_atomic_read_u32(&shent->refcount), + pg_atomic_read_u32(&shent->generation)); + dshash_release_lock(pgStatLocal.shared_hash, shent); + return true; + } + freed = pgstat_drop_entry_internal(shent, NULL); /* diff --git a/src/backend/utils/activity/pgstat_xact.c b/src/backend/utils/activity/pgstat_xact.c index 1877d22f146..65108ffaefe 100644 --- a/src/backend/utils/activity/pgstat_xact.c +++ b/src/backend/utils/activity/pgstat_xact.c @@ -84,7 +84,7 @@ AtEOXact_PgStat_DroppedStats(PgStat_SubXactStatus *xact_state, bool isCommit) * Transaction that dropped an object committed. Drop the stats * too. */ - if (!pgstat_drop_entry(it->kind, it->dboid, it->objoid)) + if (!pgstat_drop_entry(it->kind, it->dboid, it->objoid, true)) not_freed_count++; } else if (!isCommit && pending->is_create) @@ -93,7 +93,7 @@ AtEOXact_PgStat_DroppedStats(PgStat_SubXactStatus *xact_state, bool isCommit) * Transaction that created an object aborted. Drop the stats * associated with the object. */ - if (!pgstat_drop_entry(it->kind, it->dboid, it->objoid)) + if (!pgstat_drop_entry(it->kind, it->dboid, it->objoid, true)) not_freed_count++; } @@ -158,7 +158,7 @@ AtEOSubXact_PgStat_DroppedStats(PgStat_SubXactStatus *xact_state, * Subtransaction creating a new stats object aborted. Drop the * stats object. */ - if (!pgstat_drop_entry(it->kind, it->dboid, it->objoid)) + if (!pgstat_drop_entry(it->kind, it->dboid, it->objoid, true)) not_freed_count++; pfree(pending); } @@ -320,7 +320,7 @@ pgstat_execute_transactional_drops(int ndrops, struct xl_xact_stats_item *items, { xl_xact_stats_item *it = &items[i]; - if (!pgstat_drop_entry(it->kind, it->dboid, it->objoid)) + if (!pgstat_drop_entry(it->kind, it->dboid, it->objoid, true)) not_freed_count++; } diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h index 22106778396..c9b0552ed36 100644 --- a/src/include/utils/pgstat_internal.h +++ b/src/include/utils/pgstat_internal.h @@ -625,7 +625,8 @@ extern PgStat_EntryRef *pgstat_get_entry_ref(PgStat_Kind kind, Oid dboid, Oid ob 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 bool pgstat_drop_entry(PgStat_Kind kind, Oid dboid, Oid objoid, + bool missing_ok); extern void pgstat_drop_all_entries(void); extern PgStat_EntryRef *pgstat_get_entry_ref_locked(PgStat_Kind kind, Oid dboid, Oid objoid, bool nowait);