1
0
mirror of https://github.com/postgres/postgres.git synced 2025-04-24 10:47:04 +03:00

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
This commit is contained in:
Michael Paquier 2025-01-08 08:47:24 +09:00
parent a1d17a8947
commit a786cf04df

View File

@ -50,11 +50,13 @@ static bool pgoutput_origin_filter(LogicalDecodingContext *ctx,
static bool publications_valid; static bool publications_valid;
/* /*
* Private memory context for publication data, created in * Private memory contexts for publication data and relation attribute
* PGOutputData->context when starting pgoutput, and set to NULL when its * map, created in PGOutputData->context when starting pgoutput, and set
* parent context is reset via a dedicated MemoryContextCallback. * to NULL when its parent context is reset via a dedicated
* MemoryContextCallback.
*/ */
static MemoryContext pubctx = NULL; static MemoryContext pubctx = NULL;
static MemoryContext cachectx = NULL;
static List *LoadPublications(List *pubnames); static List *LoadPublications(List *pubnames);
static void publication_invalidation_cb(Datum arg, int cacheid, 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 static void
pgoutput_pubctx_reset_callback(void *arg) pgoutput_ctx_reset_callback(void *arg)
{ {
pubctx = NULL; pubctx = NULL;
cachectx = NULL;
} }
/* /*
@ -211,8 +215,13 @@ pgoutput_startup(LogicalDecodingContext *ctx, OutputPluginOptions *opt,
"logical replication publication list context", "logical replication publication list context",
ALLOCSET_SMALL_SIZES); ALLOCSET_SMALL_SIZES);
Assert(cachectx == NULL);
cachectx = AllocSetContextCreate(ctx->context,
"logical replication cache context",
ALLOCSET_SMALL_SIZES);
mcallback = palloc0(sizeof(MemoryContextCallback)); mcallback = palloc0(sizeof(MemoryContextCallback));
mcallback->func = pgoutput_pubctx_reset_callback; mcallback->func = pgoutput_ctx_reset_callback;
MemoryContextRegisterResetCallback(ctx->context, mcallback); MemoryContextRegisterResetCallback(ctx->context, mcallback);
ctx->output_plugin_private = data; ctx->output_plugin_private = data;
@ -347,8 +356,8 @@ maybe_send_schema(LogicalDecodingContext *ctx,
TupleDesc outdesc = RelationGetDescr(ancestor); TupleDesc outdesc = RelationGetDescr(ancestor);
MemoryContext oldctx; MemoryContext oldctx;
/* Map must live as long as the session does. */ /* Map must live as long as the logical decoding context. */
oldctx = MemoryContextSwitchTo(CacheMemoryContext); oldctx = MemoryContextSwitchTo(cachectx);
/* /*
* Make copies of the TupleDescs that will live as long as the map * 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. * Shutdown the output plugin.
* *
* Note, we don't need to clean the data->context and pubctx as they are * Note, we don't need to clean the data->context, pubctx and cachectx as they
* child contexts of the ctx->context so they will be cleaned up by logical * are child contexts of the ctx->context so they will be cleaned up by logical
* decoding machinery. * decoding machinery.
*/ */
static void static void
@ -628,6 +637,7 @@ pgoutput_shutdown(LogicalDecodingContext *ctx)
/* Better safe than sorry */ /* Better safe than sorry */
pubctx = NULL; pubctx = NULL;
cachectx = NULL;
} }
/* /*