Re: Fixing memory leaks in postgres_fdw

On 28/05/25 17:56, Tom Lane wrote:
> I wrote:
>> 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.
>
> Here's an attempt at fixing this properly.  I'm posting it as a
> standalone patch because I now think this part might be worth
> back-patching.  The odds of an OOM at just the wrong time aren't
> high, but losing track of an open connection seems pretty bad.
>

The v5-0001 makes sense to me. I think that now it follows a similar
approach with postgres_fdw that first create the hash entry and them set
the connection on the hash entry.

I've also reviewed the remaining patches, v4-0002, v4-0003 and v4-0004
and it all looks reasonable to me. +1 for going forward with these
patches.

The only point is that I've just tried to apply the v5-0001 on top of
the previous v4-000X patches and is raising an error:

❯❯❯ git am v5-0001-Avoid-resource-leaks-when-a-dblink-connection-fai.patch
Applying: Avoid resource leaks when a dblink connection fails.
error: patch failed: contrib/dblink/dblink.c:105
error: contrib/dblink/dblink.c: patch does not apply
Patch failed at 0001 Avoid resource leaks when a dblink connection fails.

--
Matheus Alcantara