Horton

Re: Fixing memory leaks in postgres_fdw

Etsuro Fujita <etsuro.fujita@gmail.com>

View thread View on postgr.es
From: Etsuro Fujita <etsuro.fujita@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@lists.postgresql.org
Date: 2025-05-24T12:22:59Z
Lists: pgsql-hackers 
Hi,

On Sat, May 24, 2025 at 10:10 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 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.

Ugh.

> 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.

I think that that is a good idea; +1 for removing the reliance not
only in DirectModify but in other places.  I think that that would be
also useful if extending batch INSERT to cases with RETURNING data in
postgres_fdw.

Thanks for working on this!

Best regards,
Etsuro Fujita

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.