On Thu, Sep 17, 2026 at 11:48:26AM +0900, Michael Paquier wrote: > + /* > + * Allocate the stats body before inserting a hash entry. Creating a > + * new DSA segment can raise ERROR (e.g. ENOSPC on posix shm); doing > + * that after the insert would leave a live hash entry with an > + * invalid body. > + */ > + chunk = pgstat_alloc_entry_body(kind); > > Hmm. This still leaves a local entry_ref if pgstat_alloc_entry_body() > itself fails. Compared to the case of a corrupted shmem area. I think > that I can live with that. And if I'm reading that right, the backend > reference that may still be around self-heals on re-entry if a backend > tries to insert again the same entry?
Ah. 4069df21beb8 points exactly at that case. Perhaps we should
extend pgstat_gc_entry_refs() so as it is able to handle gracefully a
partial reference then? I would imagine something like that, that
forces a release of the local entry if we don't have a shared_entry,
as of:
--- a/src/backend/utils/activity/pgstat_shmem.c
+++ b/src/backend/utils/activity/pgstat_shmem.c
@@ -839,6 +839,15 @@ pgstat_gc_entry_refs(void)
Assert(!entry_ref->shared_stats ||
entry_ref->shared_stats->magic == 0xdeadbeef);
+ /* NULL shared_entry marks a partial reference */
+ if (entry_ref->shared_entry == NULL)
+ {
+ Assert(entry_ref->shared_stats == NULL);
+ Assert(entry_ref->pending == NULL);
+ pgstat_release_entry_ref(ent->key, entry_ref, false);
+ continue;
+ }
What do you think about the attached? That would be an independent
safety measure.
--
Michael
From b71b8a5adad24d44905576749fbda337ae1650bd Mon Sep 17 00:00:00 2001 From: Michael Paquier <[email protected]> Date: Thu, 17 Sep 2026 12:41:24 +0900 Subject: [PATCH] pgstat: Add safety measure for partially initialized local reference --- src/backend/utils/activity/pgstat_shmem.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/activity/pgstat_shmem.c index bf886b4dad86..86d7d4a7c6a9 100644 --- a/src/backend/utils/activity/pgstat_shmem.c +++ b/src/backend/utils/activity/pgstat_shmem.c @@ -839,6 +839,15 @@ pgstat_gc_entry_refs(void) Assert(!entry_ref->shared_stats || entry_ref->shared_stats->magic == 0xdeadbeef); + /* NULL shared_entry marks a partial reference */ + if (entry_ref->shared_entry == NULL) + { + Assert(entry_ref->shared_stats == NULL); + Assert(entry_ref->pending == NULL); + pgstat_release_entry_ref(ent->key, entry_ref, false); + continue; + } + /* * "generation" checks for the case of entries being reinitialized, * and "dropped" for the case where these are.. dropped. -- 2.55.0
signature.asc
Description: PGP signature
