From a4ee8ccbf8b7b4a2214084f52a7f980ac8d05f6a Mon Sep 17 00:00:00 2001 From: Bryan Green Date: Wed, 28 Oct 2026 16:35:42 -0500 Subject: [PATCH] Don't lose LWLock tranches allocated during shmem initialization shmem_slru_init() can allocate LWLock tranches for an SLRU before LWLockShmemInit() runs, and LWLockShmemInit() then resets the tranche counter, so those tranches are lost. The next tranche allocated gets one of their IDs and replaces its name, and looking up the other one fails with "tranche %d is not registered". Only test_slru does this in core, but leaving the IDs at zero is the only way an extension can define an SLRU. Initialize the tranche registry on first use instead, from whichever of LWLockNewTrancheId() and LWLockShmemInit() runs first. Add a test to test_slru that fails without the fix. --- src/backend/storage/lmgr/lwlock.c | 32 +++++++++++++++++-- .../modules/test_slru/expected/test_slru.out | 7 ++++ src/test/modules/test_slru/sql/test_slru.sql | 3 ++ src/test/modules/test_slru/test_slru--1.0.sql | 2 ++ src/test/modules/test_slru/test_slru.c | 18 +++++++++++ 5 files changed, 59 insertions(+), 3 deletions(-) diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c index 82a1d4d2e26..423c867d8ca 100644 --- a/src/backend/storage/lmgr/lwlock.c +++ b/src/backend/storage/lmgr/lwlock.c @@ -197,6 +197,9 @@ static LWLockTrancheShmemData *LWLockTranches; /* backend-local copy of LWLockTranches->num_user_defined */ static int LocalNumUserDefinedTranches; +/* is LWLockTranches initialized in this shmem segment? Postmaster only */ +static bool LWLockTranchesInitialized = false; + /* * NamedLWLockTrancheRequests is a list of tranches requested with * RequestNamedLWLockTranche(). It is only valid in the postmaster; after @@ -215,6 +218,7 @@ static int num_main_array_locks; static void LWLockShmemRequest(void *arg); static void LWLockShmemInit(void *arg); +static void InitLWLockTranches(void); const ShmemCallbacks LWLockCallbacks = { .request_fn = LWLockShmemRequest, @@ -422,6 +426,8 @@ LWLockShmemRequest(void *arg) .size = sizeof(LWLockTrancheShmemData), .ptr = (void **) &LWLockTranches, ); + if (!IsUnderPostmaster) + LWLockTranchesInitialized = false; /* Space for the LWLock array */ if (!IsUnderPostmaster) @@ -447,9 +453,7 @@ LWLockShmemInit(void *arg) int pos; /* Initialize the dynamic-allocation counter for tranches */ - LWLockTranches->num_user_defined = 0; - - SpinLockInit(&LWLockTranches->lock); + InitLWLockTranches(); /* * Allocate and initialize all LWLocks in the main array. It includes all @@ -499,6 +503,25 @@ LWLockShmemInit(void *arg) Assert(pos == num_main_array_locks); } +/* + * shmem_slru_init() can allocate tranches before LWLockShmemInit() runs, so + * whichever comes first initializes the registry. + */ +static void +InitLWLockTranches(void) +{ + Assert(!IsUnderPostmaster); + + if (LWLockTranchesInitialized) + return; + + LWLockTranches->num_user_defined = 0; + SpinLockInit(&LWLockTranches->lock); + LocalNumUserDefinedTranches = 0; + + LWLockTranchesInitialized = true; +} + /* * InitLWLockAccess - initialize backend-local state needed to hold LWLocks */ @@ -575,6 +598,9 @@ LWLockNewTrancheId(const char *name) errdetail("LWLock tranche names must be no longer than %d bytes.", NAMEDATALEN - 1))); + if (!IsUnderPostmaster) + InitLWLockTranches(); + /* The counter and the tranche names are protected by the spinlock */ SpinLockAcquire(&LWLockTranches->lock); diff --git a/src/test/modules/test_slru/expected/test_slru.out b/src/test/modules/test_slru/expected/test_slru.out index 7ae1b0d4a98..09a6fdab4da 100644 --- a/src/test/modules/test_slru/expected/test_slru.out +++ b/src/test/modules/test_slru/expected/test_slru.out @@ -1,4 +1,11 @@ CREATE EXTENSION test_slru; +-- Tranches assigned by slru.c must survive startup +SELECT test_slru_tranche_names(); + test_slru_tranche_names +------------------------------------------------- + TestSLRU buffer, TestSLRU bank, test_slru extra +(1 row) + SELECT test_slru_page_exists(12345); test_slru_page_exists ----------------------- diff --git a/src/test/modules/test_slru/sql/test_slru.sql b/src/test/modules/test_slru/sql/test_slru.sql index c6454c5bf82..3b4b0919f3b 100644 --- a/src/test/modules/test_slru/sql/test_slru.sql +++ b/src/test/modules/test_slru/sql/test_slru.sql @@ -1,5 +1,8 @@ CREATE EXTENSION test_slru; +-- Tranches assigned by slru.c must survive startup +SELECT test_slru_tranche_names(); + SELECT test_slru_page_exists(12345); SELECT test_slru_page_write(12345, 'Test SLRU'); SELECT test_slru_page_read(12345); diff --git a/src/test/modules/test_slru/test_slru--1.0.sql b/src/test/modules/test_slru/test_slru--1.0.sql index 2148e7204ec..1500ad0be02 100644 --- a/src/test/modules/test_slru/test_slru--1.0.sql +++ b/src/test/modules/test_slru/test_slru--1.0.sql @@ -19,6 +19,8 @@ CREATE OR REPLACE FUNCTION test_slru_page_truncate(bigint) RETURNS VOID AS 'MODULE_PATHNAME', 'test_slru_page_truncate' LANGUAGE C; CREATE OR REPLACE FUNCTION test_slru_delete_all() RETURNS VOID AS 'MODULE_PATHNAME', 'test_slru_delete_all' LANGUAGE C; +CREATE OR REPLACE FUNCTION test_slru_tranche_names() RETURNS text + AS 'MODULE_PATHNAME', 'test_slru_tranche_names' LANGUAGE C; CREATE OR REPLACE FUNCTION test_create_multixact() RETURNS xid diff --git a/src/test/modules/test_slru/test_slru.c b/src/test/modules/test_slru/test_slru.c index 40efffdbf62..45558b4e5c4 100644 --- a/src/test/modules/test_slru/test_slru.c +++ b/src/test/modules/test_slru/test_slru.c @@ -21,6 +21,7 @@ #include "storage/ipc.h" #include "storage/shmem.h" #include "utils/builtins.h" +#include "utils/wait_classes.h" PG_MODULE_MAGIC; @@ -36,6 +37,7 @@ PG_FUNCTION_INFO_V1(test_slru_page_sync); PG_FUNCTION_INFO_V1(test_slru_page_delete); PG_FUNCTION_INFO_V1(test_slru_page_truncate); PG_FUNCTION_INFO_V1(test_slru_delete_all); +PG_FUNCTION_INFO_V1(test_slru_tranche_names); /* Number of SLRU page slots */ #define NUM_TEST_BUFFERS 16 @@ -198,6 +200,22 @@ test_slru_delete_all(PG_FUNCTION_ARGS) PG_RETURN_VOID(); } +Datum +test_slru_tranche_names(PG_FUNCTION_ARGS) +{ + int buffer_tranche = TestSlruCtl->shared->buffer_locks[0].lock.tranche; + int bank_tranche = TestSlruCtl->shared->bank_locks[0].lock.tranche; + int new_tranche; + + /* must not reuse the IDs slru.c assigned at startup */ + new_tranche = LWLockNewTrancheId("test_slru extra"); + + PG_RETURN_TEXT_P(cstring_to_text(psprintf("%s, %s, %s", + GetLWLockIdentifier(PG_WAIT_LWLOCK, buffer_tranche), + GetLWLockIdentifier(PG_WAIT_LWLOCK, bank_tranche), + GetLWLockIdentifier(PG_WAIT_LWLOCK, new_tranche)))); +} + static bool test_slru_page_precedes_logically(int64 page1, int64 page2) { -- 2.54.0.windows.1