On Tue, Sep 08, 2026 at 11:52:33PM +0000, Michael Paquier wrote: > My question regarding (1) vs (2) would be: do we have other > sub-systems that display patterns similar to pgstats when it comes to > the DSA/DSM failing? If pgstats is the only one, (1) sounds like a > solution good enough for me.
I checked all in-tree DSA_ALLOC_NO_OOM and DSHASH_INSERT_NO_OOM call sites, plus the comparable two-phase cases in typcache.c and async.c. pgstats is the only one that both publishes its surrounding object before allocating the DSA body and assumes the body is always valid. With one exception, discussed below, they all allocate before publishing. In dshash, insert_into_bucket() allocates the item before linking it into a bucket, resize() allocates the new bucket array before replacing the old one, and dshash_create() does not publish the table until its buckets exist. pgsa_set_advice_string() in contrib/pg_stash_advice allocates the advice string, then inserts with DSHASH_INSERT_NO_OOM and frees the string if that returns NULL, which is exactly the ordering the patch gives pgstats. find_or_make_matching_shared_tupledesc() in typcache.c copies the TupleDesc into DSA first, with a PG_TRY() around the insertion to free it on error. The exception, and the closest case to pgstats, is async.c: PrepareTableEntriesForListen() inserts a channel entry with listenersArray == InvalidDsaPointer and allocates afterwards. But that incomplete state is supported by design -- numListeners stays zero, so consumers do not access any array elements, and a later call retries the allocation. pgstats has no such tolerance: the existing-entry path dereferences body unconditionally, which is why the escaped error becomes a SEGV instead of a retry. So option (1) looks sufficient for this crash. Patch attached. It allocates the DSA body before inserting the shared hash entry; pgstat_init_entry() now receives a valid chunk and no longer allocates. Both callers use dshash_find_or_insert_extended() with DSHASH_INSERT_NO_OOM and free the preallocated body if insertion returns NULL or finds an existing entry. One residual case remains: dshash insertion can itself raise ERROR from dsm_create() despite DSHASH_INSERT_NO_OOM, in which case the preallocated chunk is not reclaimed. pg_stash_advice and dshash_create() have analogous unreachable-allocation cases. No inconsistent pgstats entry is published, but closing the leak would require either exception cleanup or the lower-level NO_OOM change. I left PG_TRY/PG_CATCH out based on your comments; typcache shows how it could be used if such cleanup is preferred. 8191e0c was backpatched through 15 for the same class of corruption, so this path may deserve the same treatment. If the approach looks right, I can prepare back-branch versions and add a deterministic test using an injection point in make_new_segment() before dsm_create(). Thanks, Yuriy Grigoryev
0001-Allocate-pgstats-entry-body-before-shared-hash-insert.patch
Description: 0001-Allocate-pgstats-entry-body-before-shared-hash-insert.patch
