pg_dump memory leak of 400 bytes

Started by Oleg Tselebrovskiyabout 1 year ago2 messages
#1Oleg Tselebrovskiy
o.tselebrovskiy@postgrespro.ru
2 attachment(s)

Greetings, everyone!

When I was looking through static analyzer output I've found a memory
leak in pg_dump

In function dumpFunc, when dumping a function with TRANSFORMs, we
allocate
memory for typeids array that contains OIDs of types that need to be
transformed.
The memory is allocated with palloc and is never freed.

Valgrind also confirms this:

1) Create TRANSFORM and FUNCTION using it:
CREATE TRANSFORM FOR int LANGUAGE SQL (FROM SQL WITH FUNCTION
prsd_lextype(internal),
TO SQL WITH FUNCTION int4recv(internal));
CREATE FUNCTION add(int, int) RETURNS int
AS 'select $1 + $2;'
LANGUAGE SQL TRANSFORM FOR TYPE int;

2) Use valgrind to observe the memory leak:
valgrind --leak-check=yes --time-stamp=yes
--error-markers=VALGRINDERROR-BEGIN,VALGRINDERROR-END ./pg_dump -f
somedump.sql
...
==00:00:00:00.764 50282== VALGRINDERROR-BEGIN
==00:00:00:00.764 50282== 400 bytes in 1 blocks are definitely lost in
loss record 92 of 134
==00:00:00:00.764 50282== at 0x4848899: malloc (in
/usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==00:00:00:00.764 50282== by 0x14B758: palloc (fe_memutils.c:107)
==00:00:00:00.764 50282== by 0x120D99: dumpFunc (pg_dump.c:12534)
==00:00:00:00.764 50282== by 0x10F764: main (pg_dump.c:1085)
==00:00:00:00.764 50282==
==00:00:00:00.764 50282== VALGRINDERROR-END
{
<insert_a_suppression_name_here>
Memcheck:Leak
match-leak-kinds: definite
fun:malloc
fun:palloc
fun:dumpFunc
fun:main
}
...

Also, we always allocate the same ammount of memory (FUNC_MAX_ARGS *
sizeof(Oid))

I propose two solutions for this problem:
1) Just free the memory in the end of this code block
2) Use static allocation since it is always the same amount

Patches for both variants are attached. I also propose changing palloc
to pg_malloc
since it is the only place in pg_dump where palloc is used instead of
pg_malloc

Oleg Tselebrovskiy, Postgres Pro

Attachments:

v1-0001-pg_dump_memleak.patchtext/x-diff; name=v1-0001-pg_dump_memleak.patchDownload
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index ec0cdf4ed74..9a24ebfca24 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -12531,7 +12531,7 @@ dumpFunc(Archive *fout, const FuncInfo *finfo)
 
 	if (*protrftypes)
 	{
-		Oid		   *typeids = palloc(FUNC_MAX_ARGS * sizeof(Oid));
+		Oid		   *typeids = pg_malloc(FUNC_MAX_ARGS * sizeof(Oid));
 		int			i;
 
 		appendPQExpBufferStr(q, " TRANSFORM ");
@@ -12543,6 +12543,7 @@ dumpFunc(Archive *fout, const FuncInfo *finfo)
 			appendPQExpBuffer(q, "FOR TYPE %s",
 							  getFormattedTypeName(fout, typeids[i], zeroAsNone));
 		}
+		free(typeids);
 	}
 
 	if (prokind[0] == PROKIND_WINDOW)
v1-0002-pg_dump_memleak_static.patchtext/x-diff; name=v1-0002-pg_dump_memleak_static.patchDownload
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index ec0cdf4ed74..c63b76aa382 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -12531,7 +12531,7 @@ dumpFunc(Archive *fout, const FuncInfo *finfo)
 
 	if (*protrftypes)
 	{
-		Oid		   *typeids = palloc(FUNC_MAX_ARGS * sizeof(Oid));
+		Oid		    typeids[FUNC_MAX_ARGS];
 		int			i;
 
 		appendPQExpBufferStr(q, " TRANSFORM ");
#2Daniel Gustafsson
daniel@yesql.se
In reply to: Oleg Tselebrovskiy (#1)
Re: pg_dump memory leak of 400 bytes

On 11 Dec 2024, at 10:59, Oleg Tselebrovskiy <o.tselebrovskiy@postgrespro.ru> wrote:

When I was looking through static analyzer output I've found a memory leak in pg_dump

Thanks for the report, although 400 bytes won't break the bank in the grand
scheme of things considering how much memory pg_dump use there is little point
in not freeing it.

Patches for both variants are attached. I also propose changing palloc to pg_malloc
since it is the only place in pg_dump where palloc is used instead of pg_malloc

Freeing it when not needed anymore seems appropriate, I'll go make that happen.

--
Daniel Gustafsson