Fix memory leak in pgoutput with relation attribute map

michaelpq · michaelpq · commit c53d90bb47ae · 2025-01-08T08:47:19.000+09:00
pgoutput caches the attribute map of a relation, that is free()'d only when validating a RelationSyncEntry. However, this code path is not taken when calling any of the SQL functions able to do some logical decoding, like pg_logical_slot_{get,peek}_changes(), leaking some memory into CacheMemoryContext on repeated calls. This is a follow-up of c9b3d49, this time for v13 and v14. The relation attribute map is stored in a dedicated memory context, tracked with a static variable whose state is reset with a MemoryContext reset callback attached to PGOutputData->context. This implementation is similar to the approach taken by cfd6cbc. Reported-by: Masahiko Sawada Author: Vignesh C Reviewed-by: Hou Zhijie Discussion: https://postgr.es/m/CAD21AoDkAhQVSukOfH3_reuF-j4EU0-HxMqU3dU+bSTxsqT14Q@mail.gmail.com Discussion: https://postgr.es/m/CALDaNm1hewNAsZ_e6FF52a=9drmkRJxtEPrzCB6-9mkJyeBBqA@mail.gmail.com Backpatch-through: 13
diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c
@@ -66,11 +66,13 @@ static bool publications_valid;
 static bool in_streaming;
 
 /*
- * Private memory context for publication data, created in
- * PGOutputData->context when starting pgoutput, and set to NULL when its
- * parent context is reset via a dedicated MemoryContextCallback.
+ * Private memory contexts for publication data and relation attribute
+ * map, created in PGOutputData->context when starting pgoutput, and set
+ * to NULL when its parent context is reset via a dedicated
+ * MemoryContextCallback.
  */
 static MemoryContext pubctx = NULL;
+static MemoryContext cachectx = NULL;
 
 static List *LoadPublications(List *pubnames);
 static void publication_invalidation_cb(Datum arg, int cacheid,
@@ -260,12 +262,14 @@ parse_output_parameters(List *options, PGOutputData *data)
 }
 
 /*
- * Callback of PGOutputData->context in charge of cleaning pubctx.
+ * Callback of PGOutputData->context in charge of cleaning pubctx and
+ * cachectx.
  */
 static void
-pgoutput_pubctx_reset_callback(void *arg)
+pgoutput_ctx_reset_callback(void *arg)
 {
 	pubctx = NULL;
+	cachectx = NULL;
 }
 
 /*
@@ -289,8 +293,13 @@ pgoutput_startup(LogicalDecodingContext *ctx, OutputPluginOptions *opt,
 								   "logical replication publication list context",
 								   ALLOCSET_SMALL_SIZES);
 
+	Assert(cachectx == NULL);
+	cachectx = AllocSetContextCreate(ctx->context,
+									 "logical replication cache context",
+									 ALLOCSET_SMALL_SIZES);
+
 	mcallback = palloc0(sizeof(MemoryContextCallback));
-	mcallback->func = pgoutput_pubctx_reset_callback;
+	mcallback->func = pgoutput_ctx_reset_callback;
 	MemoryContextRegisterResetCallback(ctx->context, mcallback);
 
 	ctx->output_plugin_private = data;
@@ -489,8 +498,8 @@ maybe_send_schema(LogicalDecodingContext *ctx,
 		TupleDesc	outdesc = RelationGetDescr(ancestor);
 		MemoryContext oldctx;
 
-		/* Map must live as long as the session does. */
-		oldctx = MemoryContextSwitchTo(CacheMemoryContext);
+		/* Map must live as long as the logical decoding context. */
+		oldctx = MemoryContextSwitchTo(cachectx);
 
 		/*
 		 * Make copies of the TupleDescs that will live as long as the map
@@ -812,8 +821,8 @@ pgoutput_origin_filter(LogicalDecodingContext *ctx,
 /*
  * Shutdown the output plugin.
  *
- * Note, we don't need to clean the data->context and pubctx as they are
- * child contexts of the ctx->context so they will be cleaned up by logical
+ * Note, we don't need to clean the data->context, pubctx and cachectx as they
+ * are child contexts of the ctx->context so they will be cleaned up by logical
  * decoding machinery.
  */
 static void
@@ -827,6 +836,7 @@ pgoutput_shutdown(LogicalDecodingContext *ctx)
 
 	/* Better safe than sorry */
 	pubctx = NULL;
+	cachectx = NULL;
 }
 
 /*
