On 18/08/2026 13:37, Ayush Tiwari wrote:
On Wed, 12 Aug 2026 at 15:12, Ashutosh Bapat
<[email protected] <mailto:[email protected]>> wrote:
On Wed, Aug 12, 2026 at 2:37 PM Ayush Tiwari
<[email protected] <mailto:[email protected]>>
wrote:
> Thanks for the latest patch, it looks good to me.
>
Thanks.
> I just have 2 minor editorial nits:
>
> - The commit message mentions
CallShmemCallbacksAfterStartupCleanup(), but I
> think that should be CallShmemCallbacksAfterStartup().
>
> - "unitialized" in xfunc.sgml should be "uninitialized".
>
> These can be adjusted while committing so I'm not sending an
updated patch.
>
Attached patch fixes those two as well.
Thanks! And sorry for not picking this up earlier.
Here's my take with some small changes. This is all based on things you
already discussed, I just picked the mix I liked best:
* I added the 'initialized' flag to each shmem index entry. It feels a
little heavy-handed just to deal with failed initializations after
postmaster startup, which is a rare codepath to begin with, but then
again, it's just a boolean. You still can't really retry or recover from
a failed initialization, the entry stays in the broken state until
restart, but at least you get a nicer error message and you don't need
worry about detecting it in your extension. As evidenced by smaller
changes needed to the docs to describe it :-).
* I kept the pending_shmem_requests list in the caller's memory context.
Instead of freeing it in PG_FINALLY, I just set pending_shmem_requests =
NIL, and rely on the memory context release to free it. (I was on the
fence on this, TopMemoryContext would also be quite reasonable)
* I structured the PG_TRY/CATCH block more like you had it in the
earlier patches, adding a little helper function.
What do you think?
- Heikki
From d34c03b034a8ac6d806468dde16810fadaa5cbcc Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <[email protected]>
Date: Tue, 25 Aug 2026 19:00:58 +0300
Subject: [PATCH 1/3] Fix backend state after a failed after-startup shmem
request
RegisterShmemCallbacks() left the backend in a bad state, if an error
occurred in the callbacks or if an allocation failed. Firstly,
'shmem_request_state' was left in wrong state, causing a subsequent
call to RegisterShmemCallbacks() to wrongly take the postmaster
startup codepath or assertion failures in some other functions.
Secondly, the 'pending_shmem_requests' list was not properly cleaned
up, causing a subsequent RegisterShmemCallbacks() to try to process
the stale, already-freed requests.
To fix, add a PG_TRY() block to clean those things up on error.
Author: Ayush Tiwari <[email protected]>
Reviewed-by: Ashutosh Bapat <[email protected]>
Discussion: https://www.postgresql.org/message-id/CAJTYsWVRRWH48=pcuao_2y4ap6m0qrmzxguffknrtdwk74l...@mail.gmail.com
Backpatch-through: 19
---
src/backend/storage/ipc/shmem.c | 58 +++++++++++++++++++++------------
1 file changed, 38 insertions(+), 20 deletions(-)
diff --git a/src/backend/storage/ipc/shmem.c b/src/backend/storage/ipc/shmem.c
index 228871d2525..0fadbf85a02 100644
--- a/src/backend/storage/ipc/shmem.c
+++ b/src/backend/storage/ipc/shmem.c
@@ -274,6 +274,7 @@ typedef struct
static bool firstNumaTouch = true;
static void CallShmemCallbacksAfterStartup(const ShmemCallbacks *callbacks);
+static void ProcessShmemRequestsAfterStartup(const ShmemCallbacks *callbacks);
static void InitShmemIndexEntry(ShmemRequest *request);
static bool AttachShmemIndexEntry(ShmemRequest *request, bool missing_ok);
@@ -897,26 +898,44 @@ RegisterShmemCallbacks(const ShmemCallbacks *callbacks)
static void
CallShmemCallbacksAfterStartup(const ShmemCallbacks *callbacks)
{
- bool found_any;
- bool notfound_any;
-
Assert(shmem_request_state == SRS_DONE);
- shmem_request_state = SRS_REQUESTING;
-
- /*
- * Call the request callback first. The callback makes ShmemRequest*()
- * calls for each shmem area, adding them to pending_shmem_requests.
- */
Assert(pending_shmem_requests == NIL);
- if (callbacks->request_fn)
- callbacks->request_fn(callbacks->opaque_arg);
- shmem_request_state = SRS_AFTER_STARTUP_ATTACH_OR_INIT;
- if (pending_shmem_requests == NIL)
+ PG_TRY();
+ {
+ /*
+ * Call the request callback first. The callback makes
+ * ShmemRequest*() calls for each shmem area, adding them to
+ * pending_shmem_requests.
+ */
+ shmem_request_state = SRS_REQUESTING;
+ if (callbacks->request_fn)
+ callbacks->request_fn(callbacks->opaque_arg);
+
+ /* Process all the requests */
+ shmem_request_state = SRS_AFTER_STARTUP_ATTACH_OR_INIT;
+ if (pending_shmem_requests != NIL)
+ ProcessShmemRequestsAfterStartup(callbacks);
+ }
+ PG_FINALLY();
{
shmem_request_state = SRS_DONE;
- return;
+ pending_shmem_requests = NIL;
}
+ PG_END_TRY();
+}
+
+static void
+ProcessShmemRequestsAfterStartup(const ShmemCallbacks *callbacks)
+{
+ bool found_any;
+ bool notfound_any;
+
+ /* There should be some requests to process */
+ Assert(pending_shmem_requests != NIL);
+
+ /* Caller manages the global state variable */
+ Assert(shmem_request_state == SRS_AFTER_STARTUP_ATTACH_OR_INIT);
/*
* Hold ShmemIndexLock while we allocate all the shmem entries and run all
@@ -934,7 +953,11 @@ CallShmemCallbacksAfterStartup(const ShmemCallbacks *callbacks)
found_any = notfound_any = false;
foreach_ptr(ShmemRequest, request, pending_shmem_requests)
{
- if (hash_search(ShmemIndex, request->options->name, HASH_FIND, NULL))
+ ShmemIndexEnt *index_entry;
+
+ index_entry = (ShmemIndexEnt *)
+ hash_search(ShmemIndex, request->options->name, HASH_FIND, NULL);
+ if (index_entry)
found_any = true;
else
notfound_any = true;
@@ -952,11 +975,7 @@ CallShmemCallbacksAfterStartup(const ShmemCallbacks *callbacks)
AttachShmemIndexEntry(request, false);
else
InitShmemIndexEntry(request);
-
- pfree(request->options);
}
- list_free_deep(pending_shmem_requests);
- pending_shmem_requests = NIL;
/* Finish by calling the appropriate subsystem-specific callback */
if (found_any)
@@ -971,7 +990,6 @@ CallShmemCallbacksAfterStartup(const ShmemCallbacks *callbacks)
}
LWLockRelease(ShmemIndexLock);
- shmem_request_state = SRS_DONE;
}
/*
--
2.47.3
From 6852c567d367451afa89941f4587da6c1df8d677 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <[email protected]>
Date: Tue, 25 Aug 2026 19:05:39 +0300
Subject: [PATCH 2/3] Track which shmem areas have been fully initialized
If SHMEM_CALLBACKS_ALLOW_AFTER_STARTUP is used to allocate shared
memory after startup, but the initialization fails half-way through,
the shmem area is left in an indeterminate state. Furthermore, if
multiple shmem areas are registered in one RegisterShmemCallbacks()
call, some might be allocated while others are not.
This commit adds an explicit 'initialized' flag to each shmem area.
We still leave behind an uninitialized area on error, but at least
they are now clearly marked, and you get a slightly nicer error
message if you try to re-register them. It'd be nice to clean up more
thoroughly and support actually retrying the allocations, but in
practice, the most likely reason for a shmem allocation or
initialization to fail is that you are out of shared memory and
retrying wouldn't help with that.
This isn't exactly a new problem, the old ShmemInitStruct() interface
had similar issues if the initialization code failed, or if you
allocated multiple structs and some allocations failed. It was just
left to the calling code to deal with it.
Author: Ayush Tiwari <[email protected]>
Reviewed-by: Ashutosh Bapat <[email protected]>
Discussion: https://www.postgresql.org/message-id/CAJTYsWVRRWH48=pcuao_2y4ap6m0qrmzxguffknrtdwk74l...@mail.gmail.com
Backpatch-through: 19
---
doc/src/sgml/xfunc.sgml | 5 ++-
src/backend/storage/ipc/shmem.c | 67 ++++++++++++++++++++++++++++++---
2 files changed, 66 insertions(+), 6 deletions(-)
diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
index 97f3cb625e2..a90dba0662f 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -3740,7 +3740,10 @@ my_shmem_init(void *arg)
on whether the requested memory areas were already initialized by
another backend. The callbacks will be called while holding an internal
lock (ShmemIndexLock), which prevents the race condition of two backends
- trying to initialize the memory area at the same time.
+ trying to initialize the memory area at the same time. If the
+ allocation or initialization fails for any reason, the shared memory
+ areas are left in an abandoned state and any attempt to attach or
+ re-initialize them will fail until the server is restarted.
</para>
</sect3>
diff --git a/src/backend/storage/ipc/shmem.c b/src/backend/storage/ipc/shmem.c
index 0fadbf85a02..3f6a69212ae 100644
--- a/src/backend/storage/ipc/shmem.c
+++ b/src/backend/storage/ipc/shmem.c
@@ -144,6 +144,8 @@
#include "utils/builtins.h"
#include "utils/tuplestore.h"
+typedef struct ShmemIndexEnt ShmemIndexEnt;
+
/*
* Registered callbacks.
*
@@ -164,6 +166,9 @@ typedef struct
{
ShmemStructOpts *options;
ShmemRequestKind kind;
+
+ /* InitShmemIndexEntry() sets this pointer when the area is allocated *
+ ShmemIndexEnt *index_entry;
} ShmemRequest;
static List *pending_shmem_requests;
@@ -262,12 +267,13 @@ static HTAB *ShmemIndex;
#define SHMEM_INDEX_ADDITIONAL_SIZE (128)
/* this is a hash bucket in the shmem index table */
-typedef struct
+typedef struct ShmemIndexEnt
{
char key[SHMEM_INDEX_KEYSIZE]; /* string name */
void *location; /* location in shared mem */
Size size; /* # bytes requested for the structure */
Size allocated_size; /* # bytes actually allocated */
+ bool initialized; /* has the init callback been run? */
} ShmemIndexEnt;
/* To get reliable results for NUMA inquiry we need to "touch pages" once */
@@ -378,6 +384,7 @@ ShmemRequestInternal(ShmemStructOpts *options, ShmemRequestKind kind)
request = palloc_object(ShmemRequest);
request->options = options;
request->kind = kind;
+ request->index_entry = NULL;
pending_shmem_requests = lappend(pending_shmem_requests, request);
}
@@ -436,10 +443,7 @@ ShmemInitRequested(void)
foreach_ptr(ShmemRequest, request, pending_shmem_requests)
{
InitShmemIndexEntry(request);
- pfree(request->options);
}
- list_free_deep(pending_shmem_requests);
- pending_shmem_requests = NIL;
/*
* Call the subsystem-specific init callbacks to finish initialization of
@@ -451,6 +455,15 @@ ShmemInitRequested(void)
callbacks->init_fn(callbacks->opaque_arg);
}
+ /* Now we can mark all the areas as initialized and free the requests */
+ foreach_ptr(ShmemRequest, request, pending_shmem_requests)
+ {
+ request->index_entry->initialized = true;
+ pfree(request->options);
+ }
+ list_free_deep(pending_shmem_requests);
+ pending_shmem_requests = NIL;
+
shmem_request_state = SRS_DONE;
}
@@ -552,7 +565,12 @@ InitShmemIndexEntry(ShmemRequest *request)
index_entry->allocated_size = allocated_size;
index_entry->location = structPtr;
- /* Initialize depending on the kind of shmem area it is */
+ /*
+ * The area is considered fully initialized only after the subsystem's
+ * init callback has been called. For now, perform only basic
+ * initialization based on the kind of shmem area it is.
+ */
+ index_entry->initialized = false;
switch (request->kind)
{
case SHMEM_KIND_STRUCT:
@@ -566,6 +584,9 @@ InitShmemIndexEntry(ShmemRequest *request)
shmem_slru_init(structPtr, request->options);
break;
}
+
+ /* return the pointer to the entry to the caller */
+ request->index_entry = index_entry;
}
/*
@@ -595,6 +616,20 @@ AttachShmemIndexEntry(ShmemRequest *request, bool missing_ok)
return false;
}
+ /*
+ * If it was previously allocated but not fully initialized, error out.
+ * There is currently no way of retrying or cleaning up an uninitialized
+ * entry, it just lingers until the server is shut down. But this can
+ * only happen when allocating areas after postmaster startup, and it's
+ * unlikely that you could successfully retry anyway. The most likely
+ * reason for failed initialization is that you are out of shared memory
+ * and retrying won't help with that.
+ */
+ if (!index_entry->initialized)
+ ereport(ERROR,
+ (errmsg("cannot attach to shared memory struct \"%s\" because it was not fully initialized",
+ request->options->name)));
+
/* Check that the size in the index matches the request */
if (index_entry->size != request->options->size &&
request->options->size != SHMEM_ATTACH_UNKNOWN_SIZE)
@@ -623,6 +658,8 @@ AttachShmemIndexEntry(ShmemRequest *request, bool missing_ok)
break;
}
+ request->index_entry = index_entry;
+
return true;
}
@@ -733,6 +770,7 @@ InitShmemAllocator(PGShmemHeader *seghdr)
result->size = ShmemAllocator->index_size;
result->allocated_size = ShmemAllocator->index_size;
result->location = ShmemAllocator->index;
+ result->initialized = true;
}
}
@@ -958,7 +996,17 @@ ProcessShmemRequestsAfterStartup(const ShmemCallbacks *callbacks)
index_entry = (ShmemIndexEnt *)
hash_search(ShmemIndex, request->options->name, HASH_FIND, NULL);
if (index_entry)
+ {
+ /*
+ * Check for a half-initialized area. (See also similar check in
+ * AttachShmemIndexEntry())
+ */
+ if (!index_entry->initialized)
+ ereport(ERROR,
+ (errmsg("cannot attach to shared memory struct \"%s\" because it was not fully initialized",
+ request->options->name)));
found_any = true;
+ }
else
notfound_any = true;
}
@@ -989,6 +1037,11 @@ ProcessShmemRequestsAfterStartup(const ShmemCallbacks *callbacks)
callbacks->init_fn(callbacks->opaque_arg);
}
+ foreach_ptr(ShmemRequest, request, pending_shmem_requests)
+ {
+ request->index_entry->initialized = true;
+ }
+
LWLockRelease(ShmemIndexLock);
}
@@ -1053,7 +1106,11 @@ ShmemInitStruct(const char *name, Size size, bool *foundPtr)
/* Initialize it if not found */
if (!*foundPtr)
+ {
InitShmemIndexEntry(&request);
+ /* no additional initialization needed */
+ request.index_entry->initialized = true;
+ }
LWLockRelease(ShmemIndexLock);
--
2.47.3
From 82472c69efe4c7d8d07023eb4ef3b08bb33b97f2 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <[email protected]>
Date: Tue, 25 Aug 2026 19:12:09 +0300
Subject: [PATCH 3/3] Add tests for failing shmem allocations after startup
To cover the failure cases that the previous commits hardened.
Author: Ayush Tiwari <[email protected]>
Reviewed-by: Ashutosh Bapat <[email protected]>
Discussion: https://www.postgresql.org/message-id/CAJTYsWVRRWH48=pcuao_2y4ap6m0qrmzxguffknrtdwk74l...@mail.gmail.com
Backpatch-through: 19
---
src/test/modules/test_shmem/Makefile | 3 ++
src/test/modules/test_shmem/meson.build | 3 ++
.../test_shmem/t/001_late_shmem_alloc.pl | 47 ++++++++++++++++++-
src/test/modules/test_shmem/test_shmem.c | 26 +++++++++-
4 files changed, 76 insertions(+), 3 deletions(-)
diff --git a/src/test/modules/test_shmem/Makefile b/src/test/modules/test_shmem/Makefile
index 2407f7462fe..fed8e29c8f5 100644
--- a/src/test/modules/test_shmem/Makefile
+++ b/src/test/modules/test_shmem/Makefile
@@ -2,6 +2,9 @@
PGFILEDESC = "test_shmem - test code for shmem allocations"
+EXTRA_INSTALL = src/test/modules/injection_points
+export enable_injection_points
+
MODULE_big = test_shmem
OBJS = \
$(WIN32RES) \
diff --git a/src/test/modules/test_shmem/meson.build b/src/test/modules/test_shmem/meson.build
index fb4bf328b8f..8f98f2c4e31 100644
--- a/src/test/modules/test_shmem/meson.build
+++ b/src/test/modules/test_shmem/meson.build
@@ -26,6 +26,9 @@ tests += {
'sd': meson.current_source_dir(),
'bd': meson.current_build_dir(),
'tap': {
+ 'env': {
+ 'enable_injection_points': get_option('injection_points') ? 'yes' : 'no',
+ },
'tests': [
't/001_late_shmem_alloc.pl',
],
diff --git a/src/test/modules/test_shmem/t/001_late_shmem_alloc.pl b/src/test/modules/test_shmem/t/001_late_shmem_alloc.pl
index 5cf07d071ec..b09889f4fb0 100644
--- a/src/test/modules/test_shmem/t/001_late_shmem_alloc.pl
+++ b/src/test/modules/test_shmem/t/001_late_shmem_alloc.pl
@@ -15,8 +15,50 @@ my $node = PostgreSQL::Test::Cluster->new('main');
$node->init;
$node->start;
+# Test a failure in initialization of the shared memory area.
+SKIP:
+{
+ skip "injection points not supported by this build",
+ if $ENV{enable_injection_points} ne 'yes';
+ $node->safe_psql("postgres", "CREATE EXTENSION injection_points;");
+ $node->safe_psql("postgres",
+ "SELECT injection_points_attach('test-shmem-init', 'error');");
+
+ # A failure in the requesting shared memory should not affect server
+ # availability. The session should remain useful, however trying to create
+ # the same extension again will fail.
+ my (undef, undef, $stderr) =
+ $node->psql("postgres", "CREATE EXTENSION test_shmem;");
+ like($stderr,
+ qr/error triggered for injection point test-shmem-init/,
+ "failure in initialization is reported");
+ $node->safe_psql("postgres",
+ "SELECT injection_points_detach('test-shmem-init');");
+ (undef, undef, $stderr) =
+ $node->psql("postgres", "CREATE EXTENSION test_shmem;");
+ like($stderr, qr/cannot attach to shared memory/,
+ "post-init extension creation fails");
+
+ # Only a server restart can remove the partially allocated shared memory
+ # area.
+ $node->restart;
+}
+
+# Test failure when the request is larger than the memory reserved for
+# after-startup requests.
+my $session = $node->background_psql('postgres', on_error_stop => 0);
+$session->query(q[SET test_shmem.area_size = '128kB';], verbose => 0);
+$session->query("CREATE EXTENSION test_shmem;", verbose => 0);
+like($session->{stderr}, qr/not enough shared memory/,
+ "an after-startup request larger than the reserve fails");
-$node->safe_psql("postgres", "CREATE EXTENSION test_shmem;");
+# The server and the backend should still be available. Since there was only one
+# area requested, the failure did not change anything in the shared memory.
+# Verify that the request for smaller area succeeds in the same session.
+$session->{stderr} = '';
+$session->query("SET test_shmem.area_size = default;", verbose => 0);
+$session->query_safe("CREATE EXTENSION test_shmem;", verbose => 0);
+$session->quit;
# Check that the attach counter is incremented on a new connection
my $attach_count1 =
@@ -28,8 +70,9 @@ cmp_ok($attach_count2, '>', $attach_count1,
$node->stop;
###
-# Test that loading via shared_preload_libraries also works
+# Test that loading via shared_preload_libraries also works, even for large request.
###
+$node->append_conf('postgresql.conf', "test_shmem.area_size = '128kB'");
$node->append_conf('postgresql.conf',
"shared_preload_libraries = 'test_shmem'");
$node->start;
diff --git a/src/test/modules/test_shmem/test_shmem.c b/src/test/modules/test_shmem/test_shmem.c
index 9bd4012b435..2956f488135 100644
--- a/src/test/modules/test_shmem/test_shmem.c
+++ b/src/test/modules/test_shmem/test_shmem.c
@@ -17,9 +17,13 @@
#include "postgres.h"
+#include <limits.h>
+
#include "fmgr.h"
#include "miscadmin.h"
#include "storage/shmem.h"
+#include "utils/guc.h"
+#include "utils/injection_point.h"
PG_MODULE_MAGIC;
@@ -29,11 +33,14 @@ typedef struct TestShmemData
int value;
bool initialized;
int attach_count;
+ char variable_sized_array[FLEXIBLE_ARRAY_MEMBER];
} TestShmemData;
static TestShmemData *TestShmem;
static bool attached_or_initialized = false;
+static int test_shmem_area_size = sizeof(TestShmemData);
+static bool test_shmem_guc_defined = false;
static void test_shmem_request(void *arg);
static void test_shmem_init(void *arg);
@@ -52,7 +59,7 @@ test_shmem_request(void *arg)
elog(LOG, "test_shmem_request callback called");
ShmemRequestStruct(.name = "test_shmem area",
- .size = sizeof(TestShmemData),
+ .size = test_shmem_area_size,
.ptr = (void **) &TestShmem);
}
@@ -60,6 +67,8 @@ static void
test_shmem_init(void *arg)
{
elog(LOG, "init callback called");
+ /* Induce an error while initializing shared structure. */
+ INJECTION_POINT("test-shmem-init", NULL);
if (TestShmem->initialized)
elog(ERROR, "shmem area already initialized");
TestShmem->initialized = true;
@@ -86,6 +95,21 @@ void
_PG_init(void)
{
elog(LOG, "test_shmem module's _PG_init called");
+
+ if (!test_shmem_guc_defined)
+ {
+ DefineCustomIntVariable("test_shmem.area_size",
+ "Size of the shmem area to request.",
+ NULL,
+ &test_shmem_area_size,
+ sizeof(TestShmemData),
+ sizeof(TestShmemData), INT_MAX,
+ PGC_USERSET,
+ GUC_UNIT_BYTE,
+ NULL, NULL, NULL);
+ MarkGUCPrefixReserved("test_shmem");
+ test_shmem_guc_defined = true;
+ }
RegisterShmemCallbacks(&TestShmemCallbacks);
}
--
2.47.3
