MIME-Version: 1.0
References: 
 <CAExHW5vM1bneLYfg0wGeAa=52UiJ3z4vKd3AJ72X8Fw6k3KKrg@mail.gmail.com>
 <113724ab-0028-493f-9605-6e8570f0939f@iki.fi>
 <CAExHW5uTNWOSxJDWQAUnS0tZawob2_J3dRAtc67NHNZ98X4_xA@mail.gmail.com>
 <CAExHW5t439y61YD9bc7d5wZWHp6J=M43Qu3eEZOBPguZML7o2A@mail.gmail.com>
 <CAExHW5v5FVZbsO9sLzztMZ11C3hgGStE=HkkV2bQkCyncess4w@mail.gmail.com>
 <b2451225-0a10-4ded-abef-4e9efbde7a8e@iki.fi>
 <CAExHW5tCC0T1ky=Jnq-AvMxa67Adaw7aQ4iQAO=BSdHcbSNBVg@mail.gmail.com>
 <791c3f18-f4de-4d84-ac6b-c7ccc074dd38@iki.fi>
 <CAExHW5tS7GncN90oJWOSzW_3F1EHL9xwe59L7Req3nUVgmObUw@mail.gmail.com>
 <9d919bd9-94dd-4bda-8ccf-ebced4178c53@iki.fi>
 <CAExHW5sYgt=XekOoAE-Tu_Dv8obWZCjCqAPT9vtN2D4m=M=drQ@mail.gmail.com>
 <470e7ebe-0971-49f6-8e46-9b8f6395f88b@iki.fi>
 <87y0iz2c1v.fsf@wibble.ilmari.org>
 <4297b4ee-5248-415f-ae12-abf2a23f4ab6@iki.fi>
 <CAExHW5uMQGvQH6GKaBZVtH4S9O13TwN+_0Vy1gUpAW=_T_AmRA@mail.gmail.com>
 <ea001640-3818-4023-b0d5-17ce05c20701@iki.fi>
 <CAExHW5uDJdmfUVX=meyecbY2sGKq07Y33NQ68Wv2-ktaag=9qg@mail.gmail.com>
 <555608f6-63bb-4dda-b1e4-f378f0febd38@iki.fi>
In-Reply-To: <555608f6-63bb-4dda-b1e4-f378f0febd38@iki.fi>
From: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Date: Mon, 15 Jun 2026 09:58:26 +0530
Message-ID: 
 <CAExHW5sHs+eSiTDOd14buayc6JbBX=Hm5ssFMBK0Ki9sTGEOuA@mail.gmail.com>
Subject: Re: Better shared data structure management and resizable shared data
 structures
To: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: =?UTF-8?Q?Dagfinn_Ilmari_Manns=C3=A5ker?= <ilmari@ilmari.org>,
	Robert Haas <robertmhaas@gmail.com>, Andres Freund <andres@anarazel.de>,
	pgsql-hackers <pgsql-hackers@postgresql.org>, chaturvedipalak1911@gmail.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAExHW5sHs%2BeSiTDOd14buayc6JbBX%3DHm5ssFMBK0Ki9sTGEOuA%40mail.gmail.com>
Precedence: bulk

On Fri, Jun 12, 2026 at 9:07=E2=80=AFPM Heikki Linnakangas <hlinnaka@iki.fi=
> wrote:
>
> On 21/04/2026 19:05, Ashutosh Bapat wrote:
> > On Tue, Apr 21, 2026 at 1:10=E2=80=AFPM Heikki Linnakangas <hlinnaka@ik=
i.fi> wrote:
> >>
> >> On 07/04/2026 17:19, Ashutosh Bapat wrote:
> >>> Hi Heikki,
> >>> CallShmemCallbacksAfterStartup() holds ShmemIndexLock while invoking
> >>> init_fn/attach_fn callbacks. That looks wrong. Before this commit,
> >>> init or attach code was not run with the lock held. Any reason the
> >>> lock is held while calling init and attach callbacks. Since these
> >>> function can come from extensions, we don't have control on what goes
> >>> in those functions, and thus looks problematic. Further, it will
> >>> serialize all the attach_fn executions across backends, since each
> >>> will be run under the lock.
> >>
> >> This was intentional, I added a note in the docs about it:
> >>
> >>         When <function>RegisterShmemCallbacks()</function> is called a=
fter
> >>         startup, it will immediately call the appropriate callbacks,
> >> depending
> >>         on whether the requested memory areas were already initialized=
 by
> >>         another backend. The callbacks will be called while holding an
> >> internal
> >>         lock, which prevents concurrent two backends from initializing=
 the
> >>         memory area concurrently.
> >>
> >> That "internal lock" is ShmemIndexLock. I piggybacked on that since th=
e
> >> code needs to acquire it anyway for the hash table lookups.
> >
> > I had read this part, but didn't realize it's ShmemIndexLock. The
> > document and the code are placed far apart and the comments in the
> > code do not help connecting these two. The comment before
> > LWLockAcquire() call doesn't say anything about init functions.
> > /* Hold ShmemIndexLock while we allocate all the shmem entries */
> >
> >> With the old ShmemInitStruct() interface, extensions needed to do the
> >> locking themselves, usually by holding AddinShmemInitLock.
> >>
> >> (Now that I read that again, the grammar on the last sentence sounds
> >> awkward...)
> >
> > Given that the init_fn is called in only one backend which requests
> > the structures first, do we need a lock?
>
> If two backends request the same structure concurrently, which one is
> "first"? That's what the lock determines.
>
> It's not safe to release the lock before the init callback has finished.
> Otherwise, another backend might attach to the struct before it's fully
> initialized and read uninitialized values.
>
> >>> In my case, the init_fn was performing ShmemIndex lookup which
> >>> deadlocked. It's questionable whether init function should lookup
> >>> ShmemIndex but, it's not something that needs to be prohibited
> >>> either.
> >> Yeah I'm curious what the use case is. We could easily introduce anoth=
er
> >> lock or reuse AddinShmemInitLock for this.
> >
> > In case of resizable shared memory structures, I was adding mprotect
> > to make sure that the part of the shared address space which is
> > reserved but not used is protected from inadvertent access. The
> > mprotect is wrapped in a shmem API which fetches the ShmemIndex entry
> > of the shared structure, figures out the part of the address space to
> > protect using maximum_size and current size and calls mprotect
> > appropriately. To fetch the ShmemIndex entry it acquires a ShmemIndex
> > lock. The shmem API was supposed to be called from init_fn() and
> > attach_fn() to protect the address spaces as soon as the structure is
> > attached to. See patches attached to [1] for code.
> >
> > [1] https://www.postgresql.org/message-id/CAExHW5v5muT_SKV2NCxxVmvC=3D_=
38Rw0aiv-wU4CGzHaBCRYzqA@mail.gmail.com
>
> Ok. So if I understand correctly, holding ShmemIndexLock is not a actual
> problem per se, you just didn't expect it. Right?
>
> I propose the attached to improve the wording a little on the docs,
> comments, and error message.

The patch helps to set the expectations right.

ShmemIndexLock is for protecting entries in ShmemIndex; I didn't
expect it to protect the Shared structures as well. I thought a shared
structure specific lock, which usually every shared structure is
expected to have, would protect its initialization and content. But I
see that I was wrong. Even those locks need to be initialized; so they
can't be used here. ShmemIndexLock works here with the proposed
comment changes.

--
Best Wishes,
Ashutosh Bapat