From a786cf04df767e007069e6490734b21ff6a0418e Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Wed, 8 Jan 2025 08:47:24 +0900 Subject: [PATCH] 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 --- src/backend/replication/pgoutput/pgoutput.c | 30 ++++++++++++++------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c index 054ee8abbd2..3d98d60d6c4 100644 --- a/src/backend/replication/pgoutput/pgoutput.c +++ b/src/backend/replication/pgoutput/pgoutput.c @@ -50,11 +50,13 @@ static bool pgoutput_origin_filter(LogicalDecodingContext *ctx, static bool publications_valid; /* - * 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, @@ -182,12 +184,14 @@ parse_output_parameters(List *options, uint32 *protocol_version, } /* - * 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; } /* @@ -211,8 +215,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; @@ -347,8 +356,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 @@ -613,8 +622,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 @@ -628,6 +637,7 @@ pgoutput_shutdown(LogicalDecodingContext *ctx) /* Better safe than sorry */ pubctx = NULL; + cachectx = NULL; } /*