Content-Type: multipart/mixed; boundary="------------grpQeMCQxKwFtDoP0jwnhuSq"
Message-ID: <555608f6-63bb-4dda-b1e4-f378f0febd38@iki.fi>
Date: Fri, 12 Jun 2026 18:37:21 +0300
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Better shared data structure management and resizable shared data
 structures
To: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
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
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>
Content-Language: en-US
From: Heikki Linnakangas <hlinnaka@iki.fi>
In-Reply-To: 
 <CAExHW5uDJdmfUVX=meyecbY2sGKq07Y33NQ68Wv2-ktaag=9qg@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/555608f6-63bb-4dda-b1e4-f378f0febd38%40iki.fi>
Precedence: bulk

This is a multi-part message in MIME format.
--------------grpQeMCQxKwFtDoP0jwnhuSq
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

On 21/04/2026 19:05, Ashutosh Bapat wrote:
> On Tue, Apr 21, 2026 at 1:10 PM Heikki Linnakangas <hlinnaka@iki.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 after
>>         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 the
>> 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 another
>> 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=_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.

- Heikki

--------------grpQeMCQxKwFtDoP0jwnhuSq
Content-Type: text/x-patch; charset=UTF-8;
 name="improve-shmem-init-callback-locking-docs.patch"
Content-Disposition: attachment;
 filename="improve-shmem-init-callback-locking-docs.patch"
Content-Transfer-Encoding: base64

ZGlmZiAtLWdpdCBhL2RvYy9zcmMvc2dtbC94ZnVuYy5zZ21sIGIvZG9jL3NyYy9zZ21sL3hm
dW5jLnNnbWwKaW5kZXggYmFlMTZkN2ZiNTMuLmNiM2NjMDlmMTZkIDEwMDY0NAotLS0gYS9k
b2Mvc3JjL3NnbWwveGZ1bmMuc2dtbAorKysgYi9kb2Mvc3JjL3NnbWwveGZ1bmMuc2dtbApA
QCAtMzczOSw4ICszNzM5LDggQEAgbXlfc2htZW1faW5pdCh2b2lkICphcmcpCiAgICAgICBz
dGFydHVwLCBpdCB3aWxsIGltbWVkaWF0ZWx5IGNhbGwgdGhlIGFwcHJvcHJpYXRlIGNhbGxi
YWNrcywgZGVwZW5kaW5nCiAgICAgICBvbiB3aGV0aGVyIHRoZSByZXF1ZXN0ZWQgbWVtb3J5
IGFyZWFzIHdlcmUgYWxyZWFkeSBpbml0aWFsaXplZCBieQogICAgICAgYW5vdGhlciBiYWNr
ZW5kLiBUaGUgY2FsbGJhY2tzIHdpbGwgYmUgY2FsbGVkIHdoaWxlIGhvbGRpbmcgYW4gaW50
ZXJuYWwKLSAgICAgIGxvY2ssIHdoaWNoIHByZXZlbnRzIGNvbmN1cnJlbnQgdHdvIGJhY2tl
bmRzIGZyb20gaW5pdGlhbGl6aW5nIHRoZQotICAgICAgbWVtb3J5IGFyZWEgY29uY3VycmVu
dGx5LgorICAgICAgbG9jayAoU2htZW1JbmRleExvY2spLCB3aGljaCBwcmV2ZW50cyB0aGUg
cmFjZSBjb25kaXRpb24gb2YgdHdvIGJhY2tlbmRzCisgICAgICB0cnlpbmcgdG8gaW5pdGlh
bGl6aW5nIHRoZSBtZW1vcnkgYXJlYSBhdCB0aGUgc2FtZSB0aW1lLgogICAgICA8L3BhcmE+
CiAgICAgPC9zZWN0Mz4KIApkaWZmIC0tZ2l0IGEvc3JjL2JhY2tlbmQvc3RvcmFnZS9pcGMv
c2htZW0uYyBiL3NyYy9iYWNrZW5kL3N0b3JhZ2UvaXBjL3NobWVtLmMKaW5kZXggZjFmN2Nk
M2E0ZmYuLjFmYmJhOWMzYTRjIDEwMDY0NAotLS0gYS9zcmMvYmFja2VuZC9zdG9yYWdlL2lw
Yy9zaG1lbS5jCisrKyBiL3NyYy9iYWNrZW5kL3N0b3JhZ2UvaXBjL3NobWVtLmMKQEAgLTkx
OCw3ICs5MTgsMTAgQEAgQ2FsbFNobWVtQ2FsbGJhY2tzQWZ0ZXJTdGFydHVwKGNvbnN0IFNo
bWVtQ2FsbGJhY2tzICpjYWxsYmFja3MpCiAJCXJldHVybjsKIAl9CiAKLQkvKiBIb2xkIFNo
bWVtSW5kZXhMb2NrIHdoaWxlIHdlIGFsbG9jYXRlIGFsbCB0aGUgc2htZW0gZW50cmllcyAq
LworCS8qCisJICogSG9sZCBTaG1lbUluZGV4TG9jayB3aGlsZSB3ZSBhbGxvY2F0ZSBhbGwg
dGhlIHNobWVtIGVudHJpZXMgYW5kIHJ1biBhbGwKKwkgKiB0aGUgaW5pdGlhbGl6ZXJzLgor
CSAqLwogCUxXTG9ja0FjcXVpcmUoU2htZW1JbmRleExvY2ssIExXX0VYQ0xVU0lWRSk7CiAK
IAkvKgpAQCAtOTM3LDcgKzk0MCw3IEBAIENhbGxTaG1lbUNhbGxiYWNrc0FmdGVyU3RhcnR1
cChjb25zdCBTaG1lbUNhbGxiYWNrcyAqY2FsbGJhY2tzKQogCQkJbm90Zm91bmRfYW55ID0g
dHJ1ZTsKIAl9CiAJaWYgKGZvdW5kX2FueSAmJiBub3Rmb3VuZF9hbnkpCi0JCWVsb2coRVJS
T1IsICJmb3VuZCBzb21lIGJ1dCBub3QgYWxsIik7CisJCWVsb2coRVJST1IsICJzb21lIG9m
IHRoZSByZXF1ZXN0ZWQgc2htZW0gYXJlYXMgaGF2ZSBhbHJlYWR5IGJlZW4gaW5pdGlhbGl6
ZWQiKTsKIAogCS8qCiAJICogQWxsb2NhdGUgb3IgYXR0YWNoIGFsbCB0aGUgc2htZW0gYXJl
YXMgcmVxdWVzdGVkIGJ5IHRoZSByZXF1ZXN0X2ZuCg==

--------------grpQeMCQxKwFtDoP0jwnhuSq--