On 2024-Nov-30, Michael Paquier wrote:
> After sleeping on that, and because the leak is minimal, I'm thinking
> about just applying the fix only on HEAD and call it a day. This
> changes the structure of Publication so as we use a char[NAMEDATALEN]
> rather than a char*, avoiding the pstrdup(), for the publication name
> and free it in the list_free_deep() when reloading the list of
> publications.
I'm not sure about your proposed fix. Isn't it simpler to have another
memory context which we can reset instead of doing list_free_deep()? It
doesn't have to be a global memory context -- since this is not
reentrant and not referenced anywhere else, it can be a simple static
variable in that block, as in the attached. I ran the stock tests (no
sysbench) and at least it doesn't crash.
This should be easily backpatchable also, since there's no ABI change.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
>From 3ed287180a007447bbee152177139cf1d4347625 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvhe...@alvh.no-ip.org>
Date: Sat, 30 Nov 2024 09:02:10 +0100
Subject: [PATCH] untested try at fixing memleak
---
src/backend/replication/pgoutput/pgoutput.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c
index 5e23453f071..010b341e0f5 100644
--- a/src/backend/replication/pgoutput/pgoutput.c
+++ b/src/backend/replication/pgoutput/pgoutput.c
@@ -2063,12 +2063,16 @@ get_rel_sync_entry(PGOutputData *data, Relation relation)
/* Reload publications if needed before use. */
if (!publications_valid)
{
- oldctx = MemoryContextSwitchTo(CacheMemoryContext);
- if (data->publications)
- {
- list_free_deep(data->publications);
- data->publications = NIL;
- }
+ static MemoryContext pubmemcxt = NULL;
+
+ if (pubmemcxt == NULL)
+ pubmemcxt = AllocSetContextCreate(CacheMemoryContext,
+ "publication list context",
+ ALLOCSET_DEFAULT_SIZES);
+ else
+ MemoryContextReset(pubmemcxt);
+ oldctx = MemoryContextSwitchTo(pubmemcxt);
+
data->publications = LoadPublications(data->publication_names);
MemoryContextSwitchTo(oldctx);
publications_valid = true;
--
2.39.5
