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 >
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
