Horton

Fixing memory leaks in postgres_fdw

Tom Lane <tgl@sss.pgh.pa.us>

View thread View on postgr.es
From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@lists.postgresql.org
Date: 2025-05-24T01:10:23Z
Lists: pgsql-hackers

Attachments

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

Commits

  1. Silence leakage complaint about postgres_fdw's InitPgFdwOptions.

  2. Run pgindent on the changes of the previous patch.

  3. Reap the benefits of not having to avoid leaking PGresults.

  4. Create infrastructure to reliably prevent leakage of PGresults.

  5. Fix memory leakage in postgres_fdw's DirectModify code path.

  6. Avoid resource leaks when a dblink connection fails.