pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
GetNamedLWLockTranche crashes on Windows in normal backend
33+ messages / 3 participants
[nested] [flat]

Commits from this thread:
  2025-09-18 c3cc2ab87d  Fix re-initialization of LWLock-related shared memory.
  2025-09-11 ed1aad15e0  Move named LWLock tranche requests to shared memory.
* GetNamedLWLockTranche crashes on Windows in normal backend
@ 2025-08-22 19:21  Sami Imseih <samimseih@gmail.com>
  0 siblings, 1 reply; 33+ messages in thread

From: Sami Imseih @ 2025-08-22 19:21 UTC (permalink / raw)
  To: pgsql-hackers

Hi,

While working on [0], I observed that $SUBJECT. I encountered this issue
while building test cases for [0], and in which GetNamedLWLockTranche is
called outside of startup.

This crash, reproducible on HEAD, occurs because NamedLWLockTrancheRequestArray
( a local array populated during shmem_request ) is not copied over in
exec:ed backends.
This is reproducible on all EXEC_BACKEND environments.
On Linux, this is not an issue because of the magic of fork.

Since this data is not in shared memory, it will need much extra handling
in launch_backend.c, if at all safe, to get it copied. I am not too
familiar with
EXEC_BACKEND, so I could be wrong.
NamedLWLockTrancheRequests, which tracks the length of
NamedLWLockTrancheRequestArray is copied successfully.

Now, I wonder if the correct solution is just to allow GetNamedLWLockTranche
to be called ONLY when !IsUnderPostmaster, and error out if that's not
the case.
The docs [1] for the function mention it should be used during startup, and I
can't envision a reason one would want to use it outside.

We can also create a new global bool for shmem_startup progress, similar to
process_shmem_requests_in_progress, and ensure that this function is only
called at that time.

I repro'd this on a Windows machine, but one can also enable EXEC_BACKEND
in pg_config_manual.h and call GetNamedLWLockTranche by a normal backend.

I did this by injecting
```
GetNamedLWLockTranche("pg_stat_statements");
```
inside pgss_ExecutorEnd


[0] https://www.postgresql.org/message-id/CAA5RZ0vvED3naph8My8Szv6DL4AxOVK3eTPS0qXsaKi%3DbVdW2A%40mail.g...
[1] https://www.postgresql.org/docs/current/xfunc-c.html#XFUNC-SHARED-ADDIN-AT-STARTUP

--
Sami Imseih
Amazon Web Services (AWS)





^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: GetNamedLWLockTranche crashes on Windows in normal backend
@ 2025-08-25 16:46  Nathan Bossart <nathandbossart@gmail.com>
  parent: Sami Imseih <samimseih@gmail.com>
  0 siblings, 1 reply; 33+ messages in thread

From: Nathan Bossart @ 2025-08-25 16:46 UTC (permalink / raw)
  To: Sami Imseih <samimseih@gmail.com>; +Cc: pgsql-hackers

On Fri, Aug 22, 2025 at 02:21:55PM -0500, Sami Imseih wrote:
> While working on [0], I observed that $SUBJECT. I encountered this issue
> while building test cases for [0], and in which GetNamedLWLockTranche is
> called outside of startup.
> 
> [...]
> 
> I repro'd this on a Windows machine, but one can also enable EXEC_BACKEND
> in pg_config_manual.h and call GetNamedLWLockTranche by a normal backend.
> 
> I did this by injecting
> ```
> GetNamedLWLockTranche("pg_stat_statements");
> ```
> inside pgss_ExecutorEnd

If this fails, why doesn't the call in pgss_shmem_startup() also fail?  Was
pg_stat_statements not loaded via shared_preload_libraries?  And is the
failure a segfault or a "requested tranche is not registered" error?

-- 
nathan





^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: GetNamedLWLockTranche crashes on Windows in normal backend
@ 2025-08-25 17:58  Sami Imseih <samimseih@gmail.com>
  parent: Nathan Bossart <nathandbossart@gmail.com>
  0 siblings, 1 reply; 33+ messages in thread

From: Sami Imseih @ 2025-08-25 17:58 UTC (permalink / raw)
  To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: pgsql-hackers

> On Fri, Aug 22, 2025 at 02:21:55PM -0500, Sami Imseih wrote:
> > While working on [0], I observed that $SUBJECT. I encountered this issue
> > while building test cases for [0], and in which GetNamedLWLockTranche is
> > called outside of startup.
> >
> > [...]
> >
> > I repro'd this on a Windows machine, but one can also enable EXEC_BACKEND
> > in pg_config_manual.h and call GetNamedLWLockTranche by a normal backend.
> >
> > I did this by injecting
> > ```
> > GetNamedLWLockTranche("pg_stat_statements");
> > ```
> > inside pgss_ExecutorEnd
>
> If this fails, why doesn't the call in pgss_shmem_startup() also fail?  Was
> pg_stat_statements not loaded via shared_preload_libraries?

because the array is valid in postmaster, but it's not for a normal backend,
since it's not getting copied.

Yes, pg_stat_statements was loaded via shared_preload_libraries. Nothing
was done out of the ordinary.

> And is the
> failure a segfault or a "requested tranche is not registered" error?

It's a segfault.

--
Sami





^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: GetNamedLWLockTranche crashes on Windows in normal backend
@ 2025-08-25 18:31  Nathan Bossart <nathandbossart@gmail.com>
  parent: Sami Imseih <samimseih@gmail.com>
  0 siblings, 1 reply; 33+ messages in thread

From: Nathan Bossart @ 2025-08-25 18:31 UTC (permalink / raw)
  To: Sami Imseih <samimseih@gmail.com>; +Cc: pgsql-hackers

On Mon, Aug 25, 2025 at 12:58:08PM -0500, Sami Imseih wrote:
>> If this fails, why doesn't the call in pgss_shmem_startup() also fail?  Was
>> pg_stat_statements not loaded via shared_preload_libraries?
> 
> because the array is valid in postmaster, but it's not for a normal backend,
> since it's not getting copied.
> 
> Yes, pg_stat_statements was loaded via shared_preload_libraries. Nothing
> was done out of the ordinary.
> 
>> And is the
>> failure a segfault or a "requested tranche is not registered" error?
> 
> It's a segfault.

Oh, I see what's happening.  We are never calling GetNamedLWLockTranche()
in backends because ShmemInitStruct() always sets *found.

I'd argue that calling GetNamedLWLockTranche() outside of an "if (!found)"
block in a shmem_startup_hook doesn't follow convention, but the docs have
this note:

	shmem_startup_hook provides a convenient place for the initialization
	code, but it is not strictly required that all such code be placed in
	this hook.

Like the inaccurate sentence immediately following this one [0], it was
added by commit 964152c.  IMHO we should adjust the documentation to match
reality.

[0] https://postgr.es/m/aJ9QIF3g2QQko9wq%40nathan

-- 
nathan





^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: GetNamedLWLockTranche crashes on Windows in normal backend
@ 2025-08-25 18:44  Sami Imseih <samimseih@gmail.com>
  parent: Nathan Bossart <nathandbossart@gmail.com>
  0 siblings, 2 replies; 33+ messages in thread

From: Sami Imseih @ 2025-08-25 18:44 UTC (permalink / raw)
  To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: pgsql-hackers

> On Mon, Aug 25, 2025 at 12:58:08PM -0500, Sami Imseih wrote:
> >> If this fails, why doesn't the call in pgss_shmem_startup() also fail?  Was
> >> pg_stat_statements not loaded via shared_preload_libraries?
> >
> > because the array is valid in postmaster, but it's not for a normal backend,
> > since it's not getting copied.
> >
> > Yes, pg_stat_statements was loaded via shared_preload_libraries. Nothing
> > was done out of the ordinary.
> >
> >> And is the
> >> failure a segfault or a "requested tranche is not registered" error?
> >
> > It's a segfault.
>
> Oh, I see what's happening.  We are never calling GetNamedLWLockTranche()
> in backends because ShmemInitStruct() always sets *found.
>
> I'd argue that calling GetNamedLWLockTranche() outside of an "if (!found)"
> block in a shmem_startup_hook doesn't follow convention, but the docs have
> this note:
>
>         shmem_startup_hook provides a convenient place for the initialization
>         code, but it is not strictly required that all such code be placed in
>         this hook.
>
> Like the inaccurate sentence immediately following this one [0], it was
> added by commit 964152c.  IMHO we should adjust the documentation to match
> reality.
>
> [0] https://postgr.es/m/aJ9QIF3g2QQko9wq%40nathan

Why not ERROR out completely if we are calling this function outside
of postmaster
startup? I don't think we should be satisfied with just a
documentation change, since this
leads to a segfault. We can do something like below at the top of
GetNamedLWLockTranche

```
if (IsUnderPostmaster)
     elog(ERROR, "GetNamedLWLockTranche can only be called during
postmaster startup");
```

Another approach is to just change GetNamedLWLockTranche to use
NamedLWLockTrancheArray since that is already copied in EXEC_BACKEND, and allow
GetNamedLWLockTranche to continue to be used outside of startup. In
this case, we
will need to add num_lwlocks field to NamedLWLockTrancheArray. This
might be better
to backpatch, since we will not be changing user facing behavior.

Attached is a repro patch. You will need to set
shared_preload_libraries = 'pg_stat_statements'
as well.


--
Sami

Attachments:

v1-0001-repro.patch (application/octet-stream, 1.4K) download From 81511510a8f390a28ab429766f9eb2635c5d8f6c Mon Sep 17 00:00:00 2001 From: Ubuntu <ubuntu@ip-172-31-41-11.ec2.internal> Date: Mon, 25 Aug 2025 18:22:11 +0000 Subject: [PATCH v1 1/2] repro --- contrib/pg_stat_statements/pg_stat_statements.c | 2 +- src/include/pg_config_manual.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c index 1cb368c8590..42c7379e4a5 100644 --- a/contrib/pg_stat_statements/pg_stat_statements.c +++ b/contrib/pg_stat_statements/pg_stat_statements.c @@ -1076,7 +1076,7 @@ static void pgss_ExecutorEnd(QueryDesc *queryDesc) { int64 queryId = queryDesc->plannedstmt->queryId; - + GetNamedLWLockTranche("pg_stat_statements"); if (queryId != INT64CONST(0) && queryDesc->totaltime && pgss_enabled(nesting_level)) { diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h index 7e1aa422332..763239e7829 100644 --- a/src/include/pg_config_manual.h +++ b/src/include/pg_config_manual.h @@ -117,9 +117,9 @@ * fork()). On other platforms, it's only useful for verifying those * otherwise Windows-specific code paths. */ -#if defined(WIN32) && !defined(__CYGWIN__) +//#if defined(WIN32) && !defined(__CYGWIN__) #define EXEC_BACKEND -#endif +//#endif /* * USE_POSIX_FADVISE controls whether Postgres will attempt to use the -- 2.43.0
^ permalink raw reply [nested|flat] 33+ messages in thread

* Re: GetNamedLWLockTranche crashes on Windows in normal backend
@ 2025-08-25 18:51  Sami Imseih <samimseih@gmail.com>
  parent: Sami Imseih <samimseih@gmail.com>
  1 sibling, 0 replies; 33+ messages in thread

From: Sami Imseih @ 2025-08-25 18:51 UTC (permalink / raw)
  To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: pgsql-hackers

> Attached is a repro patch. You will need to set
> shared_preload_libraries = 'pg_stat_statements'
> as well.

I forgot to mention. Once patched, you can run a simple
select statement from a new connection to force the crash.
This is because GetNamedLWLockTranche will be called
at pgss_ExecutorEnd.

--
Sami





^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: GetNamedLWLockTranche crashes on Windows in normal backend
@ 2025-08-25 19:02  Nathan Bossart <nathandbossart@gmail.com>
  parent: Sami Imseih <samimseih@gmail.com>
  1 sibling, 1 reply; 33+ messages in thread

From: Nathan Bossart @ 2025-08-25 19:02 UTC (permalink / raw)
  To: Sami Imseih <samimseih@gmail.com>; +Cc: pgsql-hackers

On Mon, Aug 25, 2025 at 01:44:22PM -0500, Sami Imseih wrote:
> Another approach is to just change GetNamedLWLockTranche to use
> NamedLWLockTrancheArray since that is already copied in EXEC_BACKEND, and
> allow GetNamedLWLockTranche to continue to be used outside of startup. In
> this case, we will need to add num_lwlocks field to
> NamedLWLockTrancheArray. This might be better to backpatch, since we will
> not be changing user facing behavior.

That seems like a reasonable thing to try.  It looks like
NamedLWLockTrancheRequests is copied, too.  Could we used that instead of
adding a new variable?

My guess is that this has been broken since this code was introduced in
commit c1772ad from 2016/v9.6, and AFAIK you are the first to report it, so
I don't feel a tremendous amount of urgency to fix it on the back-branches.

-- 
nathan





^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: GetNamedLWLockTranche crashes on Windows in normal backend
@ 2025-08-25 19:34  Sami Imseih <samimseih@gmail.com>
  parent: Nathan Bossart <nathandbossart@gmail.com>
  0 siblings, 1 reply; 33+ messages in thread

From: Sami Imseih @ 2025-08-25 19:34 UTC (permalink / raw)
  To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: pgsql-hackers

> > Another approach is to just change GetNamedLWLockTranche to use
> > NamedLWLockTrancheArray since that is already copied in EXEC_BACKEND, and
> > allow GetNamedLWLockTranche to continue to be used outside of startup. In
> > this case, we will need to add num_lwlocks field to
> > NamedLWLockTrancheArray. This might be better to backpatch, since we will
> > not be changing user facing behavior.
>
> That seems like a reasonable thing to try.  It looks like
> NamedLWLockTrancheRequests is copied, too.  Could we used that instead of
> adding a new variable?

We would not need a new variable. Currently it's assumed that
NamedLWLockTrancheRequests will be the size of both
NamedLWLockTrancheRequests and NamedLWLockTrancheArray.

```
/*
* NamedLWLockTrancheRequests is both the valid length of the request array,
* and the length of the shared-memory NamedLWLockTrancheArray later on.
* This variable and NamedLWLockTrancheArray are non-static so that
* postmaster.c can copy them to child processes in EXEC_BACKEND builds.
*/
int NamedLWLockTrancheRequests = 0;
```

> My guess is that this has been broken since this code was introduced in
> commit c1772ad from 2016/v9.6, and AFAIK you are the first to report it, so
> I don't feel a tremendous amount of urgency to fix it on the back-branches.

Yeah, I was doing something for testing that is not likely to be done in
the real-world. Also, considering this is an EXEC_BACKEND case only,
it makes it even less likely.

--
Sami





^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: GetNamedLWLockTranche crashes on Windows in normal backend
@ 2025-08-25 21:28  Sami Imseih <samimseih@gmail.com>
  parent: Sami Imseih <samimseih@gmail.com>
  0 siblings, 1 reply; 33+ messages in thread

From: Sami Imseih @ 2025-08-25 21:28 UTC (permalink / raw)
  To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: pgsql-hackers

Here it is.

I was hoping we can even get rid of NamedLWLockTrancheRequests
altogether, but that's not going to be possible because
RequestNamedLWLockTranche happens earlier than
CreateLWLocks, so NamedLWLockTrancheRequests is essentially
tracking the requested lwlocks until we get a chance to create and
initialize the LWLocks and store them in the shared memory,
NamedLWLockTrancheArray.

--
Sami

Attachments:

0001-Fix-EXEC_BACKEND-segfault-on-NamedLWLockTrancheReque.patch (application/octet-stream, 2.6K) download From 60f6ab331e436ece58b52abab8a96f60cadcf197 Mon Sep 17 00:00:00 2001 From: Ubuntu <ubuntu@ip-172-31-41-11.ec2.internal> Date: Mon, 25 Aug 2025 21:12:22 +0000 Subject: [PATCH 1/1] Fix EXEC_BACKEND segfault on NamedLWLockTrancheRequestArray access Unlike fork(), EXEC_BACKEND did not pass NamedLWLockTrancheRequestArray down to backends. This caused crashes if backends accessed the array, such as in GetNamedLWLockTranche. This patch fixes the issue by adding a num_lwlocks field to the shared memory NamedLWLockTrancheArray, with the postmaster copying the value from NamedLWLockTrancheRequestArray. GetNamedLWLockTranche now looks up the LWLock from NamedLWLockTrancheArray instead. NamedLWLockTrancheArray already points to shared memory and is passed down to backends in launch_backend.c. NamedLWLockTrancheRequests still acts as the counter for both arrays and is already handled in EXEC_BACKEND, so no further changes are needed there. This issue was discovered during testing of nearby code. No reports have been seen in the field, so backpatching is not required. --- src/backend/storage/lmgr/lwlock.c | 5 +++-- src/include/storage/lwlock.h | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c index c80b43f1f55..2e7392b633a 100644 --- a/src/backend/storage/lmgr/lwlock.c +++ b/src/backend/storage/lmgr/lwlock.c @@ -514,6 +514,7 @@ InitializeLWLocks(void) strcpy(name, request->tranche_name); tranche->trancheId = LWLockNewTrancheId(); tranche->trancheName = name; + tranche->num_lwlocks = request->num_lwlocks; for (j = 0; j < request->num_lwlocks; j++, lock++) LWLockInitialize(&lock->lock, tranche->trancheId); @@ -554,11 +555,11 @@ GetNamedLWLockTranche(const char *tranche_name) lock_pos = NUM_FIXED_LWLOCKS; for (i = 0; i < NamedLWLockTrancheRequests; i++) { - if (strcmp(NamedLWLockTrancheRequestArray[i].tranche_name, + if (strcmp(NamedLWLockTrancheArray[i].trancheName, tranche_name) == 0) return &MainLWLockArray[lock_pos]; - lock_pos += NamedLWLockTrancheRequestArray[i].num_lwlocks; + lock_pos += NamedLWLockTrancheArray[i].num_lwlocks; } elog(ERROR, "requested tranche is not registered"); diff --git a/src/include/storage/lwlock.h b/src/include/storage/lwlock.h index 5e717765764..074ac87cbf4 100644 --- a/src/include/storage/lwlock.h +++ b/src/include/storage/lwlock.h @@ -78,6 +78,7 @@ typedef struct NamedLWLockTranche { int trancheId; char *trancheName; + int num_lwlocks; } NamedLWLockTranche; extern PGDLLIMPORT NamedLWLockTranche *NamedLWLockTrancheArray; -- 2.43.0
^ permalink raw reply [nested|flat] 33+ messages in thread

* Re: GetNamedLWLockTranche crashes on Windows in normal backend
@ 2025-08-25 21:41  Nathan Bossart <nathandbossart@gmail.com>
  parent: Sami Imseih <samimseih@gmail.com>
  0 siblings, 1 reply; 33+ messages in thread

From: Nathan Bossart @ 2025-08-25 21:41 UTC (permalink / raw)
  To: Sami Imseih <samimseih@gmail.com>; +Cc: pgsql-hackers

On Mon, Aug 25, 2025 at 04:28:09PM -0500, Sami Imseih wrote:
> Here it is.

Looks reasonable to me.  I'll leave this one around for a week or two to
give others a chance to comment.

-- 
nathan





^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: GetNamedLWLockTranche crashes on Windows in normal backend
@ 2025-09-03 21:36  Nathan Bossart <nathandbossart@gmail.com>
  parent: Nathan Bossart <nathandbossart@gmail.com>
  0 siblings, 1 reply; 33+ messages in thread

From: Nathan Bossart @ 2025-09-03 21:36 UTC (permalink / raw)
  To: Sami Imseih <samimseih@gmail.com>; +Cc: pgsql-hackers

Well, commit 38b602b certainly doesn't do us any favors here since it
removed NamedLWLockTrancheArray.  Given the lack of reports from the field,
I suspect the best path forward is to add an ERROR for unsafe accesses and
to fix the documentation, as discussed upthread.

-- 
nathan





^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: GetNamedLWLockTranche crashes on Windows in normal backend
@ 2025-09-04 18:23  Sami Imseih <samimseih@gmail.com>
  parent: Nathan Bossart <nathandbossart@gmail.com>
  0 siblings, 1 reply; 33+ messages in thread

From: Sami Imseih @ 2025-09-04 18:23 UTC (permalink / raw)
  To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: pgsql-hackers

commit 38b602b certainly doesn't do us any favors here since it
> removed NamedLWLockTrancheArray.  Given the lack of reports from the field,
> I suspect the best path forward is to add an ERROR for unsafe accesses and
> to fix the documentation, as discussed upthread.

I suggest we set an in progres flag for shmem_startup_hook, as we do for
shmem_request_hook, and error out this function if called outside of this
hook. The error will be something like this:

```
ERROR:  cannot return the address of LWLocks outside of shmem_startup_hook
```

We can also update the docs.

If you agree with the above, I'll get the patch ready?

--
Sami Imseih
Amazon Web Services (AWS)





^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: GetNamedLWLockTranche crashes on Windows in normal backend
@ 2025-09-04 18:41  Nathan Bossart <nathandbossart@gmail.com>
  parent: Sami Imseih <samimseih@gmail.com>
  0 siblings, 1 reply; 33+ messages in thread

From: Nathan Bossart @ 2025-09-04 18:41 UTC (permalink / raw)
  To: Sami Imseih <samimseih@gmail.com>; +Cc: pgsql-hackers

On Thu, Sep 04, 2025 at 01:23:26PM -0500, Sami Imseih wrote:
> I suggest we set an in progres flag for shmem_startup_hook, as we do for
> shmem_request_hook, and error out this function if called outside of this
> hook. The error will be something like this:
> 
> ```
> ERROR:  cannot return the address of LWLocks outside of shmem_startup_hook
> ```
> 
> We can also update the docs.
> 
> If you agree with the above, I'll get the patch ready?

Yeah, I think modeling this after commit 4f2400c is a reasonable thing to
explore.

-- 
nathan





^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: GetNamedLWLockTranche crashes on Windows in normal backend
@ 2025-09-04 21:12  Sami Imseih <samimseih@gmail.com>
  parent: Nathan Bossart <nathandbossart@gmail.com>
  0 siblings, 1 reply; 33+ messages in thread

From: Sami Imseih @ 2025-09-04 21:12 UTC (permalink / raw)
  To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: pgsql-hackers

> Yeah, I think modeling this after commit 4f2400c is a reasonable thing to
> explore.

Here it is as described above.

--
Sami

Attachments:

v2-0001-Prevent-crash-when-retreiving-LWLock-tranches.patch (application/octet-stream, 5.0K) download From 00bc0ef20633bffc066f850d1a553108cebcda4a Mon Sep 17 00:00:00 2001 From: Ubuntu <ubuntu@ip-172-31-46-230.ec2.internal> Date: Thu, 4 Sep 2025 21:08:56 +0000 Subject: [PATCH v2 1/1] Prevent crash when retreiving LWLock tranches GetNamedLWLockTranche() can crash if called outside shmem_startup_hook(). Under EXEC_BACKEND, there is no provision to copy the NamedLWLockTrancheRequestArray, whereas fork() automatically copies it for a normal backend. Moreover, there is no valid use case for calling GetNamedLWLockTranche() outside shmem_startup_hook(). To address this, shmem startup is centralized in process_shmem_startup(), with process_shmem_startup_in_progress indicating whether we are inside this hook. This enforces that GetNamedLWLockTranche() is only called during the startup hook. Update documentation to reflect this behavior. Discussion: https://www.postgresql.org/message-id/flat/CAA5RZ0v1_15QPg5Sqd2Qz5rh_qcsyCeHHmRDY89xVHcy2yt5BQ@mail.gmail.com --- doc/src/sgml/xfunc.sgml | 3 ++- src/backend/storage/ipc/ipci.c | 16 ++++++++++++---- src/backend/storage/lmgr/lwlock.c | 9 +++++++++ src/include/storage/ipc.h | 2 ++ 4 files changed, 25 insertions(+), 5 deletions(-) diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml index da21ef56891..774d21c8734 100644 --- a/doc/src/sgml/xfunc.sgml +++ b/doc/src/sgml/xfunc.sgml @@ -3743,7 +3743,8 @@ void RequestNamedLWLockTranche(const char *tranche_name, int num_lwlocks) </programlisting> This ensures that an array of <literal>num_lwlocks</literal> LWLocks is available under the name <literal>tranche_name</literal>. A pointer to - this array can be obtained by calling: + this array can be obtained in the <literal>shmem_startup_hook</literal> + by calling: <programlisting> LWLockPadded *GetNamedLWLockTranche(const char *tranche_name) </programlisting> diff --git a/src/backend/storage/ipc/ipci.c b/src/backend/storage/ipc/ipci.c index 2fa045e6b0f..786c5f258c6 100644 --- a/src/backend/storage/ipc/ipci.c +++ b/src/backend/storage/ipc/ipci.c @@ -56,6 +56,7 @@ int shared_memory_type = DEFAULT_SHARED_MEMORY_TYPE; shmem_startup_hook_type shmem_startup_hook = NULL; +bool process_shmem_startup_in_progress = false; static Size total_addin_request = 0; @@ -187,8 +188,7 @@ AttachSharedMemoryStructs(void) /* * Now give loadable modules a chance to set up their shmem allocations */ - if (shmem_startup_hook) - shmem_startup_hook(); + process_shmem_startup(); } #endif @@ -243,8 +243,7 @@ CreateSharedMemoryAndSemaphores(void) /* * Now give loadable modules a chance to set up their shmem allocations */ - if (shmem_startup_hook) - shmem_startup_hook(); + process_shmem_startup(); } /* @@ -386,3 +385,12 @@ InitializeShmemGUCs(void) sprintf(buf, "%d", num_semas); SetConfigOption("num_os_semaphores", buf, PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT); } + +void +process_shmem_startup(void) +{ + process_shmem_startup_in_progress = true; + if (shmem_startup_hook) + shmem_startup_hook(); + process_shmem_startup_in_progress = false; +} diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c index 258cdebd0f5..5249e6bc509 100644 --- a/src/backend/storage/lmgr/lwlock.c +++ b/src/backend/storage/lmgr/lwlock.c @@ -80,6 +80,7 @@ #include "pg_trace.h" #include "pgstat.h" #include "port/pg_bitutils.h" +#include "storage/ipc.h" #include "storage/proc.h" #include "storage/proclist.h" #include "storage/procnumber.h" @@ -522,6 +523,11 @@ InitLWLockAccess(void) * Caller needs to retrieve the requested number of LWLocks starting from * the base lock address returned by this API. This can be used for * tranches that are requested by using RequestNamedLWLockTranche() API. + * + * Backends may not inherit NamedLWLockTrancheRequestArray, such as + * EXEC_BACKEND. It's not worth the effort to guarantee this, because + * extensions should only need to acquire a pointer to the LWLocks in the + * startup hook. */ LWLockPadded * GetNamedLWLockTranche(const char *tranche_name) @@ -529,6 +535,9 @@ GetNamedLWLockTranche(const char *tranche_name) int lock_pos; int i; + if (!process_shmem_startup_in_progress) + elog(ERROR, "cannot return the address of LWLocks outside of shmem_startup_hook"); + /* * Obtain the position of base address of LWLock belonging to requested * tranche_name in MainLWLockArray. LWLocks for named tranches are placed diff --git a/src/include/storage/ipc.h b/src/include/storage/ipc.h index 3baf418b3d1..3a3a1d311c3 100644 --- a/src/include/storage/ipc.h +++ b/src/include/storage/ipc.h @@ -76,9 +76,11 @@ extern void check_on_shmem_exit_lists_are_empty(void); /* ipci.c */ extern PGDLLIMPORT shmem_startup_hook_type shmem_startup_hook; +extern PGDLLIMPORT bool process_shmem_startup_in_progress; extern Size CalculateShmemSize(int *num_semaphores); extern void CreateSharedMemoryAndSemaphores(void); +extern void process_shmem_startup(void); #ifdef EXEC_BACKEND extern void AttachSharedMemoryStructs(void); #endif -- 2.43.0
^ permalink raw reply [nested|flat] 33+ messages in thread

* Re: GetNamedLWLockTranche crashes on Windows in normal backend
@ 2025-09-05 01:35  Nathan Bossart <nathandbossart@gmail.com>
  parent: Sami Imseih <samimseih@gmail.com>
  0 siblings, 1 reply; 33+ messages in thread

From: Nathan Bossart @ 2025-09-05 01:35 UTC (permalink / raw)
  To: Sami Imseih <samimseih@gmail.com>; +Cc: pgsql-hackers

On Thu, Sep 04, 2025 at 04:12:09PM -0500, Sami Imseih wrote:
>> Yeah, I think modeling this after commit 4f2400c is a reasonable thing to
>> explore.
> 
> Here it is as described above.

Thanks.  This looks like the right idea to me, but let's give some time for
others to comment.

-- 
nathan





^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: GetNamedLWLockTranche crashes on Windows in normal backend
@ 2025-09-08 16:55  Nathan Bossart <nathandbossart@gmail.com>
  parent: Nathan Bossart <nathandbossart@gmail.com>
  0 siblings, 2 replies; 33+ messages in thread

From: Nathan Bossart @ 2025-09-08 16:55 UTC (permalink / raw)
  To: Sami Imseih <samimseih@gmail.com>; +Cc: pgsql-hackers

On Thu, Sep 04, 2025 at 08:35:13PM -0500, Nathan Bossart wrote:
> On Thu, Sep 04, 2025 at 04:12:09PM -0500, Sami Imseih wrote:
>>> Yeah, I think modeling this after commit 4f2400c is a reasonable thing to
>>> explore.
>> 
>> Here it is as described above.
> 
> Thanks.  This looks like the right idea to me, but let's give some time for
> others to comment.

I've started preparing this for commit, and I realized that restricting
GetNamedLWLockTranche() to shmem_startup_hook is not sufficient.
EXEC_BACKEND builds will run this hook in every backend, so unless it's
guarded behind some sort of "if (!found)" condition (i.e., only run in the
postmaster), it'll still crash.  I think we just need to add some extra
notes to the docs and check for !IsUnderPostmaster, as discussed upthread.

-- 
nathan





^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: GetNamedLWLockTranche crashes on Windows in normal backend
@ 2025-09-08 18:24  Nathan Bossart <nathandbossart@gmail.com>
  parent: Nathan Bossart <nathandbossart@gmail.com>
  1 sibling, 0 replies; 33+ messages in thread

From: Nathan Bossart @ 2025-09-08 18:24 UTC (permalink / raw)
  To: Sami Imseih <samimseih@gmail.com>; +Cc: pgsql-hackers

On Mon, Sep 08, 2025 at 11:55:14AM -0500, Nathan Bossart wrote:
> I've started preparing this for commit, and I realized that restricting
> GetNamedLWLockTranche() to shmem_startup_hook is not sufficient.
> EXEC_BACKEND builds will run this hook in every backend, so unless it's
> guarded behind some sort of "if (!found)" condition (i.e., only run in the
> postmaster), it'll still crash.  I think we just need to add some extra
> notes to the docs and check for !IsUnderPostmaster, as discussed upthread.

Or... what if we just moved the request array to shared memory?

-- 
nathan

Attachments:

v3-0001-Move-named-LWLock-tranche-request-array-to-shared.patch (text/plain, 4.7K) download From 5abfc98ef5dd26e2f3c822cbe51c6038fb4e37bf Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nathan@postgresql.org> Date: Mon, 8 Sep 2025 13:18:59 -0500 Subject: [PATCH v3 1/1] Move named LWLock tranche request array to shared memory. --- src/backend/postmaster/launch_backend.c | 3 +++ src/backend/storage/lmgr/lwlock.c | 31 +++++++++++++++++++++---- src/include/storage/lwlock.h | 4 ++++ 3 files changed, 33 insertions(+), 5 deletions(-) diff --git a/src/backend/postmaster/launch_backend.c b/src/backend/postmaster/launch_backend.c index a38979c50e4..c5ef14e1eaa 100644 --- a/src/backend/postmaster/launch_backend.c +++ b/src/backend/postmaster/launch_backend.c @@ -101,6 +101,7 @@ typedef struct struct InjectionPointsCtl *ActiveInjectionPoints; #endif int NamedLWLockTrancheRequests; + NamedLWLockTrancheRequest *NamedLWLockTrancheRequestArray; char **LWLockTrancheNames; int *LWLockCounter; LWLockPadded *MainLWLockArray; @@ -761,6 +762,7 @@ save_backend_variables(BackendParameters *param, #endif param->NamedLWLockTrancheRequests = NamedLWLockTrancheRequests; + param->NamedLWLockTrancheRequestArray = NamedLWLockTrancheRequestArray; param->LWLockTrancheNames = LWLockTrancheNames; param->LWLockCounter = LWLockCounter; param->MainLWLockArray = MainLWLockArray; @@ -1022,6 +1024,7 @@ restore_backend_variables(BackendParameters *param) #endif NamedLWLockTrancheRequests = param->NamedLWLockTrancheRequests; + NamedLWLockTrancheRequestArray = param->NamedLWLockTrancheRequestArray; LWLockTrancheNames = param->LWLockTrancheNames; LWLockCounter = param->LWLockCounter; MainLWLockArray = param->MainLWLockArray; diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c index fcbac5213a5..46c82c63ca5 100644 --- a/src/backend/storage/lmgr/lwlock.c +++ b/src/backend/storage/lmgr/lwlock.c @@ -184,14 +184,13 @@ typedef struct NamedLWLockTrancheRequest int num_lwlocks; } NamedLWLockTrancheRequest; -static NamedLWLockTrancheRequest *NamedLWLockTrancheRequestArray = NULL; - /* - * NamedLWLockTrancheRequests is the valid length of the request array. This - * variable is non-static so that postmaster.c can copy them to child processes - * in EXEC_BACKEND builds. + * NamedLWLockTrancheRequests is the valid length of the request array. These + * variables are non-static so that launch_backend.c can copy them to child + * processes in EXEC_BACKEND builds. */ int NamedLWLockTrancheRequests = 0; +NamedLWLockTrancheRequest *NamedLWLockTrancheRequestArray = NULL; /* shared memory counter of registered tranches */ int *LWLockCounter = NULL; @@ -407,6 +406,14 @@ LWLockShmemSize(void) size = add_size(size, mul_size(MAX_NAMED_TRANCHES, sizeof(char *))); size = add_size(size, mul_size(MAX_NAMED_TRANCHES, NAMEDATALEN)); + /* + * Make space for named tranche requests. This is done for the benefit of + * EXEC_BACKEND builds, which otherwise wouldn't be able to call + * GetNamedLWLockTranche() outside postmaster. + */ + size = add_size(size, mul_size(NamedLWLockTrancheRequests, + sizeof(NamedLWLockTrancheRequest))); + /* Space for the LWLock array, plus room for cache line alignment. */ size = add_size(size, LWLOCK_PADDED_SIZE); size = add_size(size, mul_size(numLocks, sizeof(LWLockPadded))); @@ -443,6 +450,20 @@ CreateLWLocks(void) ptr += NAMEDATALEN; } + /* + * Move named tranche requests to shared memory. This is done for the + * benefit of EXEC_BACKEND builds, which otherwise wouldn't be able to + * call GetNamedLWLockTranche() outside postmaster. + */ + if (NamedLWLockTrancheRequests > 0) + { + memcpy(ptr, NamedLWLockTrancheRequestArray, + NamedLWLockTrancheRequests * sizeof(NamedLWLockTrancheRequest)); + pfree(NamedLWLockTrancheRequestArray); + NamedLWLockTrancheRequestArray = (NamedLWLockTrancheRequest *) ptr; + ptr += NamedLWLockTrancheRequests * sizeof(NamedLWLockTrancheRequest); + } + /* Ensure desired alignment of LWLock array */ ptr += LWLOCK_PADDED_SIZE - ((uintptr_t) ptr) % LWLOCK_PADDED_SIZE; MainLWLockArray = (LWLockPadded *) ptr; diff --git a/src/include/storage/lwlock.h b/src/include/storage/lwlock.h index 0e9cf81a4c7..8e0d0d233b4 100644 --- a/src/include/storage/lwlock.h +++ b/src/include/storage/lwlock.h @@ -73,8 +73,12 @@ typedef union LWLockPadded extern PGDLLIMPORT LWLockPadded *MainLWLockArray; +/* forward declaration of private type for use only by lwlock.c */ +typedef struct NamedLWLockTrancheRequest NamedLWLockTrancheRequest; + extern PGDLLIMPORT char **LWLockTrancheNames; extern PGDLLIMPORT int NamedLWLockTrancheRequests; +extern PGDLLIMPORT NamedLWLockTrancheRequest *NamedLWLockTrancheRequestArray; extern PGDLLIMPORT int *LWLockCounter; /* -- 2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 33+ messages in thread

* Re: GetNamedLWLockTranche crashes on Windows in normal backend
@ 2025-09-08 19:47  Sami Imseih <samimseih@gmail.com>
  parent: Nathan Bossart <nathandbossart@gmail.com>
  1 sibling, 1 reply; 33+ messages in thread

From: Sami Imseih @ 2025-09-08 19:47 UTC (permalink / raw)
  To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: pgsql-hackers

> On Thu, Sep 04, 2025 at 08:35:13PM -0500, Nathan Bossart wrote:
> > On Thu, Sep 04, 2025 at 04:12:09PM -0500, Sami Imseih wrote:
> >>> Yeah, I think modeling this after commit 4f2400c is a reasonable thing to
> >>> explore.
> >>
> >> Here it is as described above.
> >
> > Thanks.  This looks like the right idea to me, but let's give some time for
> > others to comment.
>
> I've started preparing this for commit, and I realized that restricting
> GetNamedLWLockTranche() to shmem_startup_hook is not sufficient.
> EXEC_BACKEND builds will run this hook in every backend, so unless it's
> guarded behind some sort of "if (!found)" condition (i.e., only run in the
> postmaster), it'll still crash.  I think we just need to add some extra
> notes to the docs and check for !IsUnderPostmaster, as discussed upthread.

I think v2 is fine because it is perfectly fine for a normal backend (
EXEC_BACKEND)
to call this function as long as it's processing the startup hook. The
goal is to prevent it
from being called outside of the startup hook.

>  Or... what if we just moved the request array to shared memory?

I guess that works also, if we want to maintain the existing behavior.
I am OK with this
as well, and I don't see anything wrong with v3.

FWIW, I got the tests discussed in [0] commit ready and also included
tests for this
crash.

Attached is the patch with the tests. v4-0001 and v3-0001 are identical.
v4-0002 includes the tests.

I think we should commit these tests as well. If  you think the tests should
be a separate thread, let me know.

--
Sami

[0] https://www.postgresql.org/message-id/aLcPbyWLawp5_rdt%40nathan





^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: GetNamedLWLockTranche crashes on Windows in normal backend
@ 2025-09-08 19:59  Nathan Bossart <nathandbossart@gmail.com>
  parent: Sami Imseih <samimseih@gmail.com>
  0 siblings, 1 reply; 33+ messages in thread

From: Nathan Bossart @ 2025-09-08 19:59 UTC (permalink / raw)
  To: Sami Imseih <samimseih@gmail.com>; +Cc: pgsql-hackers

On Mon, Sep 08, 2025 at 02:47:26PM -0500, Sami Imseih wrote:
> I think v2 is fine because it is perfectly fine for a normal backend
> (EXEC_BACKEND) to call this function as long as it's processing the
> startup hook. The goal is to prevent it from being called outside of the
> startup hook.

I thought the goal was to prevent the crashes...

> I think we should commit these tests as well. If  you think the tests
> should be a separate thread, let me know.

Thanks.  I think we can discuss them here once we fix $SUBJECT.  (BTW it
looks like you forgot to attach the patches.)

-- 
nathan





^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: GetNamedLWLockTranche crashes on Windows in normal backend
@ 2025-09-08 20:34  Sami Imseih <samimseih@gmail.com>
  parent: Nathan Bossart <nathandbossart@gmail.com>
  0 siblings, 1 reply; 33+ messages in thread

From: Sami Imseih @ 2025-09-08 20:34 UTC (permalink / raw)
  To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: pgsql-hackers

> On Mon, Sep 08, 2025 at 02:47:26PM -0500, Sami Imseih wrote:
> > I think v2 is fine because it is perfectly fine for a normal backend
> > (EXEC_BACKEND) to call this function as long as it's processing the
> > startup hook. The goal is to prevent it from being called outside of the
> > startup hook.
>
> I thought the goal was to prevent the crashes...

yes it is. Ok, I see what I did wrong with my test that showed v2 working in
EXEC_BACKEND. when it was reentering the startup hook, it was skipping
the code under (!found) for shared memory existing. That is also where I
had the code to call GetNamedLWLockTranche. So, that's wrong
( which is what you also mentioned above) I moved
the GetNamedLWLockTranche outside of the (!found) block in the test.

> > I think we should commit these tests as well. If  you think the tests
> > should be a separate thread, let me know.
>
> Thanks.  I think we can discuss them here once we fix $SUBJECT.  (BTW it
> looks like you forgot to attach the patches.)

oops. Attached now.


--
Sami

Attachments: download all (2 files, .tar.gz)

v4-0002-Tests-for-LWLock-tranche-registration-improvement.patch (application/octet-stream, 16.7K) download From 930575c1145dabb001ffa65da7ced2155e12d740 Mon Sep 17 00:00:00 2001 From: Sami Imseih <simseih@amazon.com> Date: Fri, 22 Aug 2025 00:35:45 -0500 Subject: [PATCH v4 2/2] Tests for LWLock tranche registration improvements --- src/test/modules/Makefile | 3 +- src/test/modules/meson.build | 1 + src/test/modules/test_tranches/.gitignore | 4 + src/test/modules/test_tranches/Makefile | 23 ++ src/test/modules/test_tranches/meson.build | 33 +++ .../test_tranches/t/001_test_tranches.pl | 145 +++++++++++ .../test_tranches/test_tranches--1.0.sql | 35 +++ .../modules/test_tranches/test_tranches.c | 238 ++++++++++++++++++ .../test_tranches/test_tranches.control | 6 + 9 files changed, 487 insertions(+), 1 deletion(-) create mode 100644 src/test/modules/test_tranches/.gitignore create mode 100644 src/test/modules/test_tranches/Makefile create mode 100644 src/test/modules/test_tranches/meson.build create mode 100644 src/test/modules/test_tranches/t/001_test_tranches.pl create mode 100644 src/test/modules/test_tranches/test_tranches--1.0.sql create mode 100644 src/test/modules/test_tranches/test_tranches.c create mode 100644 src/test/modules/test_tranches/test_tranches.control diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile index 903a8ac151a..fd7aa4675c5 100644 --- a/src/test/modules/Makefile +++ b/src/test/modules/Makefile @@ -44,7 +44,8 @@ SUBDIRS = \ test_tidstore \ unsafe_tests \ worker_spi \ - xid_wraparound + xid_wraparound \ + test_tranches ifeq ($(enable_injection_points),yes) diff --git a/src/test/modules/meson.build b/src/test/modules/meson.build index 93be0f57289..52883dfb496 100644 --- a/src/test/modules/meson.build +++ b/src/test/modules/meson.build @@ -45,3 +45,4 @@ subdir('typcache') subdir('unsafe_tests') subdir('worker_spi') subdir('xid_wraparound') +subdir('test_tranches') diff --git a/src/test/modules/test_tranches/.gitignore b/src/test/modules/test_tranches/.gitignore new file mode 100644 index 00000000000..5dcb3ff9723 --- /dev/null +++ b/src/test/modules/test_tranches/.gitignore @@ -0,0 +1,4 @@ +# Generated subdirectories +/log/ +/results/ +/tmp_check/ diff --git a/src/test/modules/test_tranches/Makefile b/src/test/modules/test_tranches/Makefile new file mode 100644 index 00000000000..5c9e78084da --- /dev/null +++ b/src/test/modules/test_tranches/Makefile @@ -0,0 +1,23 @@ +# src/test/modules/test_tranches/Makefile + +MODULE_big = test_tranches +OBJS = \ + $(WIN32RES) \ + test_tranches.o +PGFILEDESC = "test_tranches - test code LWLock tranche management" + +EXTENSION = test_tranches +DATA = test_tranches--1.0.sql + +TAP_TESTS = 1 + +ifdef USE_PGXS +PG_CONFIG = pg_config +PGXS := $(shell $(PG_CONFIG) --pgxs) +include $(PGXS) +else +subdir = src/test/modules/test_tranches +top_builddir = ../../../.. +include $(top_builddir)/src/Makefile.global +include $(top_srcdir)/contrib/contrib-global.mk +endif diff --git a/src/test/modules/test_tranches/meson.build b/src/test/modules/test_tranches/meson.build new file mode 100644 index 00000000000..976ce9f0a25 --- /dev/null +++ b/src/test/modules/test_tranches/meson.build @@ -0,0 +1,33 @@ +# Copyright (c) 2024-2025, PostgreSQL Global Development Group + +test_tranches_sources = files( + 'test_tranches.c', +) + +if host_system == 'windows' + test_tranches_sources += rc_lib_gen.process(win32ver_rc, extra_args: [ + '--NAME', 'test_tranches', + '--FILEDESC', 'test_tranches - test code LWLock tranche management',]) +endif + +test_tranches = shared_module('test_tranches', + test_tranches_sources, + kwargs: pg_test_mod_args, +) +test_install_libs += test_tranches + +test_install_data += files( + 'test_tranches.control', + 'test_tranches--1.0.sql', +) + +tests += { + 'name': 'test_tranches', + 'sd': meson.current_source_dir(), + 'bd': meson.current_build_dir(), + 'tap': { + 'tests': [ + 't/001_test_tranches.pl', + ], + }, +} \ No newline at end of file diff --git a/src/test/modules/test_tranches/t/001_test_tranches.pl b/src/test/modules/test_tranches/t/001_test_tranches.pl new file mode 100644 index 00000000000..3d20412bfe1 --- /dev/null +++ b/src/test/modules/test_tranches/t/001_test_tranches.pl @@ -0,0 +1,145 @@ +# Copyright (c) 2024-2025, PostgreSQL Global Development Group +# +# This test verifies LWLock tranche registration. It checks for correct +# registration of tranche names, proper error handling for unregistered +# tranches, overly long tranche names, exceeding the maximum number of +# named tranches allowed, and correct LWLock initialization. + +use strict; +use warnings FATAL => 'all'; + +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +# +# Create the cluster without any requested tranches +# +my $node = PostgreSQL::Test::Cluster->new('primary'); +$node->init(); +$node->append_conf('postgresql.conf', + qq(shared_preload_libraries='test_tranches')); +$node->append_conf('postgresql.conf', + qq(test_tranches.requested_named_tranches=0)); +$node->start(); +$node->safe_psql('postgres', q(CREATE EXTENSION test_tranches)); + +# +# Get the first user-defined tranche ID and define some user-defined tranches. +# +# We don't use LWTRANCHE_FIRST_USER_DEFINED directly in the test since it could +# change in future releases, instead we expose the value via a C function. +# +my $first_user_defined = $node->safe_psql('postgres', q(select test_tranches_get_first_user_defined())); +my @user_defined = ($first_user_defined .. $first_user_defined + 5); +my ($second_user_defined, + $third_user_defined, + $fourth_user_defined, + $fifth_user_defined, + $sixth_user_defined) = @user_defined[1..5]; + +# +# Run lookup tests to ensure the correct tranche names are returned +# and an error is raised for unregistered tranches +# +$node->safe_psql('postgres', "select test_tranches_new(5);"); +my ($result, $stdout, $stderr) = $node->psql('postgres', + qq{ + select test_tranches_lookup(1); + select test_tranches_lookup($first_user_defined); + select test_tranches_lookup($second_user_defined); + select test_tranches_lookup($third_user_defined); + select test_tranches_lookup($fourth_user_defined); + select test_tranches_lookup($fifth_user_defined); + select test_tranches_lookup($sixth_user_defined); + }, + on_error_stop => 0); + +like("$stderr", qr/ERROR: tranche 100 is not registered/, "error on unregistered tranche"); +like("$stdout", + qr/ShmemIndex.*test_lock__0.*test_lock__1.*test_lock__2.*test_lock__3.*test_lock__4/s, + "tranche names match without requested tranches"); + +# +# Test the error for long tranche names +# +my $good_tranche_name = 'A' x 63; +my $bad_tranche_name = 'B' x 64; # MAX_NAMED_TRANCHE_NAME_LEN +$node->safe_psql('postgres', qq{select test_tranches_new_tranche('$good_tranche_name');}); +($result, $stdout, $stderr) = $node->psql('postgres', + qq{ + select test_tranches_new_tranche('$bad_tranche_name'); + }, + on_error_stop => 0); + +like("$stderr", qr/ERROR: tranche name too long/, "error on tranche name too long"); + +$node->restart(); + +# +# Test the error when > MAX_NAMED_TRANCHES named tranches registered +# +$node->safe_psql('postgres', qq{select test_tranches_new(255)}); +$node->safe_psql('postgres', qq{select test_tranches_new(1)}); +($result, $stdout, $stderr) = $node->psql('postgres', qq{select test_tranches_new(1);}, on_error_stop => 0); + +like("$stderr", qr/ERROR: maximum number of tranches already registered/, "error on too many tranches registered"); + +# +# Repeat the lookup test with 2 requested tranches +# +$node->append_conf('postgresql.conf', + qq(test_tranches.requested_named_tranches=2)); +$node->restart(); + +@user_defined = ($first_user_defined .. $first_user_defined + 5); +($second_user_defined, + $third_user_defined, + $fourth_user_defined, + $fifth_user_defined, + $sixth_user_defined) = @user_defined[1..5]; + +$node->safe_psql('postgres', "select test_tranches_new(3);"); +($result, $stdout, $stderr) = $node->psql('postgres', + qq{ + select test_tranches_lookup(1); + select test_tranches_lookup($first_user_defined); + select test_tranches_lookup($second_user_defined); + select test_tranches_lookup($third_user_defined); + select test_tranches_lookup($fourth_user_defined); + select test_tranches_lookup($fifth_user_defined); + select test_tranches_lookup($sixth_user_defined); + }, + on_error_stop => 0); + +like("$stderr", qr/ERROR: tranche 100 is not registered/, "error on unregistered tranche"); +like("$stdout", qr/ShmemIndex.*test_lock_0.*test_lock_1.*test_lock__2.*test_lock__3.*test_lock__4/s, + "tranche names match with requested tranches"); + +# +# Test lwlock initialize +# +$node->safe_psql('postgres', qq{select test_tranches_lwlock_initialize($first_user_defined)}); +($result, $stdout, $stderr) = $node->psql('postgres', + qq{select test_tranches_lwlock_initialize($sixth_user_defined)}, + on_error_stop => 0); + +like("$stderr", qr/ERROR: tranche 100 is not registered/, "error on LWLock initialization for invalid tranche"); + +# +# Test error for NULL tranche name +# +($result, $stdout, $stderr) = $node->psql('postgres', + qq{select test_tranches_new_tranche(NULL)}, + on_error_stop => 0); + +like("$stderr", qr/ERROR: tranche name cannot be NULL/, "error on NULL tranche name"); + +# +# Test for calling GetNamedLWLockTranche outside of shmem_startup hook +# +($result, $stdout, $stderr) = $node->safe_psql('postgres', + qq{select test_tranches_get_named_lwlock('test_lock_0')}, + on_error_stop => 0); + +done_testing(); diff --git a/src/test/modules/test_tranches/test_tranches--1.0.sql b/src/test/modules/test_tranches/test_tranches--1.0.sql new file mode 100644 index 00000000000..62f66b3aa58 --- /dev/null +++ b/src/test/modules/test_tranches/test_tranches--1.0.sql @@ -0,0 +1,35 @@ +-- test_tranches--1.0.sql + +CREATE FUNCTION test_tranches_new(bigint) +RETURNS void +AS 'MODULE_PATHNAME', 'test_tranches_new' +LANGUAGE C STRICT; + +CREATE FUNCTION test_tranches_lookup(int) +RETURNS text +AS 'MODULE_PATHNAME', 'test_tranches_lookup' +LANGUAGE C STRICT; + +CREATE FUNCTION test_tranches_get_named_lwlock(text) +RETURNS void +AS 'MODULE_PATHNAME', 'test_tranches_get_named_lwlock' +LANGUAGE C STRICT; + +CREATE FUNCTION test_tranches_get_first_user_defined() +RETURNS int +AS 'MODULE_PATHNAME', 'test_tranches_get_first_user_defined' +LANGUAGE C STRICT; + +CREATE FUNCTION test_tranches_lwlock_initialize(int) +RETURNS void +AS 'MODULE_PATHNAME', 'test_tranches_lwlock_initialize' +LANGUAGE C STRICT; + +/* + * Function is CALLED ON NULL INPUT to allow NULL input + * for testing + */ +CREATE FUNCTION test_tranches_new_tranche(text) +RETURNS int +AS 'MODULE_PATHNAME', 'test_tranches_new_tranche' +LANGUAGE C; \ No newline at end of file diff --git a/src/test/modules/test_tranches/test_tranches.c b/src/test/modules/test_tranches/test_tranches.c new file mode 100644 index 00000000000..bfc6fa5f4e2 --- /dev/null +++ b/src/test/modules/test_tranches/test_tranches.c @@ -0,0 +1,238 @@ +/*-------------------------------------------------------------------------- + * + * test_tranches.c + * Test registration of named LWLock tranches. + * + * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * IDENTIFICATION + * src/test/modules/test_tranches/test_tranches.c + * + * ------------------------------------------------------------------------- + */ + +#include "postgres.h" + +#include "miscadmin.h" +#include "storage/ipc.h" +#include "storage/lwlock.h" +#include "storage/shmem.h" +#include "utils/builtins.h" +#include "utils/guc.h" +#include "utils/wait_classes.h" + +PG_MODULE_MAGIC; + +/* hooks */ +static shmem_request_hook_type prev_shmem_request_hook = NULL; +static shmem_startup_hook_type prev_shmem_startup_hook = NULL; +static void test_tranches_shmem_request(void); +static void test_tranches_shmem_startup(void); + +/* GUC */ +static int test_tranches_requested_named_tranches = 0; + +typedef struct testTranchesSharedState +{ + int next_index; +} testTranchesSharedState; + +static testTranchesSharedState * test_lwlock_ss = NULL; + +/* + * LWLock wait event masks. Copied from src/backend/utils/activity/wait_event.c + */ +#define WAIT_EVENT_CLASS_MASK 0xFF000000 + +/* + * Module load callback + */ +void +_PG_init(void) +{ + prev_shmem_request_hook = shmem_request_hook; + shmem_request_hook = test_tranches_shmem_request; + prev_shmem_startup_hook = shmem_startup_hook; + shmem_startup_hook = test_tranches_shmem_startup; + + DefineCustomIntVariable("test_tranches.requested_named_tranches", + "Sets the number of locks created during shmem request", + NULL, + &test_tranches_requested_named_tranches, + 2, + 0, + UINT16_MAX, + PGC_POSTMASTER, + 0, + NULL, + NULL, + NULL); +} + +/* + * test_tranches_shmem_startup + * + * Initialize our shared state and create the requested number of + * named tranches. + */ +static void +test_tranches_shmem_startup(void) +{ + bool found; + + if (prev_shmem_startup_hook) + prev_shmem_startup_hook(); + + test_lwlock_ss = ShmemInitStruct("test_tranches", + sizeof(testTranchesSharedState), + &found); + if (!found) + test_lwlock_ss->next_index = test_tranches_requested_named_tranches; + + for (int i = 0; i < test_tranches_requested_named_tranches; i++) + { + char name[15]; + + snprintf(name, sizeof(name), "test_lock_%d", i); + GetNamedLWLockTranche(name); + } + +} + +static Size +test_tranches_memsize(void) +{ + Size size; + + size = MAXALIGN(sizeof(testTranchesSharedState)); + + return size; +} + +/* + * test_tranches_shmem_request + * + * Request shared memory for our shared state and the requested number + * of named tranches. + */ +static void +test_tranches_shmem_request(void) +{ + int i = 0; + + if (prev_shmem_request_hook) + prev_shmem_request_hook(); + + RequestAddinShmemSpace(test_tranches_memsize()); + + for (i = 0; i < test_tranches_requested_named_tranches; i++) + { + char name[15]; + + snprintf(name, sizeof(name), "test_lock_%d", i); + RequestNamedLWLockTranche(name, i); + } +} + +/* + * test_tranches_new + * + * Create the specified number of new named tranches. + */ +PG_FUNCTION_INFO_V1(test_tranches_new); +Datum +test_tranches_new(PG_FUNCTION_ARGS) +{ + int64 num = PG_GETARG_INT64(0); + int i; + + for (i = test_lwlock_ss->next_index; i < num + test_lwlock_ss->next_index; i++) + { + char name[50]; + + snprintf(name, 50, "test_lock__%d", i); + + LWLockNewTrancheId(name); + } + + test_lwlock_ss->next_index = i; + + PG_RETURN_VOID(); +} + +/* + * test_tranches_new_tranche + * + * Create a new named tranche with the specified name. + */ +PG_FUNCTION_INFO_V1(test_tranches_new_tranche); +Datum +test_tranches_new_tranche(PG_FUNCTION_ARGS) +{ + char *tranche_name = NULL; + + if (!PG_ARGISNULL(0)) + tranche_name = text_to_cstring(PG_GETARG_TEXT_PP(0)); + + PG_RETURN_INT32(LWLockNewTrancheId(tranche_name)); +} + +/* + * test_tranches_lookup + * + * Lookup the name of a tranche by its ID. + */ +PG_FUNCTION_INFO_V1(test_tranches_lookup); +Datum +test_tranches_lookup(PG_FUNCTION_ARGS) +{ + const char *tranche_name = GetLWLockIdentifier(PG_WAIT_LWLOCK & WAIT_EVENT_CLASS_MASK, PG_GETARG_INT32(0)); + + if (tranche_name) + PG_RETURN_TEXT_P(cstring_to_text(tranche_name)); + else + PG_RETURN_NULL(); +} + +/* + * test_tranches_get_named_lwlock + * + * A wrapper function to test calling GetNamedLWLockTranche. + */ +PG_FUNCTION_INFO_V1(test_tranches_get_named_lwlock); +Datum +test_tranches_get_named_lwlock(PG_FUNCTION_ARGS) +{ + GetNamedLWLockTranche(text_to_cstring(PG_GETARG_TEXT_PP(0))); + + PG_RETURN_NULL(); +} + +/* + * test_tranches_get_first_user_defined + * + * A wrapper function to return LWTRANCHE_FIRST_USER_DEFINED. + */ +PG_FUNCTION_INFO_V1(test_tranches_get_first_user_defined); +Datum +test_tranches_get_first_user_defined(PG_FUNCTION_ARGS) +{ + return LWTRANCHE_FIRST_USER_DEFINED; +} + +/* + * test_tranches_lwlock_initialize + * + * A wrapper function to test calling LWLockInitialize. + */ +PG_FUNCTION_INFO_V1(test_tranches_lwlock_initialize); +Datum +test_tranches_lwlock_initialize(PG_FUNCTION_ARGS) +{ + LWLock lock; + + LWLockInitialize(&lock, PG_GETARG_INT32(0)); + + PG_RETURN_VOID(); +} diff --git a/src/test/modules/test_tranches/test_tranches.control b/src/test/modules/test_tranches/test_tranches.control new file mode 100644 index 00000000000..b096c4ae115 --- /dev/null +++ b/src/test/modules/test_tranches/test_tranches.control @@ -0,0 +1,6 @@ +# test_tranches.control + +comment = 'Test LWLock tranche names tracking' +default_version = '1.0' +relocatable = false +module_pathname = '$libdir/test_tranches' \ No newline at end of file -- 2.43.0
v4-0001-Move-named-LWLock-tranche-request-array-to-shared.patch (application/octet-stream, 4.7K) download From 9b936a0cad7bbd431478b7e8141dbdb40cf5b624 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nathan@postgresql.org> Date: Mon, 8 Sep 2025 13:18:59 -0500 Subject: [PATCH v4 1/2] Move named LWLock tranche request array to shared memory. --- src/backend/postmaster/launch_backend.c | 3 +++ src/backend/storage/lmgr/lwlock.c | 31 +++++++++++++++++++++---- src/include/storage/lwlock.h | 4 ++++ 3 files changed, 33 insertions(+), 5 deletions(-) diff --git a/src/backend/postmaster/launch_backend.c b/src/backend/postmaster/launch_backend.c index a38979c50e4..c5ef14e1eaa 100644 --- a/src/backend/postmaster/launch_backend.c +++ b/src/backend/postmaster/launch_backend.c @@ -101,6 +101,7 @@ typedef struct struct InjectionPointsCtl *ActiveInjectionPoints; #endif int NamedLWLockTrancheRequests; + NamedLWLockTrancheRequest *NamedLWLockTrancheRequestArray; char **LWLockTrancheNames; int *LWLockCounter; LWLockPadded *MainLWLockArray; @@ -761,6 +762,7 @@ save_backend_variables(BackendParameters *param, #endif param->NamedLWLockTrancheRequests = NamedLWLockTrancheRequests; + param->NamedLWLockTrancheRequestArray = NamedLWLockTrancheRequestArray; param->LWLockTrancheNames = LWLockTrancheNames; param->LWLockCounter = LWLockCounter; param->MainLWLockArray = MainLWLockArray; @@ -1022,6 +1024,7 @@ restore_backend_variables(BackendParameters *param) #endif NamedLWLockTrancheRequests = param->NamedLWLockTrancheRequests; + NamedLWLockTrancheRequestArray = param->NamedLWLockTrancheRequestArray; LWLockTrancheNames = param->LWLockTrancheNames; LWLockCounter = param->LWLockCounter; MainLWLockArray = param->MainLWLockArray; diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c index fcbac5213a5..46c82c63ca5 100644 --- a/src/backend/storage/lmgr/lwlock.c +++ b/src/backend/storage/lmgr/lwlock.c @@ -184,14 +184,13 @@ typedef struct NamedLWLockTrancheRequest int num_lwlocks; } NamedLWLockTrancheRequest; -static NamedLWLockTrancheRequest *NamedLWLockTrancheRequestArray = NULL; - /* - * NamedLWLockTrancheRequests is the valid length of the request array. This - * variable is non-static so that postmaster.c can copy them to child processes - * in EXEC_BACKEND builds. + * NamedLWLockTrancheRequests is the valid length of the request array. These + * variables are non-static so that launch_backend.c can copy them to child + * processes in EXEC_BACKEND builds. */ int NamedLWLockTrancheRequests = 0; +NamedLWLockTrancheRequest *NamedLWLockTrancheRequestArray = NULL; /* shared memory counter of registered tranches */ int *LWLockCounter = NULL; @@ -407,6 +406,14 @@ LWLockShmemSize(void) size = add_size(size, mul_size(MAX_NAMED_TRANCHES, sizeof(char *))); size = add_size(size, mul_size(MAX_NAMED_TRANCHES, NAMEDATALEN)); + /* + * Make space for named tranche requests. This is done for the benefit of + * EXEC_BACKEND builds, which otherwise wouldn't be able to call + * GetNamedLWLockTranche() outside postmaster. + */ + size = add_size(size, mul_size(NamedLWLockTrancheRequests, + sizeof(NamedLWLockTrancheRequest))); + /* Space for the LWLock array, plus room for cache line alignment. */ size = add_size(size, LWLOCK_PADDED_SIZE); size = add_size(size, mul_size(numLocks, sizeof(LWLockPadded))); @@ -443,6 +450,20 @@ CreateLWLocks(void) ptr += NAMEDATALEN; } + /* + * Move named tranche requests to shared memory. This is done for the + * benefit of EXEC_BACKEND builds, which otherwise wouldn't be able to + * call GetNamedLWLockTranche() outside postmaster. + */ + if (NamedLWLockTrancheRequests > 0) + { + memcpy(ptr, NamedLWLockTrancheRequestArray, + NamedLWLockTrancheRequests * sizeof(NamedLWLockTrancheRequest)); + pfree(NamedLWLockTrancheRequestArray); + NamedLWLockTrancheRequestArray = (NamedLWLockTrancheRequest *) ptr; + ptr += NamedLWLockTrancheRequests * sizeof(NamedLWLockTrancheRequest); + } + /* Ensure desired alignment of LWLock array */ ptr += LWLOCK_PADDED_SIZE - ((uintptr_t) ptr) % LWLOCK_PADDED_SIZE; MainLWLockArray = (LWLockPadded *) ptr; diff --git a/src/include/storage/lwlock.h b/src/include/storage/lwlock.h index 0e9cf81a4c7..8e0d0d233b4 100644 --- a/src/include/storage/lwlock.h +++ b/src/include/storage/lwlock.h @@ -73,8 +73,12 @@ typedef union LWLockPadded extern PGDLLIMPORT LWLockPadded *MainLWLockArray; +/* forward declaration of private type for use only by lwlock.c */ +typedef struct NamedLWLockTrancheRequest NamedLWLockTrancheRequest; + extern PGDLLIMPORT char **LWLockTrancheNames; extern PGDLLIMPORT int NamedLWLockTrancheRequests; +extern PGDLLIMPORT NamedLWLockTrancheRequest *NamedLWLockTrancheRequestArray; extern PGDLLIMPORT int *LWLockCounter; /* -- 2.43.0
^ permalink raw reply [nested|flat] 33+ messages in thread

* Re: GetNamedLWLockTranche crashes on Windows in normal backend
@ 2025-09-11 21:18  Nathan Bossart <nathandbossart@gmail.com>
  parent: Sami Imseih <samimseih@gmail.com>
  0 siblings, 2 replies; 33+ messages in thread

From: Nathan Bossart @ 2025-09-11 21:18 UTC (permalink / raw)
  To: Sami Imseih <samimseih@gmail.com>; +Cc: pgsql-hackers

I've committed 0001, and I plan to look closer at 0002 soon.

-- 
nathan





^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: GetNamedLWLockTranche crashes on Windows in normal backend
@ 2025-09-16 19:35  Nathan Bossart <nathandbossart@gmail.com>
  parent: Nathan Bossart <nathandbossart@gmail.com>
  1 sibling, 1 reply; 33+ messages in thread

From: Nathan Bossart @ 2025-09-16 19:35 UTC (permalink / raw)
  To: Sami Imseih <samimseih@gmail.com>; +Cc: pgsql-hackers

On Thu, Sep 11, 2025 at 04:18:00PM -0500, Nathan Bossart wrote:
> I've committed 0001, and I plan to look closer at 0002 soon.

I ended up rewriting 0002 as an ordinary regression test, which resulted in
a much smaller patch with comparable coverage.  (Yes, it needs some
comments.)  Thoughts?

-- 
nathan

Attachments:

v5-0001-test_lwlock_tranches.patch (text/plain, 11.2K) download From 9cbf8e45a907445bfe5a45ef2a32f876816497ac Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nathan@postgresql.org> Date: Tue, 16 Sep 2025 10:15:11 -0500 Subject: [PATCH v5 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 | 29 +++++ .../modules/test_lwlock_tranches/meson.build | 35 ++++++ .../sql/test_lwlock_tranches.sql | 13 ++ .../test_lwlock_tranches--1.0.sql | 13 ++ .../test_lwlock_tranches.c | 112 ++++++++++++++++++ .../test_lwlock_tranches.conf | 1 + .../test_lwlock_tranches.control | 4 + 11 files changed, 238 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..b8e2d7feaca --- /dev/null +++ b/src/test/modules/test_lwlock_tranches/expected/test_lwlock_tranches.out @@ -0,0 +1,29 @@ +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 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..f4de50e38d9 --- /dev/null +++ b/src/test/modules/test_lwlock_tranches/sql/test_lwlock_tranches.sql @@ -0,0 +1,13 @@ +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'); 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..b7d9a400f3b --- /dev/null +++ b/src/test/modules/test_lwlock_tranches/test_lwlock_tranches--1.0.sql @@ -0,0 +1,13 @@ +/* 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; 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..9d4814cf48a --- /dev/null +++ b/src/test/modules/test_lwlock_tranches/test_lwlock_tranches.c @@ -0,0 +1,112 @@ +/*-------------------------------------------------------------------------- + * + * 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/dsm_registry.h" +#include "storage/ipc.h" +#include "storage/lwlock.h" +#include "storage/shmem.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 int *dynamic_tranches; + +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); +} + +static void +init_dsm(void *ptr) +{ + for (int i = 0; i < NUM_DYNAMIC_TRANCHES; i++) + ((int *) ptr)[i] = LWLockNewTrancheId(DYNAMIC_TRANCHE_NAME); +} + +PG_FUNCTION_INFO_V1(test_lwlock_tranches); +Datum +test_lwlock_tranches(PG_FUNCTION_ARGS) +{ + bool unused; + + dynamic_tranches = GetNamedDSMSegment("test_lwlock_tranches", + sizeof(int) * NUM_DYNAMIC_TRANCHES, + init_dsm, + &unused); + + 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(); +} 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)
^ permalink raw reply [nested|flat] 33+ messages in thread

* Re: GetNamedLWLockTranche crashes on Windows in normal backend
@ 2025-09-16 19:46  Nathan Bossart <nathandbossart@gmail.com>
  parent: Nathan Bossart <nathandbossart@gmail.com>
  0 siblings, 1 reply; 33+ messages in thread

From: Nathan Bossart @ 2025-09-16 19:46 UTC (permalink / raw)
  To: Sami Imseih <samimseih@gmail.com>; +Cc: pgsql-hackers

On Tue, Sep 16, 2025 at 02:35:55PM -0500, Nathan Bossart wrote:
> On Thu, Sep 11, 2025 at 04:18:00PM -0500, Nathan Bossart wrote:
>> I've committed 0001, and I plan to look closer at 0002 soon.
> 
> I ended up rewriting 0002 as an ordinary regression test, which resulted in
> a much smaller patch with comparable coverage.  (Yes, it needs some
> comments.)  Thoughts?

Sorry for the noise.  I was able to simplify the test code further.

-- 
nathan

Attachments:

v6-0001-test_lwlock_tranches.patch (text/plain, 10.9K) download From 92d7a5569d94f1b5c429d9b52868e5d3f6fe2b2c Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nathan@postgresql.org> Date: Tue, 16 Sep 2025 10:15:11 -0500 Subject: [PATCH v6 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 | 29 ++++++ .../modules/test_lwlock_tranches/meson.build | 35 +++++++ .../sql/test_lwlock_tranches.sql | 13 +++ .../test_lwlock_tranches--1.0.sql | 13 +++ .../test_lwlock_tranches.c | 98 +++++++++++++++++++ .../test_lwlock_tranches.conf | 1 + .../test_lwlock_tranches.control | 4 + 11 files changed, 224 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..b8e2d7feaca --- /dev/null +++ b/src/test/modules/test_lwlock_tranches/expected/test_lwlock_tranches.out @@ -0,0 +1,29 @@ +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 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..f4de50e38d9 --- /dev/null +++ b/src/test/modules/test_lwlock_tranches/sql/test_lwlock_tranches.sql @@ -0,0 +1,13 @@ +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'); 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..b7d9a400f3b --- /dev/null +++ b/src/test/modules/test_lwlock_tranches/test_lwlock_tranches--1.0.sql @@ -0,0 +1,13 @@ +/* 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; 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..7a6556961b1 --- /dev/null +++ b/src/test/modules/test_lwlock_tranches/test_lwlock_tranches.c @@ -0,0 +1,98 @@ +/*-------------------------------------------------------------------------- + * + * 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(); +} 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)
^ permalink raw reply [nested|flat] 33+ messages in thread

* Re: GetNamedLWLockTranche crashes on Windows in normal backend
@ 2025-09-16 21:04  Sami Imseih <samimseih@gmail.com>
  parent: Nathan Bossart <nathandbossart@gmail.com>
  0 siblings, 1 reply; 33+ messages in thread

From: Sami Imseih @ 2025-09-16 21:04 UTC (permalink / raw)
  To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: pgsql-hackers

> On Tue, Sep 16, 2025 at 02:35:55PM -0500, Nathan Bossart wrote:
> > On Thu, Sep 11, 2025 at 04:18:00PM -0500, Nathan Bossart wrote:
> >> I've committed 0001, and I plan to look closer at 0002 soon.
> >
> > I ended up rewriting 0002 as an ordinary regression test, which resulted in
> > a much smaller patch with comparable coverage.  (Yes, it needs some
> > comments.)  Thoughts?
>
> Sorry for the noise.  I was able to simplify the test code further.

This is a trimmed down test suite, but it does cover quite a bit:
tranche name length, NULL tranche names, number of tranches registered,
tranche name lookup. Overall this is still good test coverage.

A few comments.

1/
startup tranches should be:

#define NUM_STARTUP_TRANCHES (2)

instead of:

#define NUM_STARTUP_TRANCHES (256 - 2)

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

```
Datum
test_lwlock_tranche_initialize(PG_FUNCTION_ARGS)
{
LWLock lock;

LWLockInitialize(&lock, LWTRANCHE_FIRST_USER_DEFINED + PG_GETARG_INT32(0));

PG_RETURN_VOID();
}
```

and we can run this test as the first query, before we create any
dynamic tranches
```
SELECT test_lwlock_tranche_initialize(2);
```

The error message will change when a built-in tranche is added, so
the out file will need to be updated at that point, but that does not occur
too often.

--
Sami





^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: GetNamedLWLockTranche crashes on Windows in normal backend
@ 2025-09-17 15:31  Nathan Bossart <nathandbossart@gmail.com>
  parent: Sami Imseih <samimseih@gmail.com>
  0 siblings, 1 reply; 33+ messages in thread

From: Nathan Bossart @ 2025-09-17 15:31 UTC (permalink / raw)
  To: Sami Imseih <samimseih@gmail.com>; +Cc: pgsql-hackers

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

Attachments:

v7-0001-test_lwlock_tranches.patch (text/plain, 11.4K) download 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)
^ permalink raw reply [nested|flat] 33+ messages in thread

* Re: GetNamedLWLockTranche crashes on Windows in normal backend
@ 2025-09-17 16:08  Sami Imseih <samimseih@gmail.com>
  parent: Nathan Bossart <nathandbossart@gmail.com>
  0 siblings, 1 reply; 33+ messages in thread

From: Sami Imseih @ 2025-09-17 16:08 UTC (permalink / raw)
  To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: pgsql-hackers

> 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?

It does not really matter for the tests being done, but it just
seems odd that you would create 254 tranches during startup and
leave room for only 2 dynamic tranches. It is not the typical
pattern out there, in my opinion.

--
Sami





^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: GetNamedLWLockTranche crashes on Windows in normal backend
@ 2025-09-17 16:19  Nathan Bossart <nathandbossart@gmail.com>
  parent: Sami Imseih <samimseih@gmail.com>
  0 siblings, 1 reply; 33+ messages in thread

From: Nathan Bossart @ 2025-09-17 16:19 UTC (permalink / raw)
  To: Sami Imseih <samimseih@gmail.com>; +Cc: pgsql-hackers

On Wed, Sep 17, 2025 at 11:08:12AM -0500, Sami Imseih wrote:
>> 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?
> 
> It does not really matter for the tests being done, but it just
> seems odd that you would create 254 tranches during startup and
> leave room for only 2 dynamic tranches. It is not the typical
> pattern out there, in my opinion.

Okay.  I changed it in v8.

-- 
nathan

Attachments:

v8-0001-test_lwlock_tranches.patch (text/plain, 11.4K) download From f5d462e175fe6d9ab76b1e59b98013480df5ed9b Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nathan@postgresql.org> Date: Tue, 16 Sep 2025 10:15:11 -0500 Subject: [PATCH v8 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..4e2c43ac933 --- /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 (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)
^ permalink raw reply [nested|flat] 33+ messages in thread

* Re: GetNamedLWLockTranche crashes on Windows in normal backend
@ 2025-09-17 16:43  Sami Imseih <samimseih@gmail.com>
  parent: Nathan Bossart <nathandbossart@gmail.com>
  0 siblings, 1 reply; 33+ messages in thread

From: Sami Imseih @ 2025-09-17 16:43 UTC (permalink / raw)
  To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: pgsql-hackers

> Okay.  I changed it in v8.

Thanks! v8 LGTM.

--
Sami





^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: GetNamedLWLockTranche crashes on Windows in normal backend
@ 2025-09-17 19:00  Nathan Bossart <nathandbossart@gmail.com>
  parent: Sami Imseih <samimseih@gmail.com>
  0 siblings, 1 reply; 33+ messages in thread

From: Nathan Bossart @ 2025-09-17 19:00 UTC (permalink / raw)
  To: Sami Imseih <samimseih@gmail.com>; +Cc: pgsql-hackers

On Wed, Sep 17, 2025 at 11:43:31AM -0500, Sami Imseih wrote:
> Thanks! v8 LGTM.

Actually, we do want to have a few more startup tranches in order to get
some coverage on the repalloc() path in RequestNamedLWLockTranche().  I've
bumped it up to 32 and added a commit message to the patch.  Barring more
feedback, I'm planning to commit this soon.

-- 
nathan

Attachments:

v9-0001-Add-a-test-harness-for-the-LWLock-tranche-code.patch (text/plain, 11.9K) download From 112dc6f722daaaf3c08c8a268d19f6678a2d1cbe Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nathan@postgresql.org> Date: Wed, 17 Sep 2025 13:45:49 -0500 Subject: [PATCH v9 1/1] Add a test harness for the LWLock tranche code. This code is heavily used and already has decent test coverage, but it lacks a dedicated test suite. This commit changes that. Author: Sami Imseih <samimseih@gmail.com> Co-authored-by: Nathan Bossart <nathandbossart@gmail.com> Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com> Discussion: https://postgr.es/m/CAA5RZ0tQ%2BEYSTOd2hQ8RXdsNfGBLAtOe-YmnsTE6ZVg0E-4qew%40mail.gmail.com Discussion: https://postgr.es/m/CAA5RZ0vpr0P2rbA%3D_K0_SCHM7bmfVX4wEO9FAyopN1eWCYORhA%40mail.gmail.com --- 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..4f6e3a8c007 --- /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 (32) +#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)
^ permalink raw reply [nested|flat] 33+ messages in thread

* Re: GetNamedLWLockTranche crashes on Windows in normal backend
@ 2025-09-17 19:17  Sami Imseih <samimseih@gmail.com>
  parent: Nathan Bossart <nathandbossart@gmail.com>
  0 siblings, 1 reply; 33+ messages in thread

From: Sami Imseih @ 2025-09-17 19:17 UTC (permalink / raw)
  To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: pgsql-hackers

> Actually, we do want to have a few more startup tranches in order to get
> some coverage on the repalloc() path in RequestNamedLWLockTranche().  I've
> bumped it up to 32 and added a commit message to the patch.  Barring more
> feedback, I'm planning to commit this soon.

Makes sense. Thanks!

--
Sami





^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: GetNamedLWLockTranche crashes on Windows in normal backend
@ 2025-09-17 20:00  Alexander Lakhin <exclusion@gmail.com>
  parent: Nathan Bossart <nathandbossart@gmail.com>
  1 sibling, 1 reply; 33+ messages in thread

From: Alexander Lakhin @ 2025-09-17 20:00 UTC (permalink / raw)
  To: Nathan Bossart <nathandbossart@gmail.com>; Sami Imseih <samimseih@gmail.com>; +Cc: pgsql-hackers

Hello Nathan,

12.09.2025 00:18, Nathan Bossart wrote:
> I've committed 0001, and I plan to look closer at 0002 soon.
>

Please look at the recovery test failures produced since ed1aad15e, with
pg_stat_statements loaded: [1]. I've reproduced them locally with:
./configure --enable-cassert --enable-debug --enable-tap-tests -q && make -s -j8
echo "
debug_parallel_query = regress
shared_preload_libraries = 'pg_stat_statements'
" >/tmp/test.config
TEMP_CONFIG=/tmp/test.config make -s check -C src/test/recovery/

[1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=batta&dt=2025-09-12%2002%3A05%3A01

Best regards,
Alexander





^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: GetNamedLWLockTranche crashes on Windows in normal backend
@ 2025-09-17 20:04  Nathan Bossart <nathandbossart@gmail.com>
  parent: Alexander Lakhin <exclusion@gmail.com>
  0 siblings, 0 replies; 33+ messages in thread

From: Nathan Bossart @ 2025-09-17 20:04 UTC (permalink / raw)
  To: Alexander Lakhin <exclusion@gmail.com>; +Cc: Sami Imseih <samimseih@gmail.com>; pgsql-hackers

On Wed, Sep 17, 2025 at 11:00:00PM +0300, Alexander Lakhin wrote:
> Please look at the recovery test failures produced since ed1aad15e, with
> pg_stat_statements loaded:

I'm tracking a fix for that in another thread [0].

[0] https://postgr.es/m/aMrG8F-NkiedEugc%40nathan

-- 
nathan





^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: GetNamedLWLockTranche crashes on Windows in normal backend
@ 2025-09-18 20:26  Nathan Bossart <nathandbossart@gmail.com>
  parent: Sami Imseih <samimseih@gmail.com>
  0 siblings, 0 replies; 33+ messages in thread

From: Nathan Bossart @ 2025-09-18 20:26 UTC (permalink / raw)
  To: Sami Imseih <samimseih@gmail.com>; +Cc: pgsql-hackers

Committed.

-- 
nathan





^ permalink  raw  reply  [nested|flat] 33+ messages in thread


end of thread, other threads:[~2025-09-18 20:26 UTC | newest]

Thread overview: 33+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-08-22 19:21 GetNamedLWLockTranche crashes on Windows in normal backend Sami Imseih <samimseih@gmail.com>
2025-08-25 16:46 ` Nathan Bossart <nathandbossart@gmail.com>
2025-08-25 17:58   ` Sami Imseih <samimseih@gmail.com>
2025-08-25 18:31     ` Nathan Bossart <nathandbossart@gmail.com>
2025-08-25 18:44       ` Sami Imseih <samimseih@gmail.com>
2025-08-25 18:51         ` Sami Imseih <samimseih@gmail.com>
2025-08-25 19:02         ` Nathan Bossart <nathandbossart@gmail.com>
2025-08-25 19:34           ` Sami Imseih <samimseih@gmail.com>
2025-08-25 21:28             ` Sami Imseih <samimseih@gmail.com>
2025-08-25 21:41               ` Nathan Bossart <nathandbossart@gmail.com>
2025-09-03 21:36                 ` Nathan Bossart <nathandbossart@gmail.com>
2025-09-04 18:23                   ` Sami Imseih <samimseih@gmail.com>
2025-09-04 18:41                     ` Nathan Bossart <nathandbossart@gmail.com>
2025-09-04 21:12                       ` Sami Imseih <samimseih@gmail.com>
2025-09-05 01:35                         ` Nathan Bossart <nathandbossart@gmail.com>
2025-09-08 16:55                           ` Nathan Bossart <nathandbossart@gmail.com>
2025-09-08 18:24                             ` Nathan Bossart <nathandbossart@gmail.com>
2025-09-08 19:47                             ` Sami Imseih <samimseih@gmail.com>
2025-09-08 19:59                               ` Nathan Bossart <nathandbossart@gmail.com>
2025-09-08 20:34                                 ` Sami Imseih <samimseih@gmail.com>
2025-09-11 21:18                                   ` Nathan Bossart <nathandbossart@gmail.com>
2025-09-16 19:35                                     ` Nathan Bossart <nathandbossart@gmail.com>
2025-09-16 19:46                                       ` Nathan Bossart <nathandbossart@gmail.com>
2025-09-16 21:04                                         ` Sami Imseih <samimseih@gmail.com>
2025-09-17 15:31                                           ` Nathan Bossart <nathandbossart@gmail.com>
2025-09-17 16:08                                             ` Sami Imseih <samimseih@gmail.com>
2025-09-17 16:19                                               ` Nathan Bossart <nathandbossart@gmail.com>
2025-09-17 16:43                                                 ` Sami Imseih <samimseih@gmail.com>
2025-09-17 19:00                                                   ` Nathan Bossart <nathandbossart@gmail.com>
2025-09-17 19:17                                                     ` Sami Imseih <samimseih@gmail.com>
2025-09-18 20:26                                                       ` Nathan Bossart <nathandbossart@gmail.com>
2025-09-17 20:00                                     ` Alexander Lakhin <exclusion@gmail.com>
2025-09-17 20:04                                       ` Nathan Bossart <nathandbossart@gmail.com>

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox