From 79d65b79449cc85b5588eede60187b5616893361 Mon Sep 17 00:00:00 2001 From: Jingtang Zhang 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; diff --git a/src/test/modules/test_resowner/expected/test_resowner.out b/src/test/modules/test_resowner/expected/test_resowner.out index 527b678fc12..28df6e7b141 100644 --- a/src/test/modules/test_resowner/expected/test_resowner.out +++ b/src/test/modules/test_resowner/expected/test_resowner.out @@ -195,3 +195,30 @@ ERROR: ResourceOwnerEnlarge called after release started SELECT test_resowner_forget_between_phases(); ERROR: ResourceOwnerForget called for test resource after release started reset client_min_messages; +-- Reuse an auxiliary resource owner with and without a hash table. +-- For hash-table cases, force a resize before inserting into the reused hash. +-- This exposes stale entries without hanging while looking for a free slot. +SELECT test_resowner_aux(16); + test_resowner_aux +------------------- + +(1 row) + +SELECT test_resowner_aux(33); + test_resowner_aux +------------------- + +(1 row) + +SELECT test_resowner_aux(48); + test_resowner_aux +------------------- + +(1 row) + +SELECT test_resowner_aux(64); + test_resowner_aux +------------------- + +(1 row) + diff --git a/src/test/modules/test_resowner/sql/test_resowner.sql b/src/test/modules/test_resowner/sql/test_resowner.sql index 23284b7c8b9..d42b69f33db 100644 --- a/src/test/modules/test_resowner/sql/test_resowner.sql +++ b/src/test/modules/test_resowner/sql/test_resowner.sql @@ -23,3 +23,11 @@ set client_min_messages='warning'; -- order between ERROR and NOTICE varies SELECT test_resowner_remember_between_phases(); SELECT test_resowner_forget_between_phases(); reset client_min_messages; + +-- Reuse an auxiliary resource owner with and without a hash table. +-- For hash-table cases, force a resize before inserting into the reused hash. +-- This exposes stale entries without hanging while looking for a free slot. +SELECT test_resowner_aux(16); +SELECT test_resowner_aux(33); +SELECT test_resowner_aux(48); +SELECT test_resowner_aux(64); diff --git a/src/test/modules/test_resowner/test_resowner--1.0.sql b/src/test/modules/test_resowner/test_resowner--1.0.sql index 26fed7a010c..48dc6ed5d2c 100644 --- a/src/test/modules/test_resowner/test_resowner--1.0.sql +++ b/src/test/modules/test_resowner/test_resowner--1.0.sql @@ -19,6 +19,10 @@ CREATE FUNCTION test_resowner_forget_between_phases() RETURNS pg_catalog.void AS 'MODULE_PATHNAME' LANGUAGE C; +CREATE FUNCTION test_resowner_aux(nresources pg_catalog.int4) + RETURNS pg_catalog.void + AS 'MODULE_PATHNAME' LANGUAGE C; + CREATE FUNCTION test_resowner_many( nkinds pg_catalog.int4, nremember_bl pg_catalog.int4, diff --git a/src/test/modules/test_resowner/test_resowner_basic.c b/src/test/modules/test_resowner/test_resowner_basic.c index c0891f7e539..fca9fb246b7 100644 --- a/src/test/modules/test_resowner/test_resowner_basic.c +++ b/src/test/modules/test_resowner/test_resowner_basic.c @@ -19,6 +19,7 @@ PG_MODULE_MAGIC; static void ReleaseString(Datum res); static char *PrintString(Datum res); +static void CountRelease(Datum res); /* * A resource that tracks strings and prints the string when it's released. @@ -207,3 +208,89 @@ test_resowner_forget_between_phases(PG_FUNCTION_ARGS) PG_RETURN_VOID(); } + +static void +CountRelease(Datum res) +{ + int *nreleased = (int *) DatumGetPointer(res); + + (*nreleased)++; +} + +/* + * Check that an auxiliary resource owner can be reused after bulk release. + */ +PG_FUNCTION_INFO_V1(test_resowner_aux); +Datum +test_resowner_aux(PG_FUNCTION_ARGS) +{ + static const ResourceOwnerDesc before_desc = { + .name = "test resource before locks", + .release_phase = RESOURCE_RELEASE_BEFORE_LOCKS, + .release_priority = RELEASE_PRIO_FIRST, + .ReleaseResource = CountRelease + }; + static const ResourceOwnerDesc after_desc = { + .name = "test resource after locks", + .release_phase = RESOURCE_RELEASE_AFTER_LOCKS, + .release_priority = RELEASE_PRIO_FIRST, + .ReleaseResource = CountRelease + }; + int32 nresources = PG_GETARG_INT32(0); + int *nreleased[2]; + ResourceOwner resowner; + ResourceOwner save_current = CurrentResourceOwner; + ResourceOwner save_aux = AuxProcessResourceOwner; + + if (nresources <= 0) + elog(ERROR, "nresources must be greater than zero"); + + nreleased[0] = palloc0_array(int, nresources); + nreleased[1] = palloc0_array(int, nresources); + resowner = ResourceOwnerCreate(CurrentResourceOwner, "test auxiliary owner"); + + /* + * Use an isolated owner without registering an auxiliary-process exit + * hook. + */ + PG_TRY(); + { + CurrentResourceOwner = AuxProcessResourceOwner = resowner; + for (int round = 0; round < 2; round++) + { + for (int i = 0; i < nresources; i++) + { + const ResourceOwnerDesc *kind = i % 2 == 0 ? &before_desc : &after_desc; + + ResourceOwnerEnlarge(resowner); + ResourceOwnerRemember(resowner, + PointerGetDatum(&nreleased[round][i]), kind); + } + ReleaseAuxProcessResources(false); + } + } + PG_FINALLY(); + { + CurrentResourceOwner = save_current; + AuxProcessResourceOwner = save_aux; + } + PG_END_TRY(); + + ResourceOwnerDelete(resowner); + + /* + * Keep both rounds' counters alive to detect releases of stale entries. + */ + for (int round = 0; round < 2; round++) + { + for (int i = 0; i < nresources; i++) + { + if (nreleased[round][i] != 1) + elog(ERROR, "resource %d in round %d was released %d times", + i, round, nreleased[round][i]); + } + pfree(nreleased[round]); + } + + PG_RETURN_VOID(); +} base-commit: cea96cdfecbf40aa3f12c1c2572c191f27c318d0 -- 2.43.7