public inbox for [email protected]
help / color / mirror / Atom feedFrom: Ashutosh Bapat <[email protected]>
To: Heikki Linnakangas <[email protected]>
Cc: Dagfinn Ilmari Mannsåker <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: pgsql-hackers <[email protected]>
Cc: [email protected]
Subject: Re: Better shared data structure management and resizable shared data structures
Date: Tue, 7 Apr 2026 19:49:28 +0530
Message-ID: <CAExHW5uMQGvQH6GKaBZVtH4S9O13TwN+_0Vy1gUpAW=_T_AmRA@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAExHW5vM1bneLYfg0wGeAa=52UiJ3z4vKd3AJ72X8Fw6k3KKrg@mail.gmail.com>
<[email protected]>
<CA+Tgmob8ot7qam7a650nx_4jyEoFLs8s_kPMc1+EB7-dr56=WA@mail.gmail.com>
<[email protected]>
<CA+TgmoauqYXm8iA3FGRAVKxYShUxWBiS_MSLmQpTrmO7wNHamw@mail.gmail.com>
<[email protected]>
<CAExHW5uTNWOSxJDWQAUnS0tZawob2_J3dRAtc67NHNZ98X4_xA@mail.gmail.com>
<CAExHW5t439y61YD9bc7d5wZWHp6J=M43Qu3eEZOBPguZML7o2A@mail.gmail.com>
<CAExHW5v5FVZbsO9sLzztMZ11C3hgGStE=HkkV2bQkCyncess4w@mail.gmail.com>
<[email protected]>
<CAExHW5tCC0T1ky=Jnq-AvMxa67Adaw7aQ4iQAO=BSdHcbSNBVg@mail.gmail.com>
<[email protected]>
<CAExHW5tS7GncN90oJWOSzW_3F1EHL9xwe59L7Req3nUVgmObUw@mail.gmail.com>
<[email protected]>
<CAExHW5sYgt=XekOoAE-Tu_Dv8obWZCjCqAPT9vtN2D4m=M=drQ@mail.gmail.com>
<[email protected]>
<[email protected]>
<[email protected]>
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. 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.
Here's patch fixing it.
--
Best Wishes,
Ashutosh Bapat
On Tue, Apr 7, 2026 at 6:56 PM Heikki Linnakangas <[email protected]> wrote:
>
> On 07/04/2026 15:24, Dagfinn Ilmari Mannsåker wrote:
> > Heikki Linnakangas <[email protected]> writes:
> >
> >> Those are now committed, and here's a new version rebased over those
> >> changes.
> >
> > I noticed this bit during my habitual morning skim of new commits:
> >
> >> diff --git a/src/backend/utils/misc/injection_point.c b/src/backend/utils/misc/injection_point.c
> >> index c06b0e9b800..9981d6e212f 100644
> >> --- a/src/backend/utils/misc/injection_point.c
> >> +++ b/src/backend/utils/misc/injection_point.c
> >> @@ -17,6 +17,7 @@
> >> */
> >> #include "postgres.h"
> >>
> >> +#include "storage/subsystems.h"
> >> #include "utils/injection_point.h"
> >>
> >> #ifdef USE_INJECTION_POINTS
> >> @@ -109,6 +110,11 @@ typedef struct InjectionPointCacheEntry
> >>
> >> static HTAB *InjectionPointCache = NULL;
> >>
> >> +#ifdef USE_INJECTION_POINTS
> >> +static void InjectionPointShmemRequest(void *arg);
> >> +static void InjectionPointShmemInit(void *arg);
> >> +#endif
> >> +
> >
> > This is already inside an `#ifdef USE_INJECTION_POINTS` guard (in fact
> > visible at the end of the previous diff hunk), no need for another one.
>
> Fixed, thanks. I also noticed that the #include "storage/subsystems.h"
> can be moved inside the #ifdef block; fixed that too.
>
> - Heikki
>
Attachments:
[text/x-patch] v20260407-0001-Unlock-ShmemIndexLock-before-calling-init_.patch (1.7K, 2-v20260407-0001-Unlock-ShmemIndexLock-before-calling-init_.patch)
download | inline diff:
From f08bf94b4f18c7aea9c6e7d3f321c153da7a76d8 Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <[email protected]>
Date: Tue, 7 Apr 2026 19:24:15 +0530
Subject: [PATCH v20260407] Unlock ShmemIndexLock before calling init_fn
callback
CallShmemCallbacksAfterStartup() calls init_fn or attach_fn callbacks
while holding ShmemIndexLock. Those callbacks do not require the lock to
be held. Before d4885af3d65325c1fcd319e98c634fde9a200443, code
initializing the shared structures executed without holding
ShmemIndexLock.
On the other hand, those callbacks may be performing long running
operations like disk access e.g. pgss_shmem_init. Holding a lightweight
lock that long should be avoided, if possible. It will cause a delay in
loading an extension in all the backends since all attach_fns will be
serialized.
Author: Ashutosh Bapat <[email protected]>
---
src/backend/storage/ipc/shmem.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/backend/storage/ipc/shmem.c b/src/backend/storage/ipc/shmem.c
index 1ebffe5a32a..66b95713020 100644
--- a/src/backend/storage/ipc/shmem.c
+++ b/src/backend/storage/ipc/shmem.c
@@ -956,6 +956,8 @@ CallShmemCallbacksAfterStartup(const ShmemCallbacks *callbacks)
list_free_deep(pending_shmem_requests);
pending_shmem_requests = NIL;
+ LWLockRelease(ShmemIndexLock);
+
/* Finish by calling the appropriate subsystem-specific callback */
if (found_any)
{
@@ -968,7 +970,6 @@ CallShmemCallbacksAfterStartup(const ShmemCallbacks *callbacks)
callbacks->init_fn(callbacks->opaque_arg);
}
- LWLockRelease(ShmemIndexLock);
shmem_request_state = SRS_DONE;
}
base-commit: 29e7dbf5e4daa8fafc2b18a1551e7b31c8847340
--
2.34.1
view thread (82+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Better shared data structure management and resizable shared data structures
In-Reply-To: <CAExHW5uMQGvQH6GKaBZVtH4S9O13TwN+_0Vy1gUpAW=_T_AmRA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox