Horton

Re: 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: Matheus Alcantara <matheusssilv97@gmail.com>
Cc: Etsuro Fujita <etsuro.fujita@gmail.com>, pgsql-hackers@lists.postgresql.org
Date: 2025-05-27T21:38:31Z
Lists: pgsql-hackers 
Matheus Alcantara <matheusssilv97@gmail.com> writes:
> 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.

Yeah, I thought of that too.  But I think the point of the current
setup is to ensure we have the rconn block before we create the PGconn
object, because if we were to hit OOM after creating the connection,
we'd leak the PGconn, which would be quite bad.

Having said that, the idea that this sequence is OOM-safe is pretty
silly anyway, considering that createNewConnection does a pstrdup,
and creates a new hashtable entry which might require enlarging the
hashtable, and for that matter might even create the hashtable.
So maybe rather than continuing to adhere to a half-baked coding
rule, we need to think of some other way to do that.  Maybe it'd be
reasonable to create a hashtable entry with NULL rconn, and then
open the connection?  This'd require rejiggering the lookup
code to treat a hashtable entry with NULL rconn as not really
being there.  But there's not too many routines touching that
hashtable, so maybe it's not hard.

			regards, tom lane

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.