postgres.git / summary / log / commit / refs

commit    8191e0c16a0373f851a9f5a8112e3aec105b5276
Author:   Michael Paquier <michael@paquier.xyz>
Date:     Mon Sep 08 06:52:23 2025 +0000

    Fix corruption of pgstats shared hashtable due to OOM failures
    
    A new pgstats entry is created as a two-step process:
    - The entry is looked at in the shared hashtable of pgstats, and is
    inserted if not found.
    - When not found and inserted, its fields are then initialized.  This
    part include a DSA chunk allocation for the stats data of the new entry.
    
    As currently coded, if the DSA chunk allocation fails due to an
    out-of-memory failure, an ERROR is generated, leaving in the pgstats
    shared hashtable an inconsistent entry due to the first step, as the
    entry has already been inserted in the hashtable.  These broken entries
    can then be found by other backends, crashing them.
    
    There are only two callers of pgstat_init_entry(), when loading the
    pgstats file at startup and when creating a new pgstats entry.  This
    commit changes pgstat_init_entry() so as we use dsa_allocate_extended()
    with DSA_ALLOC_NO_OOM, making it return NULL on allocation failure
    instead of failing.  This way, a backend failing an entry creation can
    take appropriate cleanup actions in the shared hashtable before throwing
    an error.  Currently, this means removing the entry from the shared
    hashtable before throwing the error for the allocation failure.
    
    Out-of-memory errors unlikely happen in the wild, and we do not bother
    with back-patches when these are fixed, usually.  However, the problem
    dealt with here is a degree worse as it breaks the shared memory state
    of pgstats, impacting other processes that may look at an inconsistent
    entry that a different process has failed to create.
    
    Author: Mikhail Kot <mikhail.kot@databricks.com>
    Discussion: https://postgr.es/m/CAAi9E7jELo5_-sBENftnc2E8XhW2PKZJWfTC3i2y-GMQd2bcqQ@mail.gmail.com
    Backpatch-through: 15


 src/backend/utils/activity/pgstat.c       | 11 +++++++++++
 src/backend/utils/activity/pgstat_shmem.c | 28 +++++++++++++++++++++++++++-
 2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
index ffb5b8cce34..f8e91484e36 100644
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -1975,6 +1975,17 @@ pgstat_read_statsfile(void)
 
 					header = pgstat_init_entry(key.kind, p);
 					dshash_release_lock(pgStatLocal.shared_hash, p);
+					if (header == NULL)
+					{
+						/*
+						 * It would be tempting to switch this ERROR to a
+						 * WARNING, but it would mean that all the statistics
+						 * are discarded when the environment fails on OOM.
+						 */
+						elog(ERROR, "could not allocate entry %u/%u/%" PRIu64 " of type %c",
+							 key.kind, key.dboid,
+							 key.objid, t);
+					}
 
 					if (!read_chunk(fpin,
 									pgstat_get_entry_data(key.kind, header),
diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/activity/pgstat_shmem.c
index 62de3474453..9dc3212f7dd 100644
--- a/src/backend/utils/activity/pgstat_shmem.c
+++ b/src/backend/utils/activity/pgstat_shmem.c
@@ -289,6 +289,13 @@ pgstat_detach_shmem(void)
  * ------------------------------------------------------------
  */
 
+/*
+ * Initialize entry newly-created.
+ *
+ * Returns NULL in the event of an allocation failure, so as callers can
+ * take cleanup actions as the entry initialized is already inserted in the
+ * shared hashtable.
+ */
 PgStatShared_Common *
 pgstat_init_entry(PgStat_Kind kind,
 				  PgStatShared_HashEntry *shhashent)
@@ -311,7 +318,12 @@ pgstat_init_entry(PgStat_Kind kind,
 	pg_atomic_init_u32(&shhashent->generation, 0);
 	shhashent->dropped = false;
 
-	chunk = dsa_allocate0(pgStatLocal.dsa, pgstat_get_kind_info(kind)->shared_size);
+	chunk = dsa_allocate_extended(pgStatLocal.dsa,
+								  pgstat_get_kind_info(kind)->shared_size,
+								  DSA_ALLOC_ZERO | DSA_ALLOC_NO_OOM);
+	if (chunk == InvalidDsaPointer)
+		return NULL;
+
 	shheader = dsa_get_address(pgStatLocal.dsa, chunk);
 	shheader->magic = 0xdeadbeef;
 
@@ -509,6 +521,20 @@ pgstat_get_entry_ref(PgStat_Kind kind, Oid dboid, uint64 objid, bool create,
 		if (!shfound)
 		{
 			shheader = pgstat_init_entry(kind, shhashent);
+			if (shheader == NULL)
+			{
+				/*
+				 * Failed the allocation of a new entry, so clean up the
+				 * shared hashtable before giving up.
+				 */
+				dshash_delete_entry(pgStatLocal.shared_hash, shhashent);
+
+				ereport(ERROR,
+						(errcode(ERRCODE_OUT_OF_MEMORY),
+						 errmsg("out of memory"),
+						 errdetail("Failed while allocating entry %u/%u/%" PRIu64 ".",
+								   key.kind, key.dboid, key.objid)));
+			}
 			pgstat_acquire_entry_ref(entry_ref, shhashent, shheader);
 
 			if (created_entry != NULL)


[parent: 1f7e9ba3ac4e]