Add assertion on held AddinShmemInitLock in GetNamedLWLockTranche()
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t48220psql -h localhost -U postgresBuilt from patchset v1 (message #1), September 20, 2026 at 10:47 AM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t48220_1 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t48220_1 && git checkout t48220_1Patchset v1 (message #1) is on t48220_1
Hi all,
While digging into the LWLock code, I have noticed that
GetNamedLWLockTranche() assumes that its caller should hold the LWLock
AddinShmemInitLock to prevent any kind of race conditions when
initializing shmem areas, but we don't make sure that's the case.
The sole caller of GetNamedLWLockTranche() in core respects that, but
out-of-core code may not be that careful. How about adding an
assertion based on LWLockHeldByMeInMode() to make sure that the
ShmemInit lock is taken when this routine is called, like in the
attached?
Thanks,
--
Michael
On Fri, Jul 28, 2023 at 8:54 AM Michael Paquier <michael@paquier.xyz> wrote:
Hi all,
While digging into the LWLock code, I have noticed that
GetNamedLWLockTranche() assumes that its caller should hold the LWLock
AddinShmemInitLock to prevent any kind of race conditions when
initializing shmem areas, but we don't make sure that's the case.The sole caller of GetNamedLWLockTranche() in core respects that, but
out-of-core code may not be that careful. How about adding an
assertion based on LWLockHeldByMeInMode() to make sure that the
ShmemInit lock is taken when this routine is called, like in the
attached?
+1 for asserting that the caller holds AddinShmemInitLock to prevent
reads while someone else is adding their LWLocks.
+ Assert(LWLockHeldByMeInMode(AddinShmemInitLock, LW_EXCLUSIVE));
Why to block multiple readers (if at all there exists any), with
LWLockHeldByMeInMode(..., LW_EXCLUSIVE)? I think
Assert(LWLockHeldByMe(AddinShmemInitLock)); suffices in
GetNamedLWLockTranche.
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
On Fri, Jul 28, 2023 at 11:07:49AM +0530, Bharath Rupireddy wrote:
Why to block multiple readers (if at all there exists any), with
LWLockHeldByMeInMode(..., LW_EXCLUSIVE)? I think
Assert(LWLockHeldByMe(AddinShmemInitLock)); suffices in
GetNamedLWLockTranche.
I am not sure to follow this argument. Why would it make sense for
anybody to use that in shared mode? We document that the exclusive
mode is required because we retrieve the lock position in shmem that
an extension needs to know about when it initializes its own shmem
state, and that cannot be done safely without LW_EXCLUSIVE. Any
extension I can find in https://codesearch.debian.net/ does that as
well.
--
Michael