v23-0001-Temp-context-for-maybe_reread_subscription.patch

From 2939a576908866394795460ac509cea369280b2b Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Thu, 12 Mar 2026 18:04:35 -0700
Subject: [PATCH v23 1/4] Temp context for maybe_reread_subscription().

Move temp context from ForeignServerConnectionString() to
maybe_reread_subscription(), so that it prevents more
invalidation-related leaks. Remove PG_TRY()/PG_FINALLY() from
ForeignServerConnectionString().

Suggested-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/xvdjrdqnpap3uq7owbaox3r7p5gf7sv62aaqf2ju3vb6yglatr%40kvvwhoudrlxq
---
 src/backend/catalog/pg_subscription.c    | 14 -----
 src/backend/foreign/foreign.c            | 66 +++++++-----------------
 src/backend/replication/logical/worker.c | 30 +++++++++--
 src/include/catalog/pg_subscription.h    |  1 -
 4 files changed, 43 insertions(+), 68 deletions(-)

diff --git a/src/backend/catalog/pg_subscription.c b/src/backend/catalog/pg_subscription.c
index 3673d4f0bc1..ca053c152cf 100644
--- a/src/backend/catalog/pg_subscription.c
+++ b/src/backend/catalog/pg_subscription.c
@@ -216,20 +216,6 @@ CountDBSubscriptions(Oid dbid)
 	return nsubs;
 }
 
-/*
- * Free memory allocated by subscription struct.
- */
-void
-FreeSubscription(Subscription *sub)
-{
-	pfree(sub->name);
-	pfree(sub->conninfo);
-	if (sub->slotname)
-		pfree(sub->slotname);
-	list_free_deep(sub->publications);
-	pfree(sub);
-}
-
 /*
  * Disable the given subscription.
  */
diff --git a/src/backend/foreign/foreign.c b/src/backend/foreign/foreign.c
index 2edfac68d9b..1b53ca306a0 100644
--- a/src/backend/foreign/foreign.c
+++ b/src/backend/foreign/foreign.c
@@ -220,62 +220,32 @@ GetForeignServerByName(const char *srvname, bool missing_ok)
 
 /*
  * Retrieve connection string from server's FDW.
+ *
+ * NB: leaks into CurrentMemoryContext.
  */
 char *
 ForeignServerConnectionString(Oid userid, Oid serverid)
 {
-	MemoryContext tempContext;
-	MemoryContext oldcxt;
-	text	   *volatile connection_text = NULL;
-	char	   *result = NULL;
-
-	/*
-	 * GetForeignServer, GetForeignDataWrapper, and the connection function
-	 * itself all leak memory into CurrentMemoryContext. Switch to a temporary
-	 * context for easy cleanup.
-	 */
-	tempContext = AllocSetContextCreate(CurrentMemoryContext,
-										"FDWConnectionContext",
-										ALLOCSET_SMALL_SIZES);
-
-	oldcxt = MemoryContextSwitchTo(tempContext);
-
-	PG_TRY();
-	{
-		ForeignServer *server;
-		ForeignDataWrapper *fdw;
-		Datum		connection_datum;
-
-		server = GetForeignServer(serverid);
-		fdw = GetForeignDataWrapper(server->fdwid);
-
-		if (!OidIsValid(fdw->fdwconnection))
-			ereport(ERROR,
-					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("foreign data wrapper \"%s\" does not support subscription connections",
-							fdw->fdwname),
-					 errdetail("Foreign data wrapper must be defined with CONNECTION specified.")));
-
-
-		connection_datum = OidFunctionCall3(fdw->fdwconnection,
-											ObjectIdGetDatum(userid),
-											ObjectIdGetDatum(serverid),
-											PointerGetDatum(NULL));
+	ForeignServer *server;
+	ForeignDataWrapper *fdw;
+	Datum		connection_datum;
 
-		connection_text = DatumGetTextPP(connection_datum);
-	}
-	PG_FINALLY();
-	{
-		MemoryContextSwitchTo(oldcxt);
+	server = GetForeignServer(serverid);
+	fdw = GetForeignDataWrapper(server->fdwid);
 
-		if (connection_text)
-			result = text_to_cstring((text *) connection_text);
+	if (!OidIsValid(fdw->fdwconnection))
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("foreign data wrapper \"%s\" does not support subscription connections",
+						fdw->fdwname),
+				 errdetail("Foreign data wrapper must be defined with CONNECTION specified.")));
 
-		MemoryContextDelete(tempContext);
-	}
-	PG_END_TRY();
+	connection_datum = OidFunctionCall3(fdw->fdwconnection,
+										ObjectIdGetDatum(userid),
+										ObjectIdGetDatum(serverid),
+										PointerGetDatum(NULL));
 
-	return result;
+	return text_to_cstring(DatumGetTextPP(connection_datum));
 }
 
 
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 2d7708805a6..a8256a54a97 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -481,6 +481,7 @@ static MemoryContext LogicalStreamingContext = NULL;
 WalReceiverConn *LogRepWorkerWalRcvConn = NULL;
 
 Subscription *MySubscription = NULL;
+static MemoryContext MySubscriptionCtx = NULL;
 static bool MySubscriptionValid = false;
 
 static List *on_commit_wakeup_workers_subids = NIL;
@@ -5044,6 +5045,7 @@ void
 maybe_reread_subscription(void)
 {
 	MemoryContext oldctx;
+	MemoryContext newctx;
 	Subscription *newsub;
 	bool		started_tx = false;
 
@@ -5058,8 +5060,15 @@ maybe_reread_subscription(void)
 		started_tx = true;
 	}
 
-	/* Ensure allocations in permanent context. */
-	oldctx = MemoryContextSwitchTo(ApplyContext);
+	newctx = AllocSetContextCreate(ApplyContext,
+								   "Subscription Context",
+								   ALLOCSET_SMALL_SIZES);
+
+	/*
+	 * GetSubscription() leaks a number of small allocations, so use a
+	 * subcontext for each call.
+	 */
+	oldctx = MemoryContextSwitchTo(newctx);
 
 	newsub = GetSubscription(MyLogicalRepWorker->subid, true, true);
 
@@ -5151,7 +5160,8 @@ maybe_reread_subscription(void)
 	}
 
 	/* Clean old subscription info and switch to new one. */
-	FreeSubscription(MySubscription);
+	MemoryContextDelete(MySubscriptionCtx);
+	MySubscriptionCtx = newctx;
 	MySubscription = newsub;
 
 	MemoryContextSwitchTo(oldctx);
@@ -5796,12 +5806,19 @@ InitializeLogRepWorker(void)
 	 */
 	SetConfigOption("search_path", "", PGC_SUSET, PGC_S_OVERRIDE);
 
-	/* Load the subscription into persistent memory context. */
 	ApplyContext = AllocSetContextCreate(TopMemoryContext,
 										 "ApplyContext",
 										 ALLOCSET_DEFAULT_SIZES);
+
+	/*
+	 * GetSubscription() leaks a number of small allocations, so use a
+	 * subcontext for each call.
+	 */
+	MySubscriptionCtx = AllocSetContextCreate(ApplyContext,
+											  "Subscription Context",
+											  ALLOCSET_SMALL_SIZES);
+
 	StartTransactionCommand();
-	oldctx = MemoryContextSwitchTo(ApplyContext);
 
 	/*
 	 * Lock the subscription to prevent it from being concurrently dropped,
@@ -5810,7 +5827,10 @@ InitializeLogRepWorker(void)
 	 */
 	LockSharedObject(SubscriptionRelationId, MyLogicalRepWorker->subid, 0,
 					 AccessShareLock);
+
+	oldctx = MemoryContextSwitchTo(MySubscriptionCtx);
 	MySubscription = GetSubscription(MyLogicalRepWorker->subid, true, true);
+
 	if (!MySubscription)
 	{
 		ereport(LOG,
diff --git a/src/include/catalog/pg_subscription.h b/src/include/catalog/pg_subscription.h
index 0058d9387d7..2f6f7b57698 100644
--- a/src/include/catalog/pg_subscription.h
+++ b/src/include/catalog/pg_subscription.h
@@ -212,7 +212,6 @@ typedef struct Subscription
 
 extern Subscription *GetSubscription(Oid subid, bool missing_ok,
 									 bool aclcheck);
-extern void FreeSubscription(Subscription *sub);
 extern void DisableSubscription(Oid subid);
 
 extern int	CountDBSubscriptions(Oid dbid);
-- 
2.43.0