On Mon, Dec 02, 2024 at 03:29:56PM +0530, Amit Kapila wrote: > We can look at it from a different angle which is that the > FreePublication(s) relies on how the knowledge of Publication > structure is built. So, it doesn't look weird if we think from that > angle.
OK, I can live with that on all the stable branches with an extra list free rather than a deep list free. I agree that the memory handling of this whole area needs some rework to make such leaks harder to introduce in the WAL sender. Still, let's first solve the problem at hand :) So how about the attached that introduces a FreePublication() matching with GetPublication(), used to do the cleanup? Feel free to comment. -- Michael
From f65c7bf220ba152a868ef52bb7df245809ce6d73 Mon Sep 17 00:00:00 2001 From: Michael Paquier <mich...@paquier.xyz> Date: Tue, 3 Dec 2024 15:45:04 +0900 Subject: [PATCH v3] Fix memory leak in pgoutput with publication list cache Blah. Analyzed-by: Jeff Davis, Michael Paquier Discussion: https://postgr.es/m/z0khf9evmvloc...@paquier.xyz Backpatch-through: 13 --- src/include/catalog/pg_publication.h | 1 + src/backend/catalog/pg_publication.c | 10 ++++++++++ src/backend/replication/pgoutput/pgoutput.c | 8 +++++--- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/include/catalog/pg_publication.h b/src/include/catalog/pg_publication.h index 9a83a72d6b..4e2de947e9 100644 --- a/src/include/catalog/pg_publication.h +++ b/src/include/catalog/pg_publication.h @@ -118,6 +118,7 @@ typedef struct PublicationRelInfo } PublicationRelInfo; extern Publication *GetPublication(Oid pubid); +extern void FreePublication(Publication *pub); extern Publication *GetPublicationByName(const char *pubname, bool missing_ok); extern List *GetRelationPublications(Oid relid); diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c index 9bbb60463f..2efe86d953 100644 --- a/src/backend/catalog/pg_publication.c +++ b/src/backend/catalog/pg_publication.c @@ -1075,6 +1075,16 @@ GetPublication(Oid pubid) return pub; } +/* + * Free memory allocated by Publication structure. + */ +void +FreePublication(Publication *pub) +{ + pfree(pub->name); + pfree(pub); +} + /* * Get Publication using name. */ diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c index 5e23453f07..a51b1c15fb 100644 --- a/src/backend/replication/pgoutput/pgoutput.c +++ b/src/backend/replication/pgoutput/pgoutput.c @@ -2064,11 +2064,13 @@ get_rel_sync_entry(PGOutputData *data, Relation relation) if (!publications_valid) { oldctx = MemoryContextSwitchTo(CacheMemoryContext); - if (data->publications) + foreach(lc, data->publications) { - list_free_deep(data->publications); - data->publications = NIL; + Publication *pub = lfirst(lc); + + FreePublication(pub); } + list_free(data->publications); data->publications = LoadPublications(data->publication_names); MemoryContextSwitchTo(oldctx); publications_valid = true; -- 2.45.2
signature.asc
Description: PGP signature
