On Sat, Jul 18, 2026 at 09:47:43AM +0900, Michael Paquier wrote: > On Fri, Jul 17, 2026 at 04:15:15PM +0200, Alex Masterov wrote: >> This is the backend-local sibling of the shared/DSA-path issue fixed in >> 8191e0c16a. Present since 5891c7a8ed8 (2022); affects PG15, PG17, PG18, and >> master. > > Yeah, I can buy this one. MemoryContextAlloc() would fail an longjmp > on OOM, so we'd better keep the backend data clean. Perhaps we should > just use the Extended() flavor to bypass the failure and do cleanup > actions.. Will look later.
On HEAD, I am getting a different symptom, which is still an incorrect thing to have but I think that this is just because entry_ref finishes with non-NULL garbage when the allocation fails due to the lose end of the hash insert to caring about this data: FATAL: releasing ref with pending data But that's to me the same thing as what you are seeing. The only difference is that I think that entry_ref is just pointing to some garbage, making the assertion accidentally work. So, what I think we should do is the attached: - Switch to Extended(MCXT_ALLOC_NO_OOM) - For the cases of !found and !cache_entry->entry_ref, clean up the hash table if the allocation fails. Actually, the found && !entry_ref has always been a kind of dead code pattern? Due to the use an Alloc() that could fail, this would mess up with the backend shutdown, which is worse than assuming that entry_ref could be NULL on secondary lookup? Keeping the check is a defensive thing, which is fine at the end. Just mentioning in passing.. Alex, what do you think? That works with your scenario. The injection point should be removed at the end, just kept it here to demonstrate the problem reusing your scenario (leaks memory of course, but we don't care as long as the hash table is left untouched). -- Michael
From f75a7c14155a953af56ba3fda16e66ed8f4065d8 Mon Sep 17 00:00:00 2001 From: Michael Paquier <[email protected]> Date: Sat, 18 Jul 2026 11:47:00 +0900 Subject: [PATCH] Improve pgstat_get_entry_ref_cached() behavior on OOMs --- src/backend/utils/activity/pgstat_shmem.c | 26 ++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/activity/pgstat_shmem.c index 5ea3f1973f90..22c6ed4981a3 100644 --- a/src/backend/utils/activity/pgstat_shmem.c +++ b/src/backend/utils/activity/pgstat_shmem.c @@ -15,6 +15,7 @@ #include "pgstat.h" #include "storage/shmem.h" #include "storage/subsystems.h" +#include "utils/injection_point.h" #include "utils/memutils.h" #include "utils/pgstat_internal.h" @@ -432,9 +433,28 @@ pgstat_get_entry_ref_cached(PgStat_HashKey key, PgStat_EntryRef **entry_ref_p) { PgStat_EntryRef *entry_ref; - cache_entry->entry_ref = entry_ref = - MemoryContextAlloc(pgStatSharedRefContext, - sizeof(PgStat_EntryRef)); + entry_ref = MemoryContextAllocExtended(pgStatSharedRefContext, + sizeof(PgStat_EntryRef), + MCXT_ALLOC_NO_OOM); + + /* emulate OOM */ + if (IS_INJECTION_POINT_ATTACHED("pgstat-entry-ref-oom")) + entry_ref = NULL; + + if (unlikely(entry_ref == NULL)) + { + /* + * Clean the hash entry to keep the table consistent in the + * backend. + */ + pgstat_entry_ref_hash_delete(pgStatEntryRefHash, key); + + ereport(ERROR, + (errcode(ERRCODE_OUT_OF_MEMORY), + errmsg("out of memory"))); + } + + cache_entry->entry_ref = entry_ref; entry_ref->shared_stats = NULL; entry_ref->shared_entry = NULL; entry_ref->pending = NULL; -- 2.55.0
signature.asc
Description: PGP signature
