Re: Stale hash entries when reusing an auxiliary resource owner

Hi,

On 2026-11-03 00:43:00 +0800, Jingtang Zhang wrote:
> From 79d65b79449cc85b5588eede60187b5616893361 Mon Sep 17 00:00:00 2001
> From: Jingtang Zhang <mrdrivingduck(at)gmail(dot)com>
> Date: Tue, 3 Nov 2026 00:30:29 +0800
> Subject: [PATCH v2] Clear auxiliary resource owner hash before reuse
>
> Bulk release sorts and compacts the resource hash table but only resets
> its item count when releasing resources. Entries in the sorted prefix
> remain populated, and compaction can leave extra copies outside that
> prefix.
>
> An auxiliary process reuses its owner after releasing its resources.
> Clear the whole hash table before allowing reuse so that subsequent
> resource acquisition cannot treat stale entries as live resources or
> release them twice.
>
> Backpatch-through: 17
> ---
> src/backend/utils/resowner/resowner.c | 9 ++
> .../test_resowner/expected/test_resowner.out | 27 ++++++
> .../test_resowner/sql/test_resowner.sql | 8 ++
> .../test_resowner/test_resowner--1.0.sql | 4 +
> .../test_resowner/test_resowner_basic.c | 87 +++++++++++++++++++
> 5 files changed, 135 insertions(+)
>
> diff --git a/src/backend/utils/resowner/resowner.c b/src/backend/utils/resowner/resowner.c
> index ac413d54837..149b5273667 100644
> --- a/src/backend/utils/resowner/resowner.c
> +++ b/src/backend/utils/resowner/resowner.c
> @@ -1038,6 +1038,15 @@ ReleaseAuxProcessResources(bool isCommit)
> ResourceOwnerRelease(AuxProcessResourceOwner,
> RESOURCE_RELEASE_AFTER_LOCKS,
> isCommit, true);
> +
> + /*
> + * Sorting can leave copies of entries beyond the sorted prefix, and bulk
> + * release only resets the counts. Clear the entire table before reuse.
> + */
> + if (AuxProcessResourceOwner->hash != NULL)
> + memset(AuxProcessResourceOwner->hash, 0,
> + AuxProcessResourceOwner->capacity * sizeof(ResourceElem));
> +
> /* allow it to be reused */
> AuxProcessResourceOwner->releasing = false;
> AuxProcessResourceOwner->sorted = false;

I don't think fixing this locally in ReleaseAuxProcessResources() can be the
right fix. It's just continuing an existing pattern, admittedly. But:

It seems quite wrong that ReleaseAuxProcessResources() does stuff that callers
outside of resowner.c can't do. What if there's a need for a long-lived,
reusable, resowner outside of a context where using AuxProcessResourceOwner is
right? Seems like we should just have a helper that makes a resowner reusable
after some assertions?

As for the concrete bug at hand - why don't we fix this up in
ResourceOwnerSort()? ReleaseAuxProcessResources() shouldn't be that common,
but always memset()ing the full size of the hash can't be great? And even just
for debugging in non-auxprocess context, it doesn't seem awesome to have a
hashtable with obsoleted entries still visible?

The fast-path is the !hash case, I don't think any realistic workload would
notice the overhead of zeroing out hash table entries during the compaction in
ResourceOwnerSort()? I don't know if we'd need to do anything else?

Greetings,

Andres Freund