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: Etsuro Fujita <etsuro.fujita@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org
Date: 2025-05-25T23:59:00Z
Lists: pgsql-hackers

Attachments

I wrote:
> Here is an attempt at making a bulletproof fix by having all backend
> users of libpq go through a wrapper layer that provides the memory
> context callback.  Perhaps this is more code churn than we want to
> accept; I'm not sure.  I thought about avoiding most of the niggling
> code changes by adding
> #define PGresult BEPGresult
> #define PQclear BEPQclear
> #define PQresultStatus BEPQresultStatus
> and so forth at the bottom of the new header file, but I'm afraid
> that would create a lot of confusion.

I tried that, and it leads to such a less-messy patch that I think
we should probably do it this way, confusion or no.  One argument
that can be made in favor is that we don't really want random
notational differences between frontend and backend code that's
doing the same thing.

Also, I'd been struggling with the assumption that we should
palloc the wrapper object before calling PQgetResult; there
doesn't seem to be any nice way to make that transparent to
callers.  I realized that we can make it simple as long as
we're willing to assume that allocating with MCXT_ALLOC_NO_OOM
can't throw an error.  But we assume that in other usages too.

Hence, v3 attached.  The 0002 patch is still the same as before.

			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.