From: Tom Lane <tgl@sss.pgh.pa.us>
To: Matheus Alcantara <matheusssilv97@gmail.com>
Cc: Etsuro Fujita <etsuro.fujita@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Fixing memory leaks in postgres_fdw
Date: Tue, 27 May 2025 17:38:31 -0400
Message-ID: <1346940.1748381911@sss.pgh.pa.us> (raw)
In-Reply-To: <CAFY6G8fEa9sZ1V9cJpMOZAp6Wfa35U_rE0Y=eoQLS2G3OAqiAw@mail.gmail.com>
References: <2976982.1748049023@sss.pgh.pa.us>
<CAPmGK17iOh6W25UswGcn5k75jEukSgLMpiaJEATcAZQTec7xxg@mail.gmail.com>
<216286.1748202797@sss.pgh.pa.us>
<519144.1748217540@sss.pgh.pa.us>
<918254.1748288174@sss.pgh.pa.us>
<CAFY6G8fEa9sZ1V9cJpMOZAp6Wfa35U_rE0Y=eoQLS2G3OAqiAw@mail.gmail.com>
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, matheusssilv97@gmail.com, etsuro.fujita@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Fixing memory leaks in postgres_fdw
In-Reply-To: <1346940.1748381911@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox