Horton

Re: Fixing memory leaks in postgres_fdw

Matheus Alcantara <matheusssilv97@gmail.com>

View thread View on postgr.es
From: Matheus Alcantara <matheusssilv97@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Etsuro Fujita <etsuro.fujita@gmail.com>, pgsql-hackers@lists.postgresql.org
Date: 2025-05-27T20:45:17Z
Lists: pgsql-hackers

Attachments

Hi,

On 26/05/25 16:36, Tom Lane wrote:
> Here's a v4 that is actually more or less feature-complete:
> it removes no-longer-needed complexity such as PG_TRY blocks.
> I've checked that Valgrind shows no leaks in the postgres_fdw
> and dblink tests after applying this on top of my other
> patch series.
>
> 0001 is like the previous version except that I took out some
> inessential simplifications to get to the minimum possible
> patch.  Then 0002 does all the simplifications.  Removal of
> PG_TRY blocks implies reindenting a lot of code, but I made
> that a separate patch 0003 for ease of review.  (0003 would
> be a candidate for adding to .git-blame-ignore-revs, perhaps.)
> 0004 is the old 0002 (still unmodified) and then 0005 cleans
> up one remaining leakage observed by Valgrind.
>
>             regards, tom lane
>

The v4-0001-Fix-memory-leakage-in-postgres_fdw-s-DirectModify.patch
looks good to me.

Just some thoughts on v4-0005-Avoid-leak-when-dblink_connstr_check-fails.patch:

I think that we can delay the allocation a bit more. The
dblink_security_check just use the rconn to pfree in case of a failure,
so I think that we can remove this parameter and move the rconn
allocation to the next if (connname) block. See attached as an example.

-- 
Matheus Alcantara

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.