Assorted leaks of PGresults

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)postgreSQL(dot)org
Subject: Assorted leaks of PGresults
Date: 2017-06-15 00:36:21
Message-ID: 22620.1497486981@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Coverity complained that libpqwalreceiver.c's libpqrcv_PQexec()
potentially leaks a PGresult. It's right: if PQconsumeInput() fails after
we've already collected at least one PGresult, the function just returns
NULL without remembering to free last_result. That's easy enough to fix,
just insert "PQclear(lastResult)" before the return. I do not think this
is a significant leak, because the walreceiver process would just exit
anyway after the failure. But we ought to fix it in case somebody copies
the logic into someplace less forgiving.

In fact ... I looked through other callers of PQconsumeInput() to see if
we had the same wrong coding pattern elsewhere, and we do, in two places
in postgres_fdw/connection.c. One of those is new as of last week, in
commit ae9bfc5d6. What's worse, in those cases we have code inside the
loop that might throw elog(ERROR), resulting in a permanently leaked
PGresult --- and since the backend process would keep going, this could
possibly be repeated and build up to a leak that amounted to something
significant. We need to have PG_TRY blocks in both these places that
ensure that any held PGresult gets freed before we exit the functions.

Further review showed an additional leak introduced by ae9bfc5d6, to wit
that pgfdw_exec_cleanup_query() just forgets to clear the PGresult
returned by pgfdw_get_cleanup_result() in its normal non-error path.

In short, I think we need the attached patch in HEAD. It needs to be
back-patched too, though the code seems to be a bit different in the
back branches.

regards, tom lane

Attachment Content-Type Size
fix-some-PGresult-leaks.patch text/x-diff 7.2 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Masahiko Sawada 2017-06-15 01:22:55 Re: Get stuck when dropping a subscription during synchronizing table
Previous Message Mark Kirkwood 2017-06-14 23:48:39 Re: logical replication: \dRp+ and "for all tables"