Date: Wed, 17 Sep 2025 10:31:00 -0500
From: Nathan Bossart <nathandbossart@gmail.com>
To: Sami Imseih <samimseih@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: GetNamedLWLockTranche crashes on Windows in normal backend
Message-ID: <aMrUNHKiUUo3RaYV@nathan>
References: 
 <CAA5RZ0uWH1owL9NG6GkbvVh_CjFN5nvzExoSddDLWEJxRBNimA@mail.gmail.com>
 <aLo-UaLpNwROh8uJ@nathan>
 <aL8KcsrCdPhDBmps@nathan>
 <CAA5RZ0vpr0P2rbA=_K0_SCHM7bmfVX4wEO9FAyopN1eWCYORhA@mail.gmail.com>
 <aL81qInOS3GQWwBm@nathan>
 <CAA5RZ0s_hbrLBmRDydii6Zfpo-dQFkVjp91V0yrgUbTfpC+KTg@mail.gmail.com>
 <aMM8iD77vkXLMuB7@nathan>
 <aMm8GwQ2jNl2LiR5@nathan>
 <aMm-eU7Ur1sJYIBt@nathan>
 <CAA5RZ0uaKtz7P=po9uqx=V=h6WofMJyp9O2UzpeDw98GNxbS-w@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="hTu8Qy/GtaDsumby"
Content-Disposition: inline
In-Reply-To: 
 <CAA5RZ0uaKtz7P=po9uqx=V=h6WofMJyp9O2UzpeDw98GNxbS-w@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/aMrUNHKiUUo3RaYV%40nathan>
Precedence: bulk


--hTu8Qy/GtaDsumby
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Tue, Sep 16, 2025 at 04:04:39PM -0500, Sami Imseih wrote:
> startup tranches should be:
> 
> #define NUM_STARTUP_TRANCHES (2)
> 
> instead of:
> 
> #define NUM_STARTUP_TRANCHES (256 - 2)

Why?

> I do think we should add tests for LWLockInitialize to test error for
> an unregistered tranche.

I added an initialization test.

-- 
nathan

--hTu8Qy/GtaDsumby
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=v7-0001-test_lwlock_tranches.patch

From 7fd394355c0ee19d00247a95554040ae35c4cacb Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Tue, 16 Sep 2025 10:15:11 -0500
Subject: [PATCH v7] 1/1] test_lwlock_tranches

---
 src/test/modules/Makefile                     |   1 +
 src/test/modules/meson.build                  |   1 +
 .../modules/test_lwlock_tranches/.gitignore   |   4 +
 .../modules/test_lwlock_tranches/Makefile     |  25 ++++
 .../expected/test_lwlock_tranches.out         |  32 +++++
 .../modules/test_lwlock_tranches/meson.build  |  35 ++++++
 .../sql/test_lwlock_tranches.sql              |  15 +++
 .../test_lwlock_tranches--1.0.sql             |  16 +++
 .../test_lwlock_tranches.c                    | 110 ++++++++++++++++++
 .../test_lwlock_tranches.conf                 |   1 +
 .../test_lwlock_tranches.control              |   4 +
 11 files changed, 244 insertions(+)
 create mode 100644 src/test/modules/test_lwlock_tranches/.gitignore
 create mode 100644 src/test/modules/test_lwlock_tranches/Makefile
 create mode 100644 src/test/modules/test_lwlock_tranches/expected/test_lwlock_tranches.out
 create mode 100644 src/test/modules/test_lwlock_tranches/meson.build
 create mode 100644 src/test/modules/test_lwlock_tranches/sql/test_lwlock_tranches.sql
 create mode 100644 src/test/modules/test_lwlock_tranches/test_lwlock_tranches--1.0.sql
 create mode 100644 src/test/modules/test_lwlock_tranches/test_lwlock_tranches.c
 create mode 100644 src/test/modules/test_lwlock_tranches/test_lwlock_tranches.conf
 create mode 100644 src/test/modules/test_lwlock_tranches/test_lwlock_tranches.control

diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile
index 903a8ac151a..8a3cd2afab7 100644
--- a/src/test/modules/Makefile
+++ b/src/test/modules/Makefile
@@ -29,6 +29,7 @@ SUBDIRS = \
 		  test_integerset \
 		  test_json_parser \
 		  test_lfind \
+		  test_lwlock_tranches \
 		  test_misc \
 		  test_oat_hooks \
 		  test_parser \
diff --git a/src/test/modules/meson.build b/src/test/modules/meson.build
index 93be0f57289..717e85066ba 100644
--- a/src/test/modules/meson.build
+++ b/src/test/modules/meson.build
@@ -28,6 +28,7 @@ subdir('test_int128')
 subdir('test_integerset')
 subdir('test_json_parser')
 subdir('test_lfind')
+subdir('test_lwlock_tranches')
 subdir('test_misc')
 subdir('test_oat_hooks')
 subdir('test_parser')
diff --git a/src/test/modules/test_lwlock_tranches/.gitignore b/src/test/modules/test_lwlock_tranches/.gitignore
new file mode 100644
index 00000000000..5dcb3ff9723
--- /dev/null
+++ b/src/test/modules/test_lwlock_tranches/.gitignore
@@ -0,0 +1,4 @@
+# Generated subdirectories
+/log/
+/results/
+/tmp_check/
diff --git a/src/test/modules/test_lwlock_tranches/Makefile b/src/test/modules/test_lwlock_tranches/Makefile
new file mode 100644
index 00000000000..e357b7d6d66
--- /dev/null
+++ b/src/test/modules/test_lwlock_tranches/Makefile
@@ -0,0 +1,25 @@
+# src/test/modules/test_lwlock_tranches/Makefile
+
+MODULE_big = test_lwlock_tranches
+OBJS = \
+	$(WIN32RES) \
+	test_lwlock_tranches.o
+PGFILEDESC = "test_lwlock_tranches - test code for LWLock tranches allocated by extensions"
+
+EXTENSION = test_lwlock_tranches
+DATA = test_lwlock_tranches--1.0.sql
+
+REGRESS_OPTS = --temp-config $(top_srcdir)/src/test/modules/test_lwlock_tranches/test_lwlock_tranches.conf
+REGRESS = test_lwlock_tranches
+NO_INSTALLCHECK = 1
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = src/test/modules/test_lwlock_tranches
+top_builddir = ../../../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
diff --git a/src/test/modules/test_lwlock_tranches/expected/test_lwlock_tranches.out b/src/test/modules/test_lwlock_tranches/expected/test_lwlock_tranches.out
new file mode 100644
index 00000000000..38c227b5bc3
--- /dev/null
+++ b/src/test/modules/test_lwlock_tranches/expected/test_lwlock_tranches.out
@@ -0,0 +1,32 @@
+CREATE EXTENSION test_lwlock_tranches;
+\c
+SELECT test_lwlock_tranches();
+ test_lwlock_tranches 
+----------------------
+ 
+(1 row)
+
+\c
+SELECT test_lwlock_tranche_creation(NULL);
+ERROR:  tranche name cannot be NULL
+\c
+SELECT test_lwlock_tranche_creation(repeat('a', 64));
+ERROR:  tranche name too long
+DETAIL:  LWLock tranche names must be no longer than 63 bytes.
+\c
+SELECT test_lwlock_tranche_creation('test');
+ERROR:  maximum number of tranches already registered
+DETAIL:  No more than 256 tranches may be registered.
+\c
+SELECT test_lwlock_tranche_lookup('test_lwlock_tranches_startup');
+ test_lwlock_tranche_lookup 
+----------------------------
+ 
+(1 row)
+
+\c
+SELECT test_lwlock_tranche_lookup('bogus');
+ERROR:  requested tranche is not registered
+\c
+SELECT test_lwlock_initialize(65535);
+ERROR:  tranche 65535 is not registered
diff --git a/src/test/modules/test_lwlock_tranches/meson.build b/src/test/modules/test_lwlock_tranches/meson.build
new file mode 100644
index 00000000000..755c2b72267
--- /dev/null
+++ b/src/test/modules/test_lwlock_tranches/meson.build
@@ -0,0 +1,35 @@
+# Copyright (c) 2025, PostgreSQL Global Development Group
+
+test_lwlock_tranches_sources = files(
+  'test_lwlock_tranches.c',
+)
+
+if host_system == 'windows'
+  test_lwlock_tranches_sources += rc_lib_gen.process(win32ver_rc, extra_args: [
+    '--NAME', 'test_lwlock_tranches',
+    '--FILEDESC', 'test_lwlock_tranches - test code for LWLock tranches allocated by extensions',])
+endif
+
+test_lwlock_tranches = shared_module('test_lwlock_tranches',
+  test_lwlock_tranches_sources,
+  kwargs: pg_test_mod_args,
+)
+test_install_libs += test_lwlock_tranches
+
+test_install_data += files(
+  'test_lwlock_tranches.control',
+  'test_lwlock_tranches--1.0.sql',
+)
+
+tests += {
+  'name': 'test_lwlock_tranches',
+  'sd': meson.current_source_dir(),
+  'bd': meson.current_build_dir(),
+  'regress': {
+    'sql': [
+      'test_lwlock_tranches',
+    ],
+    'regress_args': ['--temp-config', files('test_lwlock_tranches.conf')],
+    'runningcheck': false,
+  },
+}
diff --git a/src/test/modules/test_lwlock_tranches/sql/test_lwlock_tranches.sql b/src/test/modules/test_lwlock_tranches/sql/test_lwlock_tranches.sql
new file mode 100644
index 00000000000..18845343ebc
--- /dev/null
+++ b/src/test/modules/test_lwlock_tranches/sql/test_lwlock_tranches.sql
@@ -0,0 +1,15 @@
+CREATE EXTENSION test_lwlock_tranches;
+\c
+SELECT test_lwlock_tranches();
+\c
+SELECT test_lwlock_tranche_creation(NULL);
+\c
+SELECT test_lwlock_tranche_creation(repeat('a', 64));
+\c
+SELECT test_lwlock_tranche_creation('test');
+\c
+SELECT test_lwlock_tranche_lookup('test_lwlock_tranches_startup');
+\c
+SELECT test_lwlock_tranche_lookup('bogus');
+\c
+SELECT test_lwlock_initialize(65535);
diff --git a/src/test/modules/test_lwlock_tranches/test_lwlock_tranches--1.0.sql b/src/test/modules/test_lwlock_tranches/test_lwlock_tranches--1.0.sql
new file mode 100644
index 00000000000..34821981521
--- /dev/null
+++ b/src/test/modules/test_lwlock_tranches/test_lwlock_tranches--1.0.sql
@@ -0,0 +1,16 @@
+/* src/test/modules/test_lwlock_tranches/test_lwlock_tranches--1.0.sql */
+
+-- complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "CREATE EXTENSION test_lwlock_tranches" to load this file. \quit
+
+CREATE FUNCTION test_lwlock_tranches() RETURNS VOID
+	AS 'MODULE_PATHNAME' LANGUAGE C;
+
+CREATE FUNCTION test_lwlock_tranche_creation(tranche_name TEXT) RETURNS VOID
+	AS 'MODULE_PATHNAME' LANGUAGE C;
+
+CREATE FUNCTION test_lwlock_tranche_lookup(tranche_name TEXT) RETURNS VOID
+	AS 'MODULE_PATHNAME' LANGUAGE C;
+
+CREATE FUNCTION test_lwlock_initialize(tranche_id INT) RETURNS VOID
+	AS 'MODULE_PATHNAME' LANGUAGE C;
diff --git a/src/test/modules/test_lwlock_tranches/test_lwlock_tranches.c b/src/test/modules/test_lwlock_tranches/test_lwlock_tranches.c
new file mode 100644
index 00000000000..a52525b5b1e
--- /dev/null
+++ b/src/test/modules/test_lwlock_tranches/test_lwlock_tranches.c
@@ -0,0 +1,110 @@
+/*--------------------------------------------------------------------------
+ *
+ * test_lwlock_tranches.c
+ *		Test code for LWLock tranches allocated by extensions.
+ *
+ * Copyright (c) 2025, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ *		src/test/modules/test_lwlock_tranches/test_lwlock_tranches.c
+ *
+ * -------------------------------------------------------------------------
+ */
+
+#include "postgres.h"
+
+#include "fmgr.h"
+#include "miscadmin.h"
+#include "storage/lwlock.h"
+#include "utils/builtins.h"
+#include "utils/wait_classes.h"
+
+PG_MODULE_MAGIC;
+
+#define STARTUP_TRANCHE_NAME "test_lwlock_tranches_startup"
+#define DYNAMIC_TRANCHE_NAME "test_lwlock_tranches_dynamic"
+
+#define NUM_STARTUP_TRANCHES (256 - 2)
+#define NUM_DYNAMIC_TRANCHES (256 - NUM_STARTUP_TRANCHES)
+
+#define GET_TRANCHE_NAME(a) GetLWLockIdentifier(PG_WAIT_LWLOCK, (a))
+
+static shmem_request_hook_type prev_shmem_request_hook;
+static void test_lwlock_tranches_shmem_request(void);
+
+void
+_PG_init(void)
+{
+	prev_shmem_request_hook = shmem_request_hook;
+	shmem_request_hook = test_lwlock_tranches_shmem_request;
+}
+
+static void
+test_lwlock_tranches_shmem_request(void)
+{
+	if (prev_shmem_request_hook)
+		prev_shmem_request_hook();
+
+	for (int i = 0; i < NUM_STARTUP_TRANCHES; i++)
+		RequestNamedLWLockTranche(STARTUP_TRANCHE_NAME, 1);
+}
+
+PG_FUNCTION_INFO_V1(test_lwlock_tranches);
+Datum
+test_lwlock_tranches(PG_FUNCTION_ARGS)
+{
+	int			dynamic_tranches[NUM_DYNAMIC_TRANCHES];
+
+	for (int i = 0; i < NUM_DYNAMIC_TRANCHES; i++)
+		dynamic_tranches[i] = LWLockNewTrancheId(DYNAMIC_TRANCHE_NAME);
+
+	for (int i = 0; i < NUM_STARTUP_TRANCHES; i++)
+	{
+		if (strcmp(GET_TRANCHE_NAME(LWTRANCHE_FIRST_USER_DEFINED + i),
+				   STARTUP_TRANCHE_NAME) != 0)
+			elog(ERROR, "incorrect startup lock tranche name");
+	}
+
+	for (int i = 0; i < NUM_DYNAMIC_TRANCHES; i++)
+	{
+		if (strcmp(GET_TRANCHE_NAME(dynamic_tranches[i]),
+				   DYNAMIC_TRANCHE_NAME) != 0)
+			elog(ERROR, "incorrect dynamic lock tranche name");
+	}
+
+	PG_RETURN_VOID();
+}
+
+PG_FUNCTION_INFO_V1(test_lwlock_tranche_creation);
+Datum
+test_lwlock_tranche_creation(PG_FUNCTION_ARGS)
+{
+	char	   *tranche_name = PG_ARGISNULL(0) ? NULL : TextDatumGetCString(PG_GETARG_DATUM(0));
+
+	(void) LWLockNewTrancheId(tranche_name);
+
+	PG_RETURN_VOID();
+}
+
+PG_FUNCTION_INFO_V1(test_lwlock_tranche_lookup);
+Datum
+test_lwlock_tranche_lookup(PG_FUNCTION_ARGS)
+{
+	char	   *tranche_name = TextDatumGetCString(PG_GETARG_DATUM(0));
+
+	(void) GetNamedLWLockTranche(tranche_name);
+
+	PG_RETURN_VOID();
+}
+
+PG_FUNCTION_INFO_V1(test_lwlock_initialize);
+Datum
+test_lwlock_initialize(PG_FUNCTION_ARGS)
+{
+	int			tranche_id = PG_GETARG_INT32(0);
+	LWLock		lock;
+
+	LWLockInitialize(&lock, tranche_id);
+
+	PG_RETURN_VOID();
+}
diff --git a/src/test/modules/test_lwlock_tranches/test_lwlock_tranches.conf b/src/test/modules/test_lwlock_tranches/test_lwlock_tranches.conf
new file mode 100644
index 00000000000..acbe5bf51c3
--- /dev/null
+++ b/src/test/modules/test_lwlock_tranches/test_lwlock_tranches.conf
@@ -0,0 +1 @@
+shared_preload_libraries = 'test_lwlock_tranches'
diff --git a/src/test/modules/test_lwlock_tranches/test_lwlock_tranches.control b/src/test/modules/test_lwlock_tranches/test_lwlock_tranches.control
new file mode 100644
index 00000000000..1cde04bdc0b
--- /dev/null
+++ b/src/test/modules/test_lwlock_tranches/test_lwlock_tranches.control
@@ -0,0 +1,4 @@
+comment = 'Test code for LWLock tranches allocated by extensions'
+default_version = '1.0'
+module_pathname = '$libdir/test_lwlock_tranches'
+relocatable = true
-- 
2.39.5 (Apple Git-154)


--hTu8Qy/GtaDsumby--