Fix memory leak in pgoutput with relation attribute map

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 c9b3d4909bbf, 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 cfd6cbcf9be0.

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
index 9fd879ee0c85b4285d7e7d4ba57edd6371a976e6..a81215cff8654b5de44381ae605f5daf75fadfa2 100644 (file)
--- 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;
 }
 
 /*