pg.ddx.io  pgsql-committers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-committers@lists.postgresql.org
Subject: pgsql: Fix memory leakage in postgres_fdw's DirectModify code path.
Date: Fri, 30 May 2025 17:46:02 +0000
Message-ID: <E1uL3nt-000l4o-2F@gemulon.postgresql.org> (raw)

Fix memory leakage in postgres_fdw's DirectModify code path.

postgres_fdw tries to use PG_TRY blocks to ensure that it will
eventually free the PGresult created by the remote modify command.
However, it's fundamentally impossible for this scheme to work
reliably when there's RETURNING data, because the query could fail
in between invocations of postgres_fdw's DirectModify methods.
There is at least one instance of exactly this situation in the
regression tests, and the ensuing session-lifespan leak is visible
under Valgrind.

We can improve matters by using a memory context reset callback
attached to the ExecutorState context.  That ensures that the
PGresult will be freed when the ExecutorState context is torn
down, even if control never reaches postgresEndDirectModify.

I have little faith that there aren't other potential PGresult
leakages in the backend modules that use libpq.  So I think it'd
be a good idea to apply this concept universally by creating
infrastructure that attaches a reset callback to every PGresult
generated in the backend.  However, that seems too invasive for
v18 at this point, let alone the back branches.  So for the
moment, apply this narrow fix that just makes DirectModify safe.
I have a patch in the queue for the more general idea, but it
will have to wait for v19.

Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Matheus Alcantara <matheusssilv97@gmail.com>
Discussion: https://postgr.es/m/2976982.1748049023@sss.pgh.pa.us
Backpatch-through: 13

Branch
------
REL_14_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/4a07c096132c389a6d3e42e4af9078999a6a48e8

Modified Files
--------------
contrib/postgres_fdw/postgres_fdw.c | 60 ++++++++++++++++++++-----------------
1 file changed, 33 insertions(+), 27 deletions(-)



view thread (6+ messages)  latest in thread

Message-ID: <E1uL3nt-000l4o-2F@gemulon.postgresql.org>
Permalink:  ../E1uL3nt-000l4o-2F@gemulon.postgresql.org/
Also on:    postgresql.org/message-id/E1uL3nt-000l4o-2F@gemulon.postgresql.org

 · 

reply

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-committers@postgresql.org
  Cc: tgl@sss.pgh.pa.us, pgsql-committers@lists.postgresql.org
  Subject: Re: pgsql: Fix memory leakage in postgres_fdw's DirectModify code path.
  In-Reply-To: <E1uL3nt-000l4o-2F@gemulon.postgresql.org>

* 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