Re: Add assertion on held AddinShmemInitLock in GetNamedLWLockTranche()

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