From aa618c279b52c796f1989bf1cb0f294570be1e0b Mon Sep 17 00:00:00 2001
From: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Date: Thu, 6 Aug 2026 13:27:29 +0530
Subject: [PATCH 1/3] Fix stale state after a failed after-startup shmem
 request

CallShmemCallbacksAfterStartup() reset pending_shmem_requests and
shmem_request_state only on success.  If its request callback or a later
allocation failed, it left both behind.  The requests and their List
were allocated in the caller's memory context, so error cleanup could
free the List while pending_shmem_requests still pointed to it.  A retry
in the same backend then crashed:

    TRAP: failed Assert("IsPointerList(list)"), File: "list.c", Line: 341

The stale shmem_request_state also made later calls to
RegisterShmemCallbacks() silently take the startup-registration path
instead of processing the request immediately.

Allocate after-startup requests in a child of the current transaction
context, with a reset callback that clears the List and restores the
state.  Normal error cleanup then handles failures without PG_TRY().
Shared memory already allocated before a failure remains allocated, as
shared memory cannot be freed, but the process-local bookkeeping is
restored.

Extend test_shmem to cover failures while collecting and allocating
requests, retried in one session because the state is backend-local.

Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
---
 src/backend/storage/ipc/shmem.c               | 57 +++++++++++++++++--
 .../test_shmem/t/001_late_shmem_alloc.pl      | 28 +++++++++
 .../modules/test_shmem/test_shmem--1.0.sql    |  4 ++
 src/test/modules/test_shmem/test_shmem.c      | 39 +++++++++++++
 4 files changed, 122 insertions(+), 6 deletions(-)

diff --git a/src/backend/storage/ipc/shmem.c b/src/backend/storage/ipc/shmem.c
index a3d56cf55dd..9986d6a69ce 100644
--- a/src/backend/storage/ipc/shmem.c
+++ b/src/backend/storage/ipc/shmem.c
@@ -142,6 +142,7 @@
 #include "storage/shmem_internal.h"
 #include "storage/spin.h"
 #include "utils/builtins.h"
+#include "utils/memutils.h"
 #include "utils/tuplestore.h"
 
 /*
@@ -168,6 +169,13 @@ typedef struct
 
 static List *pending_shmem_requests;
 
+/*
+ * During an after-startup request, allocate the request list in a context
+ * that is reset on error, so that it cannot become a dangling pointer.
+ */
+static MemoryContext late_shmem_requests_context;
+static MemoryContextCallback late_shmem_requests_callback;
+
 /*
  * Per-process state machine, for sanity checking that we do things in the
  * right order.
@@ -274,6 +282,8 @@ typedef struct
 static bool firstNumaTouch = true;
 
 static void CallShmemCallbacksAfterStartup(const ShmemCallbacks *callbacks);
+static void BeginLateShmemRequests(void);
+static void ForgetLateShmemRequests(void *arg);
 static void InitShmemIndexEntry(ShmemRequest *request);
 static bool AttachShmemIndexEntry(ShmemRequest *request, bool missing_ok);
 
@@ -336,6 +346,7 @@ void
 ShmemRequestInternal(ShmemStructOpts *options, ShmemRequestKind kind)
 {
 	ShmemRequest *request;
+	MemoryContext oldcontext = NULL;
 
 	/* Check the options */
 	if (options->name == NULL)
@@ -374,10 +385,47 @@ ShmemRequestInternal(ShmemStructOpts *options, ShmemRequestKind kind)
 	}
 
 	/* Request looks valid, remember it */
+	if (late_shmem_requests_context != NULL)
+		oldcontext = MemoryContextSwitchTo(late_shmem_requests_context);
 	request = palloc(sizeof(ShmemRequest));
 	request->options = options;
 	request->kind = kind;
 	pending_shmem_requests = lappend(pending_shmem_requests, request);
+	if (oldcontext != NULL)
+		MemoryContextSwitchTo(oldcontext);
+}
+
+/* Start collecting an after-startup request. */
+static void
+BeginLateShmemRequests(void)
+{
+	MemoryContext parent = CurTransactionContext != NULL ?
+		CurTransactionContext : CurrentMemoryContext;
+
+	Assert(pending_shmem_requests == NIL);
+	Assert(late_shmem_requests_context == NULL);
+	Assert(late_shmem_requests_callback.func == NULL);
+
+	late_shmem_requests_context =
+		AllocSetContextCreate(parent, "Pending shmem requests",
+						  ALLOCSET_SMALL_SIZES);
+	late_shmem_requests_callback.func = ForgetLateShmemRequests;
+	late_shmem_requests_callback.arg = NULL;
+	MemoryContextRegisterResetCallback(late_shmem_requests_context,
+									   &late_shmem_requests_callback);
+}
+
+/* Discard an after-startup request and reset the process-local state. */
+static void
+ForgetLateShmemRequests(void *arg)
+{
+	foreach_ptr(ShmemRequest, request, pending_shmem_requests)
+		pfree(request->options);
+
+	pending_shmem_requests = NIL;
+	late_shmem_requests_context = NULL;
+	late_shmem_requests_callback.func = NULL;
+	shmem_request_state = SRS_DONE;
 }
 
 /*
@@ -901,6 +949,7 @@ CallShmemCallbacksAfterStartup(const ShmemCallbacks *callbacks)
 	bool		notfound_any;
 
 	Assert(shmem_request_state == SRS_DONE);
+	BeginLateShmemRequests();
 	shmem_request_state = SRS_REQUESTING;
 
 	/*
@@ -914,7 +963,7 @@ CallShmemCallbacksAfterStartup(const ShmemCallbacks *callbacks)
 
 	if (pending_shmem_requests == NIL)
 	{
-		shmem_request_state = SRS_DONE;
+		MemoryContextDelete(late_shmem_requests_context);
 		return;
 	}
 
@@ -952,11 +1001,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 +1016,7 @@ CallShmemCallbacksAfterStartup(const ShmemCallbacks *callbacks)
 	}
 
 	LWLockRelease(ShmemIndexLock);
-	shmem_request_state = SRS_DONE;
+	MemoryContextDelete(late_shmem_requests_context);
 }
 
 /*
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..a9481997d52 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
@@ -25,6 +25,34 @@ my $attach_count2 =
   $node->safe_psql("postgres", "SELECT get_test_shmem_attach_count();");
 cmp_ok($attach_count2, '>', $attach_count1,
 	"attach callback is called in each backend");
+
+sub try_shmem_failure_twice
+{
+	my ($mode) = @_;
+	my $sql = qq[
+DO \$\$
+BEGIN
+	FOR i IN 1..2 LOOP
+		BEGIN
+			PERFORM test_shmem_failure($mode);
+		EXCEPTION WHEN others THEN
+			RAISE NOTICE 'attempt %: %', i, SQLERRM;
+		END;
+	END LOOP;
+END
+\$\$;];
+	return $node->psql('postgres', $sql);
+}
+
+# The state is backend-local, so the two attempts must share a session.
+foreach my $mode (0, 1)
+{
+	my ($ret, $stdout, $stderr) = try_shmem_failure_twice($mode);
+
+	is($ret, 0, "session survives repeated failing shmem request $mode");
+	like($stderr, qr/attempt 1: /, "shmem request $mode fails");
+	like($stderr, qr/attempt 2: /, "shmem request $mode fails when retried");
+}
 $node->stop;
 
 ###
diff --git a/src/test/modules/test_shmem/test_shmem--1.0.sql b/src/test/modules/test_shmem/test_shmem--1.0.sql
index 2d01fd9256c..62d3458fea9 100644
--- a/src/test/modules/test_shmem/test_shmem--1.0.sql
+++ b/src/test/modules/test_shmem/test_shmem--1.0.sql
@@ -7,3 +7,7 @@
 CREATE FUNCTION get_test_shmem_attach_count()
 RETURNS pg_catalog.int4 STRICT
 AS 'MODULE_PATHNAME' LANGUAGE C;
+
+CREATE FUNCTION test_shmem_failure(pg_catalog.int4)
+RETURNS pg_catalog.void STRICT
+AS 'MODULE_PATHNAME' LANGUAGE C;
diff --git a/src/test/modules/test_shmem/test_shmem.c b/src/test/modules/test_shmem/test_shmem.c
index 9bd4012b435..348f26877af 100644
--- a/src/test/modules/test_shmem/test_shmem.c
+++ b/src/test/modules/test_shmem/test_shmem.c
@@ -38,6 +38,7 @@ static bool attached_or_initialized = false;
 static void test_shmem_request(void *arg);
 static void test_shmem_init(void *arg);
 static void test_shmem_attach(void *arg);
+static void test_shmem_failure_request(void *arg);
 
 static const ShmemCallbacks TestShmemCallbacks = {
 	.flags = SHMEM_CALLBACKS_ALLOW_AFTER_STARTUP,
@@ -46,6 +47,13 @@ static const ShmemCallbacks TestShmemCallbacks = {
 	.attach_fn = test_shmem_attach,
 };
 
+static const ShmemCallbacks TestShmemFailureCallbacks = {
+	.flags = SHMEM_CALLBACKS_ALLOW_AFTER_STARTUP,
+	.request_fn = test_shmem_failure_request,
+};
+
+static int	failure_mode;
+
 static void
 test_shmem_request(void *arg)
 {
@@ -82,6 +90,28 @@ test_shmem_attach(void *arg)
 	attached_or_initialized = true;
 }
 
+/* Request callback used by the after-startup failure tests. */
+static void
+test_shmem_failure_request(void *arg)
+{
+	static void *ptr1;
+
+	switch (failure_mode)
+	{
+		case 0:
+			ShmemRequestStruct(.name = "test_shmem callback error area",
+							   .size = 1024, .ptr = &ptr1);
+			elog(ERROR, "test_shmem request callback failed on purpose");
+		case 1:
+			ShmemRequestStruct(.name = "test_shmem oversized area",
+							   .size = (Size) 1024 * 1024 * 1024,
+							   .ptr = &ptr1);
+			break;
+		default:
+			elog(ERROR, "unrecognized test_shmem failure mode: %d", failure_mode);
+	}
+}
+
 void
 _PG_init(void)
 {
@@ -89,6 +119,15 @@ _PG_init(void)
 	RegisterShmemCallbacks(&TestShmemCallbacks);
 }
 
+PG_FUNCTION_INFO_V1(test_shmem_failure);
+Datum
+test_shmem_failure(PG_FUNCTION_ARGS)
+{
+	failure_mode = PG_GETARG_INT32(0);
+	RegisterShmemCallbacks(&TestShmemFailureCallbacks);
+	PG_RETURN_VOID();
+}
+
 PG_FUNCTION_INFO_V1(get_test_shmem_attach_count);
 Datum
 get_test_shmem_attach_count(PG_FUNCTION_ARGS)
-- 
2.34.1

