Prevent inconsistent use of stats entry for replication slots

Concurrent activity around replication slot creation and drop could
cause a replication slot to use a stats entry it should not have used
when created, triggering an assertion failure when retrieving this
inconsistent entry from the dshash table used by the stats facility.

The issue is that pgstat_drop_replslot() calls pgstat_drop_entry()
without checking the result.  If pgstat_drop_entry() cannot free the
entry related to the object dropped, pgstat_request_entry_refs_gc()
should be called.  AtEOXact_PgStat_DroppedStats() and surrounding
routines dropping stats entries already do that.

This is documented in pgstat_internal.h, but let's add a comment at the
top of pgstat_drop_entry() as that can be easy to miss.

Reported-by: Alexander Lakhin
Author: Floris Van Nee
Analyzed-by: Andres Freund
Discussion: https://postgr.es/m/17947-b9554521ad963c9c@postgresql.org
Backpatch-through: 15

diff --git a/src/backend/utils/activity/pgstat_replslot.c b/src/backend/utils/activity/pgstat_replslot.c
index f08abad49451dc5e44a54227daeff0423211826d..8685e23302357f6a257e05543b090c25c4e5ce4e 100644 (file)
--- a/src/backend/utils/activity/pgstat_replslot.c
+++ b/src/backend/utils/activity/pgstat_replslot.c
@@ -153,8 +153,9 @@ pgstat_acquire_replslot(ReplicationSlot *slot)
 void
 pgstat_drop_replslot(ReplicationSlot *slot)
 {
-       pgstat_drop_entry(PGSTAT_KIND_REPLSLOT, InvalidOid,
-                                         ReplicationSlotIndex(slot));
+       if (!pgstat_drop_entry(PGSTAT_KIND_REPLSLOT, InvalidOid,
+                                                  ReplicationSlotIndex(slot)))
+               pgstat_request_entry_refs_gc();
 }
 
 /*

diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/activity/pgstat_shmem.c
index d1149adf70272766e8ce1d14b69af0c2a0be931e..3fecb8684a2ef339a7ed69143303d7fe63bc5037 100644 (file)
--- a/src/backend/utils/activity/pgstat_shmem.c
+++ b/src/backend/utils/activity/pgstat_shmem.c
@@ -854,6 +854,17 @@ pgstat_drop_database_and_contents(Oid dboid)
                pgstat_request_entry_refs_gc();
 }
 
+/*
+ * Drop a single stats entry.
+ *
+ * This routine returns false if the stats entry of the dropped object could
+ * not be freed, true otherwise.
+ *
+ * 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)
 {