DSA_ALLOC_NO_OOM vs dsm_create ERROR leaving a half-initialized pgstats hash entry

Hi,

I'm splitting this out of the pgstat_read_statsfile() cleanup thread [1],
so that discussion can stay about the restore path.

The case I reproduced is exactly that hole:
dsa_allocate_extended(..., DSA_ALLOC_NO_OOM) can still raise ERROR from
dsm_create() in make_new_segment(), after pgstat_init_entry() has marked
the hash entry live and before its body is assigned. The NULL cleanup
in pgstat_get_entry_ref() is then bypassed.

I reproduced this on a running TAP cluster under ASan with a constrained
/dev/shm (recovery/020_archive_status and 034_create_database). The
same postgresql.log shows, about a second apart:

FATAL: could not resize shared memory segment "/PostgreSQL.…"
to 1048576 bytes: No space left on device
… then a later backend …
AddressSanitizer: SEGV on unknown address 0x0
in pgstat_acquire_entry_ref()
from pgstat_get_entry_ref() existing-entry path

gdb on the crashing backend showed a live shared hash entry:

dropped = false, refcount = 1, generation = 0,
body = InvalidDsaPointer,
kind = relation, dboid = 0, objid = pg_authid or pg_database

There was no "Failed while allocating entry" / "could not allocate
entry" message, so the InvalidDsaPointer cleanup added by 8191e0c did
not run. On InitPostgres the ERROR is promoted to FATAL and the
connecting backend exits, but the postmaster does not reinitialize
shared memory, so the half-initialized entry remains visible.

I see two possible layers at which to address this.

1. pgstats only: allocate the DSA body before inserting the shared hash
entry, and initialize the entry only after a valid chunk has been
obtained. A dsm_create() failure would then not leave a live entry
whose body is InvalidDsaPointer.

This is the "flip the order" approach discussed in [2]. It would
require changing the two callers of pgstat_init_entry(), and dealing
with a concurrently inserted entry by freeing the preallocated,
unused chunk.

2. DSA/DSM: make DSA_ALLOC_NO_OOM cover failures to create or resize a
new DSM segment as well, so dsa_allocate_extended() consistently
returns InvalidDsaPointer for allocation failures instead of raising
ERROR.

This seems closer to the documented DSA_ALLOC_NO_OOM contract, but it
is the lower-level change Michael mentioned. It would need to distinguish
resource exhaustion, such as ENOSPC while resizing a POSIX shared
memory object, from DSM failures that should still be reported as
errors.

I would rather not go back to PG_TRY/PG_CATCH around
pgstat_init_entry(); that was considered in [2] and dropped in favour
of returning NULL.

Option (1) could close the pgstats corruption independently of the
lower-level question. Option (2) would make the NO_OOM behavior
consistent for other callers as well.

I can prepare the pgstats patch for (1), or investigate the DSA/DSM
approach first if you think that is the better layer. I can also add a
deterministic failure-injection test for the new-segment path.

Thanks,
Yuriy Grigoryev

[1] https://postgr.es/m/d55ecaf911844d53bd0a931751dce582@localhost.localdomain
[2] https://postgr.es/m/CAAi9E7jELo5_-sBENftnc2E8XhW2PKZJWfTC3i2y-GMQd2bcqQ@mail.gmail.com