Fixing memory leaks in postgres_fdw

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Fixing memory leaks in postgres_fdw
Date: 2025-05-24 01:10:23
Message-ID: 2976982.1748049023@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Running contrib/postgres_fdw's regression tests under Valgrind shows
two different sources of memory leaks. One is a basically-cosmetic
issue in InitPgFdwOptions, but the other is real and troublesome.
The DirectModify code path relies on PG_TRY blocks to ensure that it
releases the PGresult for the foreign modify command, but that can't
work because (at least in cases with RETURNING data) the PGresult has
to survive across successive calls to postgresIterateDirectModify.
If an error occurs in the query in between those steps, we have no
opportunity to clean up.

I thought of fixing this by using a memory context reset callback
to ensure that the PGresult is cleaned up when the executor's context
goes away, and that seems to work nicely (see 0001 attached).
However, I feel like this is just a POC, because now that we have that
concept we might be able to use it elsewhere in postgres_fdw to
eliminate most or even all of its reliance on PG_TRY. That should be
faster as well as much less bug-prone. But I'm putting it up at this
stage for comments, in case anyone thinks this is not the direction to
head in.

0002 attached deals with the other thing. If you apply these
on top of my valgrind-cleanup patches at [1], you'll see that
contrib/postgres_fdw's tests go through leak-free.

regards, tom lane

[1] https://www.postgresql.org/message-id/2884224.1748035274%40sss.pgh.pa.us

Attachment Content-Type Size
v1-0001-Fix-memory-leakage-in-postgres_fdw-s-DirectModify.patch text/x-diff 7.1 KB
v1-0002-Silence-leakage-complaint-about-postgres_fdw-s-In.patch text/x-diff 3.7 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2025-05-24 01:42:34 Re: queryId constant squashing does not support prepared statements
Previous Message Sami Imseih 2025-05-24 01:05:47 Re: queryId constant squashing does not support prepared statements