Unexpected behavior after OOM errors

On Wed, 17 Jun 2026 at 08:00, Alexander Lakhin <exclusion@gmail.com> wrote:

Hello hackers,

I'd like to share my findings related to OOM error handling. I'm not sure
how large the class of such anomalies is (and if all of these can be
detected and fixed), but please look at a few issues I have discovered so
far:

1) An issue in lookup_type_cache()

The following modification:

<snip>

makes this script:

<snip>

trigger an assertion failure:

<snip>

Without asserts enables, the server might crash.

I believe this is caused by partial subsystem initialization. Attached
patch 0001 should address this failure without causing the server to
restart on OOM.

2) An issue in GetSnapshotData()

The following modification:

<snip>

makes this script (max_prepared_transactions = 2 in postgresql.conf):

<snip>

trigger a segmentation fault:

<snip>

Again, caused by partial initialization, though in this case it's of a
SnapshotData* which is later checked again. Attached patch 0002 should
address this failure.

3) An issue in StandbyAcquireAccessExclusiveLock()

<snip>

I'm not sure how to solve this correctly; I think ideally the
StandbyAcquireAccessExclusiveLock() hash code would be wrapped by a
critical section, but I'm not 100% sure if that will be a sufficient
approach; and it'd definitely need some code to allow the various
hashmaps' memctxs to alloc during critical sections.

Kind regards,

Matthias van de Meent
Databricks (https://www.databricks.com)

On Wed, Jun 17, 2026 at 02:27:25PM +0200, Matthias van de Meent wrote:

On Wed, 17 Jun 2026 at 08:00, Alexander Lakhin <exclusion@gmail.com> wrote:

1) An issue in lookup_type_cache()

I believe this is caused by partial subsystem initialization. Attached
patch 0001 should address this failure without causing the server to
restart on OOM.

Hmm. I think that this is an ordering problem. We could make the
callbacks be registered last, once we are sure that the two hash
tables and the in-progress list have been initialized. I am not sure
that this requires a new facility; it is also an advantage to keep the
initialization sequence in a one code path, without an abstraction.

RelIdToTypeIdCacheHash and RelIdToTypeIdCacheHash are in the
TopMemoryContext, static to the process, so we could just check them
for NULL-ness to make the initialization repeatable. That gives me
the attached v2. Reusing Alexander's randomness trick, that looks
stable here.

2) An issue in GetSnapshotData()

Again, caused by partial initialization, though in this case it's of a
SnapshotData* which is later checked again. Attached patch 0002 should
address this failure.

Yeah, that seems right to make repeated calls of GetSnapshotData()
able to work. LGTM.

3) An issue in StandbyAcquireAccessExclusiveLock()

<snip>

I'm not sure how to solve this correctly; I think ideally the
StandbyAcquireAccessExclusiveLock() hash code would be wrapped by a
critical section, but I'm not 100% sure if that will be a sufficient
approach; and it'd definitely need some code to allow the various
hashmaps' memctxs to alloc during critical sections.

Not checked this one yet.

Thoughts about the first part?
--
Michael

On 18/06/2026 07:37, Michael Paquier wrote:

On Wed, Jun 17, 2026 at 02:27:25PM +0200, Matthias van de Meent wrote:

On Wed, 17 Jun 2026 at 08:00, Alexander Lakhin <exclusion@gmail.com> wrote:

1) An issue in lookup_type_cache()

I believe this is caused by partial subsystem initialization. Attached
patch 0001 should address this failure without causing the server to
restart on OOM.

Hmm. I think that this is an ordering problem. We could make the
callbacks be registered last, once we are sure that the two hash
tables and the in-progress list have been initialized. I am not sure
that this requires a new facility; it is also an advantage to keep the
initialization sequence in a one code path, without an abstraction.

RelIdToTypeIdCacheHash and RelIdToTypeIdCacheHash are in the
TopMemoryContext, static to the process, so we could just check them
for NULL-ness to make the initialization repeatable. That gives me
the attached v2. Reusing Alexander's randomness trick, that looks
stable here.

Yeah, this can be solved by ordering. It's a bit fiddly though. I don't
know about Matthias's proposal either, but it'd be nice to have a less
fiddly system for these.

One idea is to have something similar to
START_CRIT_SECTION()/END_CRIT_SECTION(), but instead of promoting the
ERROR to a PANIC, promote it to FATAL. That way, if any of these
one-time allocations fail, the backend exits. If you're so
memory-starved that you cannot even initialize the type cache, you won't
be able to do anything useful with the connection anyway.

Another idea is that instead of having these be singletons in the type
cache, initialized on first use, move it to a new TypeCacheInitialize()
function that is always called at backend startup, like
RelationCacheInitialize(). If an allocation fails at that stage, the
backend will just exit. I think that's my favorite alternative so far.

BTW, I'm surprised we create the hash tables are created in
TopMemoryContext rather than CacheMemoryContext...

- Heikki

On Thu, 18 Jun 2026 at 06:37, Michael Paquier <michael@paquier.xyz> wrote:

On Wed, Jun 17, 2026 at 02:27:25PM +0200, Matthias van de Meent wrote:

On Wed, 17 Jun 2026 at 08:00, Alexander Lakhin <exclusion@gmail.com> wrote:

1) An issue in lookup_type_cache()

I believe this is caused by partial subsystem initialization. Attached
patch 0001 should address this failure without causing the server to
restart on OOM.

Hmm. I think that this is an ordering problem. We could make the
callbacks be registered last, once we are sure that the two hash
tables and the in-progress list have been initialized.

I don't disagree that there's an ordering problem, but in my view the
main problem is the fallible initialization of N components, gated
behind a single condition. The question of when to register the
callbacks is just one part of many.

I am not sure
that this requires a new facility; it is also an advantage to keep the
initialization sequence in a one code path, without an abstraction.

I'm not a huge fan of templating if(!global) { init_global(); } all
around. But, it works.

RelIdToTypeIdCacheHash and RelIdToTypeIdCacheHash are in the
TopMemoryContext, static to the process, so we could just check them
for NULL-ness to make the initialization repeatable. That gives me
the attached v2. Reusing Alexander's randomness trick, that looks
stable here.

This un-fixes one of the unlikely issues that was fixed in my patch -
though unrelated to OOM:

Each of the calls to
CacheRegisterSyscacheCallback/CacheRegisterRelcacheCallback can throw
an ERROR when all slots have been used. This would leave the typcache
in an invalid state, so I think that must be wrapped in a critical
section: neither syscache nor relcache has options to release
callbacks, and we can't safely continue without the callbacks
installed, so once an error is thrown here this backend can't ever be
properly initialized. This is unlike OOMs, whose conditions for
failure may (and often do) change as workloads change in other
backends.

I think Heikki's suggestion for a FATAL critical section option would
be a good alternative. It wouldn't always be sufficient, but would fix
issues here.

2) An issue in GetSnapshotData()

Again, caused by partial initialization, though in this case it's of a
SnapshotData* which is later checked again. Attached patch 0002 should
address this failure.

Yeah, that seems right to make repeated calls of GetSnapshotData()
able to work. LGTM.

Thanks for committing!

Kind regards,

Matthias van de Meent
Databricks (https://www.databricks.com)

On Thu, 18 Jun 2026 at 06:37, Michael Paquier <michael@paquier.xyz> wrote:

3) An issue in StandbyAcquireAccessExclusiveLock()

<snip>

I'm not sure how to solve this correctly; I think ideally the
StandbyAcquireAccessExclusiveLock() hash code would be wrapped by a
critical section, but I'm not 100% sure if that will be a sufficient
approach; and it'd definitely need some code to allow the various
hashmaps' memctxs to alloc during critical sections.

Not checked this one yet.

I found that the attached patch v3 solves that issue. The assert fires
because we link the lock into the transaction's exclusive locks ahead
of actually having acquired the lock, and when that lock acquisition
fails, as part of the error handling we hit
StartupProcExit->ShutdownRecoveryTransactionEnvironment->StandbyReleaseAllLocks,
which causes this assertion failure because the lock was not taken by
this backend.

By moving StandbyAcquireAccessExclusiveLock's LockAcquire ahead of
when it links the lock to the transaction, the local data structure
doesn't know to clean up the lock until after it's acquired, so
failure in that process won't make error cleanup try to clean up the
lock.

Kind regards,

Matthias van de Meent
Databricks (https://www.databricks.com)

On Thu, Jun 18, 2026 at 05:27:57PM +0200, Matthias van de Meent wrote:

By moving StandbyAcquireAccessExclusiveLock's LockAcquire ahead of
when it links the lock to the transaction, the local data structure
doesn't know to clean up the lock until after it's acquired, so
failure in that process won't make error cleanup try to clean up the
lock.

Yep, reordering these two actions would take care of the list
inconsistency where the startup process goes down following the ERROR
promoted to a FATAL.

I have been fingering the idea of backpatching this fix for a few
minutes, actually, but discarded the idea at the end. It does not
require a random pattern to cause the failure, being actionable
through a combination of GUCs as Alexander has proved. Still, the
only consequence is an extra LOG entry telling that the lock is not
being tracked for non-assert builds. Confusing, OK, but not really
critical.

Comments?
--
Michael

On Thu, Jun 18, 2026 at 11:27:28AM +0200, Matthias van de Meent wrote:

Each of the calls to
CacheRegisterSyscacheCallback/CacheRegisterRelcacheCallback can throw
an ERROR when all slots have been used. This would leave the typcache
in an invalid state, so I think that must be wrapped in a critical
section: neither syscache nor relcache has options to release
callbacks, and we can't safely continue without the callbacks
installed, so once an error is thrown here this backend can't ever be
properly initialized. This is unlike OOMs, whose conditions for
failure may (and often do) change as workloads change in other
backends.

We don't ERROR when failing to register a syscache/relcache callback,
we FATAL if we reach one of the thresholds. Reaching these thresholds
points to me to a programming error anyway, so these should not matter
in the field. The OOM is a random pattern that can happen outside the
Postgres realm.

Just in case, I have planted a elog(FATAL) triggering randomly in the
middle of cache registration callback calls, and the typcache
inconsistency does not come in play with the shutdown sequence once
these trigger even if we have the tables set but not the callbacks.
As a whole, I tend to think that reordering the actions is a solution
good enough here.

I think Heikki's suggestion for a FATAL critical section option would
be a good alternative. It wouldn't always be sufficient, but would fix
issues here.

That sounds like an interesting idea, potentially reusable for other
areas, but I'm not really convinced that we need to add this kind of
facility for the case dealt with here. To me, that's also where we
could use a TRY/CATCH block and call it a day. If others feel
differently about this matter, I'm fine to be outvoted.
--
Michael

On Fri, 19 Jun 2026 at 01:55, Michael Paquier <michael@paquier.xyz> wrote:

On Thu, Jun 18, 2026 at 11:27:28AM +0200, Matthias van de Meent wrote:

Each of the calls to
CacheRegisterSyscacheCallback/CacheRegisterRelcacheCallback can throw
an ERROR when all slots have been used. This would leave the typcache
in an invalid state, so I think that must be wrapped in a critical
section: neither syscache nor relcache has options to release
callbacks, and we can't safely continue without the callbacks
installed, so once an error is thrown here this backend can't ever be
properly initialized. This is unlike OOMs, whose conditions for
failure may (and often do) change as workloads change in other
backends.

We don't ERROR when failing to register a syscache/relcache callback,
we FATAL if we reach one of the thresholds.

Ah, thanks for correcting me. I'm not sure why I had ERROR in mind,
but you're obviously correct. Your patch v2 LGTM.

Reaching these thresholds
points to me to a programming error anyway, so these should not matter
in the field.

I don't think that's (necessarily) correct. These callbacks are
accessible to extensions, and if you load sufficiently many of those
you could still run out of slots even if each extension stayed well
within a reasonable threshold.

I think Heikki's suggestion for a FATAL critical section option would
be a good alternative. It wouldn't always be sufficient, but would fix
issues here.

That sounds like an interesting idea, potentially reusable for other
areas, but I'm not really convinced that we need to add this kind of
facility for the case dealt with here. To me, that's also where we
could use a TRY/CATCH block and call it a day. If others feel
differently about this matter, I'm fine to be outvoted.

While TRY/CATCH would work, I'm not so keen on adding those to
system-initalizing code; the allocations generally go into contexts
that aren't cleaned up nicely during error handling. E.g. a partial
hash initialization for any of the cache hashmaps due to OOM will leak
its allocations.
Failing the connection for that makes sure we complete the right
cleanup procedures and not leak those resources (and it adds another
item to the multi-threading concerns list).

-Matthias

On Fri, 19 Jun 2026 at 01:30, Michael Paquier <michael@paquier.xyz> wrote:

On Thu, Jun 18, 2026 at 05:27:57PM +0200, Matthias van de Meent wrote:

By moving StandbyAcquireAccessExclusiveLock's LockAcquire ahead of
when it links the lock to the transaction, the local data structure
doesn't know to clean up the lock until after it's acquired, so
failure in that process won't make error cleanup try to clean up the
lock.

Yep, reordering these two actions would take care of the list
inconsistency where the startup process goes down following the ERROR
promoted to a FATAL.

I have been fingering the idea of backpatching this fix for a few
minutes, actually, but discarded the idea at the end. It does not
require a random pattern to cause the failure, being actionable
through a combination of GUCs as Alexander has proved. Still, the
only consequence is an extra LOG entry telling that the lock is not
being tracked for non-assert builds. Confusing, OK, but not really
critical.

Comments?

Because it fixes an assertion, I'd vote for backpatching; but because
the case is handled safely without assertions I also wouldn't be upset
if it wasn't backpatched.

-Matthias

On Fri, Jun 19, 2026 at 09:18:03AM +0200, Matthias van de Meent wrote:

On Fri, 19 Jun 2026 at 01:55, Michael Paquier <michael@paquier.xyz> wrote:

We don't ERROR when failing to register a syscache/relcache callback,
we FATAL if we reach one of the thresholds.

Ah, thanks for correcting me. I'm not sure why I had ERROR in mind,
but you're obviously correct. Your patch v2 LGTM.

Cool, thanks.

Reaching these thresholds
points to me to a programming error anyway, so these should not matter
in the field.

I don't think that's (necessarily) correct. These callbacks are
accessible to extensions, and if you load sufficiently many of those
you could still run out of slots even if each extension stayed well
within a reasonable threshold.

Sure, but then the only way to get out of the problem is to patch the
backend so there are more slots available. At the end, when it comes
to core (and there should be some margin anyway), I am not really
worried.
--
Michael

On Fri, Jun 19, 2026 at 09:22:28AM +0200, Matthias van de Meent wrote:

Because it fixes an assertion, I'd vote for backpatching; but because
the case is handled safely without assertions I also wouldn't be upset
if it wasn't backpatched.

With some tweaks to the comments, applied this one on HEAD, without a
backpatch.
--
Michael

On Fri, Jun 19, 2026 at 08:12:47PM +0900, Michael Paquier wrote:

On Fri, Jun 19, 2026 at 09:18:03AM +0200, Matthias van de Meent wrote:

Ah, thanks for correcting me. I'm not sure why I had ERROR in mind,
but you're obviously correct. Your patch v2 LGTM.

Cool, thanks.

And done that as well. If we invent a new facility, I'd be curious to
see how this would apply here or to other parts of the backend. And
perhaps not only for initialization steps?
--
Michael

Hello Michael and Matthias,

20.06.2026 10:45, Michael Paquier wrote:

On Fri, Jun 19, 2026 at 08:12:47PM +0900, Michael Paquier wrote:

On Fri, Jun 19, 2026 at 09:18:03AM +0200, Matthias van de Meent wrote:

Ah, thanks for correcting me. I'm not sure why I had ERROR in mind,
but you're obviously correct. Your patch v2 LGTM.

Cool, thanks.

And done that as well. If we invent a new facility, I'd be curious to
see how this would apply here or to other parts of the backend. And
perhaps not only for initialization steps?

Thank you for working on the fixes!

Best regards,
Alexander

Hello Michael and Matthias,

Could you please look at other similar anomalies I have discovered?:

(Assuming the attached OOM injection patch applied.)
1) An issue in generate_partition_qual():
--- a/src/backend/utils/cache/partcache.c
+++ b/src/backend/utils/cache/partcache.c
@@ -419,3 +419,5 @@ generate_partition_qual(Relation rel)
          oldcxt = MemoryContextSwitchTo(rel->rd_partcheckcxt);
+oom_prob = 0.1;
          rel->rd_partcheck = copyObject(result);
+oom_prob = 0;
          MemoryContextSwitchTo(oldcxt);

for i in {1..100}; do
echo "
CREATE TABLE t1(a INT PRIMARY KEY) PARTITION BY RANGE (a);
CREATE TABLE t1p1 PARTITION OF t1 FOR VALUES FROM (1) TO (2) PARTITION BY LIST (a);
CREATE TABLE t2 (a INT, CONSTRAINT fkey FOREIGN KEY(a) REFERENCES t1(a));
INSERT INTO t2 VALUES (1);
INSERT INTO t2 VALUES (1);
DROP TABLE t1, t2 CASCADE;
" | psql

grep 'TRAP' server.log && break;
done

triggers
TRAP: failed Assert("cache->cc_tupdesc != NULL"), File: "catcache.c", Line: 1112, PID: 43932

2) lookup_type_cache():
--- a/src/backend/utils/cache/typcache.c
+++ b/src/backend/utils/cache/typcache.c
@@ -463,8 +463,10 @@ lookup_type_cache(Oid type_id, int flags)
      in_progress_list[in_progress_offset] = type_id;

+oom_prob = 0.1;
     /* Try to look up an existing entry */
     typentry = (TypeCacheEntry *) hash_search(TypeCacheHash,
                                               &type_id,
                                               HASH_FIND, NULL);
+oom_prob = 0;
     if (typentry == NULL)
     {

for i in {1..10}; do
echo "
CREATE TABLE t(a int);
\c
SELECT * FROM t ORDER BY a;

DROP TABLE t;
" | psql

grep 'TRAP' server.log && break;
done

triggers
TRAP: failed Assert("cache->cc_tupdesc != NULL"), File: "catcache.c", Line: 1112, PID: 43932

3) pgstat_prep_pending_entry() / pgstat_write_statsfiles():
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -1333,3 +1333,5 @@ pgstat_prep_pending_entry(PgStat_Kind kind, Oid dboid, uint64 objid, bool *creat

+oom_prob = 0.005;
         entry_ref->pending = MemoryContextAllocZero(pgStatPendingContext, entrysize);
+oom_prob = 0;
         dlist_push_tail(&pgStatPending, &entry_ref->pending_node);

for i in {1..100}; do
pg_ctl -l server.log start
echo "
CREATE TABLE t (a int);
ALTER TABLE t SET (parallel_workers = 20);
SET parallel_setup_cost = 0;
SELECT SUM(a) FROM t;
DROP TABLE t;
" | psql
pg_ctl -l server.log stop

grep 'TRAP' server.log && break;
done

triggers
TRAP: failed Assert("!ps->dropped"), File: "pgstat.c", Line: 1730, PID: 844901
maybe this Assert can be removed, because there is no corresponding
assert in pgstat_build_snapshot(), but I saw also:
TRAP: failed Assert("rel->pgstat_info->relation == NULL"), File: "pgstat_relation.c", Line: 142, PID: 157820
TRAP: failed Assert("ent->entry_ref != NULL"), File: "pgstat_shmem.c", Line: 827, PID: 190431

4) make_new_heap():
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -1255,3 +1255,5 @@ make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, Oid NewAccessMethod,
       */
+oom_prob = 0.01;
      CommandCounterIncrement();
+oom_prob = 0;

for i in {1..100}; do
echo "
CREATE TABLE t1(i int PRIMARY KEY);
CLUSTER t1 USING t1_pkey;
CREATE TABLE t2(i int PRIMARY KEY);
CLUSTER t2 USING t2_pkey;
DROP TABLE t2;
DROP TABLE t1;
" | psql

grep 'TRAP' server.log && break;
done

triggers:
TRAP: failed Assert("ItemIdIsNormal(lp)"), File: "heapam.c", Line: 2813, PID: 943626
also:
ERROR:  attempted to delete invisible tuple

(similar to [1]/messages/by-id/21363eb7-606c-468d-88f4-c14162ddafc8@gmail.com, which can also be triggered with OOM inside
CommandCounterIncrement(), but this one is even more interesting, IMO)

[1]: /messages/by-id/21363eb7-606c-468d-88f4-c14162ddafc8@gmail.com

Best regards,
Alexander

Hello Michael, Matthias, and Alexander,

The assertion in issue #3 (ent->entry_ref != NULL, pgstat_shmem.c) also has
a separate trigger I'd like to report — a different code path that poisons
the same hash in pgstat_get_entry_ref_cached().
pgstat_get_entry_ref_cached() inserts a zero-filled hash slot — entry_ref
== NULL — before allocating memory for it. If MemoryContextAlloc() fails
with OOM it longjmps out, leaving the NULL slot permanently poisoned in
pgStatEntryRefHash. The hash is backend-local and cross-transaction, so
abort does not clean it up.

Crash vectors:
- Debug builds: backend exit hits Assert(ent->entry_ref != NULL) in
pgstat_release_matching_entry_refs()
- Release builds: any GC pass (DROP TABLE etc.) causes
pgstat_gc_entry_refs() to unconditionally dereference the NULL → SIGSEGV

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.

The bug can be reproduced without your OOM injection patch using the
built-in injection points infrastructure (--enable-injection-points
--enable-cassert). Apply this one-liner to pgstat_get_entry_ref_cached():

--- a/src/backend/utils/activity/pgstat_shmem.c
+++ b/src/backend/utils/activity/pgstat_shmem.c
@@ -14,6 +14,7 @@

#include "pgstat.h"
#include "storage/shmem.h"
+#include "utils/injection_point.h"
#include "utils/memutils.h"
#include "utils/pgstat_internal.h"

@@ -387,6 +388,9 @@
      if (!found || !cache_entry->entry_ref)
      {
              PgStat_EntryRef *entry_ref;
+
+             /* slot exists, entry_ref == NULL -- same window a real OOM
longjmps from */
+             INJECTION_POINT("pgstat-entry-ref-oom");

cache_entry->entry_ref = entry_ref =
MemoryContextAlloc(pgStatSharedRefContext,

Then:

CREATE EXTENSION injection_points;
CREATE TABLE t (id int PRIMARY KEY, v int);
INSERT INTO t VALUES (1, 0);
SELECT injection_points_attach('pgstat-entry-ref-oom', 'error');
UPDATE t SET v = v + 1 WHERE id = 1;
SELECT injection_points_detach('pgstat-entry-ref-oom');
-- disconnect; backend exit crashes

The UPDATE fails as expected:
ERROR: error triggered for injection point pgstat-entry-ref-oom

But on session exit, the backend crashes:
TRAP: failed Assert("ent->entry_ref != NULL"), File: "pgstat_shmem.c",
Line: 781
server process (PID ...) was terminated by signal 6: Abort trap

Alexey

On Fri, Jul 17, 2026 at 04:15:15PM +0200, Alex Masterov wrote:

The assertion in issue #3 (ent->entry_ref != NULL, pgstat_shmem.c) also has
a separate trigger I'd like to report — a different code path that poisons
the same hash in pgstat_get_entry_ref_cached().
pgstat_get_entry_ref_cached() inserts a zero-filled hash slot — entry_ref
== NULL — before allocating memory for it. If MemoryContextAlloc() fails
with OOM it longjmps out, leaving the NULL slot permanently poisoned in
pgStatEntryRefHash. The hash is backend-local and cross-transaction, so
abort does not clean it up.

Crash vectors:
- Debug builds: backend exit hits Assert(ent->entry_ref != NULL) in
pgstat_release_matching_entry_refs()
- Release builds: any GC pass (DROP TABLE etc.) causes
pgstat_gc_entry_refs() to unconditionally dereference the NULL → SIGSEGV

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.
--
Michael

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

On Fri, Jul 17, 2026 at 12:00:01PM +0300, Alexander Lakhin wrote:

(Assuming the attached OOM injection patch applied.)
1) An issue in generate_partition_qual():
--- a/src/backend/utils/cache/partcache.c
+++ b/src/backend/utils/cache/partcache.c
@@ -419,3 +419,5 @@ generate_partition_qual(Relation rel)
         oldcxt = MemoryContextSwitchTo(rel->rd_partcheckcxt);
+oom_prob = 0.1;
         rel->rd_partcheck = copyObject(result);
+oom_prob = 0;
         MemoryContextSwitchTo(oldcxt);

This one is again an ordering problem: let's assign rd_partcheckcxt
and rd_partcheck once we are sure that all the allocations have
succeeded, so as we don't finish with a semi-allocated transient
state.

2) lookup_type_cache():
--- a/src/backend/utils/cache/typcache.c
+++ b/src/backend/utils/cache/typcache.c
@@ -463,8 +463,10 @@ lookup_type_cache(Oid type_id, int flags)
     in_progress_list[in_progress_offset] = type_id;

+oom_prob = 0.1;
     /* Try to look up an existing entry */
     typentry = (TypeCacheEntry *) hash_search(TypeCacheHash,
                                               &type_id,
                                               HASH_FIND, NULL);
+oom_prob = 0;

triggers
TRAP: failed Assert("cache->cc_tupdesc != NULL"), File:
"catcache.c", Line: 1112, PID: 43932

This one is tricky. That's obviously due to the fact that we store a
type ID in the in_progress_list and on OOM we finish with a stale
entry in the list, no cleaned up. My first impression was: could we
delay the registration of the type ID in the in_progress_list? But
the answer to that is no; we want it from the point where we to the
lookups in the typcache hash table to protect from concurrent
invalidation. So I am not sure what to do here yet.

3) pgstat_prep_pending_entry() / pgstat_write_statsfiles():
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -1333,3 +1333,5 @@ pgstat_prep_pending_entry(PgStat_Kind kind, Oid dboid, uint64 objid, bool *creat

+oom_prob = 0.005;
         entry_ref->pending = MemoryContextAllocZero(pgStatPendingContext, entrysize);
+oom_prob = 0;
         dlist_push_tail(&pgStatPending, &entry_ref->pending_node);

triggers
TRAP: failed Assert("!ps->dropped"), File: "pgstat.c", Line: 1730, PID: 844901
maybe this Assert can be removed, because there is no corresponding

Yes, perhaps we should do that. I am not completely sure if this is
the best thing to do, though.

4) make_new_heap():
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -1255,3 +1255,5 @@ make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, Oid NewAccessMethod,
      */
+oom_prob = 0.01;
     CommandCounterIncrement();
+oom_prob = 0;

triggers:
TRAP: failed Assert("ItemIdIsNormal(lp)"), File: "heapam.c", Line: 2813, PID: 943626
also:
ERROR:  attempted to delete invisible tuple

I'll let Alvaro comment on this one. It deserves an open item.

For now, I have 1 out of 4, the partcache one. 25% is still better
than 0%. :p
--
Michael

Hi Michael,

The patch looks correct to me. Deleting the hash slot before raising the
error is exactly the right cleanup, and MCXT_ALLOC_NO_OOM is the cleanest
way to get there.

On the different symptom you see on HEAD (FATAL: releasing ref with pending
data vs. the Assert I got on 17.10): I think this is just a matter of
what's in the hash at exit time. The FATAL fires when the iterator hits a
valid entry_ref with unflushed pending stats before it reaches the poisoned
slot; the Assert fires if it reaches the NULL slot first. My repro sequence
(INSERT committed, then UPDATE aborted) likely leaves no pending stats, so
the NULL slot is always hit first. Anyway, it's the same root cause.

Regarding found && !cache_entry->entry_ref being dead code, I agree.

Thanks for picking this up so quickly!

Alexey