diff options
author | Michael Paquier <michael@paquier.xyz> | 2025-01-08 08:47:19 +0900 |
---|---|---|
committer | Michael Paquier <michael@paquier.xyz> | 2025-01-08 08:47:19 +0900 |
commit | c53d90bb47ae488beebe64d651ef926c86a51bde (patch) | |
tree | 336bbc579057844f1573fb2dee1646c6b528fb94 /src | |
parent | f154f028d856bf5ad6ec419d7947865c172d14ae (diff) | |
download | postgresql-c53d90bb47ae488beebe64d651ef926c86a51bde.tar.gz postgresql-c53d90bb47ae488beebe64d651ef926c86a51bde.zip |
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
Diffstat (limited to 'src')
-rw-r--r-- | src/backend/replication/pgoutput/pgoutput.c | 30 |
1 files changed, 20 insertions, 10 deletions
diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c index 9fd879ee0c8..a81215cff86 100644 --- 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; } /* |