From ee1dfccc0306812c356c84bbd7e2558f27d7d348 Mon Sep 17 00:00:00 2001
From: Hou Zhijie <houzj.fnst@cn.fujitsu.com>
Date: Thu, 17 Aug 2023 19:29:34 +0800
Subject: [PATCH v4] cleanup decoding context in error cases

Some of the management functions for logical decoding didn't clean up the
decoding context when an error occurs during decoding. This can
result in accessing unfreed resources and assert failure the next time we call
these functions. Fix it by cleaning up the decoding context and slots in error
cases.
---
 .../replication/logical/logicalfuncs.c        | 18 ++++-----
 src/backend/replication/slotfuncs.c           | 37 +++++++++++--------
 2 files changed, 29 insertions(+), 26 deletions(-)

diff --git a/src/backend/replication/logical/logicalfuncs.c b/src/backend/replication/logical/logicalfuncs.c
index dc29b6c674..73c88a74d2 100644
--- a/src/backend/replication/logical/logicalfuncs.c
+++ b/src/backend/replication/logical/logicalfuncs.c
@@ -109,7 +109,7 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin
 	MemoryContext per_query_ctx;
 	MemoryContext oldcontext;
 	XLogRecPtr	end_of_wal;
-	LogicalDecodingContext *ctx;
+	LogicalDecodingContext *volatile ctx = NULL;
 	ResourceOwner old_resowner = CurrentResourceOwner;
 	ArrayType  *arr;
 	Size		ndim;
@@ -311,19 +311,17 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin
 			 */
 			ReplicationSlotMarkDirty();
 		}
-
-		/* free context, call shutdown callback */
-		FreeDecodingContext(ctx);
-
-		ReplicationSlotRelease();
-		InvalidateSystemCaches();
 	}
-	PG_CATCH();
+	PG_FINALLY();
 	{
+		/* Free the context and call shutdown callback if necessary */
+		if (ctx != NULL)
+			FreeDecodingContext(ctx);
+
+		ReplicationSlotRelease();
+
 		/* clear all timetravel entries */
 		InvalidateSystemCaches();
-
-		PG_RE_THROW();
 	}
 	PG_END_TRY();
 
diff --git a/src/backend/replication/slotfuncs.c b/src/backend/replication/slotfuncs.c
index 16a3527903..626169ef25 100644
--- a/src/backend/replication/slotfuncs.c
+++ b/src/backend/replication/slotfuncs.c
@@ -150,15 +150,21 @@ create_logical_replication_slot(char *name, char *plugin,
 											   .segment_close = wal_segment_close),
 									NULL, NULL, NULL);
 
-	/*
-	 * If caller needs us to determine the decoding start point, do so now.
-	 * This might take a while.
-	 */
-	if (find_startpoint)
-		DecodingContextFindStartpoint(ctx);
-
-	/* don't need the decoding context anymore */
-	FreeDecodingContext(ctx);
+	PG_TRY();
+	{
+		/*
+		 * If caller needs us to determine the decoding start point, do so now.
+		 * This might take a while.
+		 */
+		if (find_startpoint)
+			DecodingContextFindStartpoint(ctx);
+	}
+	PG_FINALLY();
+	{
+		/* don't need the decoding context anymore */
+		FreeDecodingContext(ctx);
+	}
+	PG_END_TRY();
 }
 
 /*
@@ -461,7 +467,7 @@ pg_physical_replication_slot_advance(XLogRecPtr moveto)
 static XLogRecPtr
 pg_logical_replication_slot_advance(XLogRecPtr moveto)
 {
-	LogicalDecodingContext *ctx;
+	LogicalDecodingContext *volatile ctx = NULL;
 	ResourceOwner old_resowner = CurrentResourceOwner;
 	XLogRecPtr	retlsn;
 
@@ -550,17 +556,16 @@ pg_logical_replication_slot_advance(XLogRecPtr moveto)
 
 		retlsn = MyReplicationSlot->data.confirmed_flush;
 
-		/* free context, call shutdown callback */
-		FreeDecodingContext(ctx);
-
 		InvalidateSystemCaches();
 	}
-	PG_CATCH();
+	PG_FINALLY();
 	{
+		/* Free the context and call shutdown callback if necessary */
+		if (ctx != NULL)
+			FreeDecodingContext(ctx);
+
 		/* clear all timetravel entries */
 		InvalidateSystemCaches();
-
-		PG_RE_THROW();
 	}
 	PG_END_TRY();
 
-- 
2.30.0.windows.2