Re: Cleanup palloc'd structs on soft error path in `record_in`

From: Kirill Reshke <reshkekirill(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Cleanup palloc'd structs on soft error path in `record_in`
Date: 2026-01-17 18:21:00
Message-ID: CALdSSPgb39Wwh8C39hQsXy8Mt7n9Wvzqo9aPukGrZhtvDOeSeQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat, 17 Jan 2026 at 23:15, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Kirill Reshke <reshkekirill(at)gmail(dot)com> writes:
> > I have been looking in coverity that runs against our PostgreSQL
> > fork(vanilla + our poor-man patches to be more cloud-native), and
> > spotted $subj.
>
> > Unlike many other reports, this looks like a real one to me?
>
> Sadly, Coverity's opinions about backend memory leaks are pretty
> nearly complete garbage, because it does not understand about our
> memory context architecture. Our expectation is that things like
> input functions will be run in a short-lived context, and anything
> they leak is to be cleaned up by resetting that context every
> so often. If we tried to make the individual input functions
> be responsible for that, we'd have leaks in hundreds of places,
> many of them a lot messier to fix than record_in(). Also, the
> result would likely be slower: we expect that MemoryContextReset
> is cheaper than a bunch of retail pfree's.
>
> > Looks like after ccff2d20ed96, when we changed elog to ereport in a
> > number of places in this function, we now fail to release memory
> > allocated for one row. Now, PostgreSQL has REJECT_LIMIT feature, so
> > looks like this leak can aggregate in COPY scenarios
>
> If you trace through that, you will find that record_in is invoked
> in the ExprContext that's managed by CopyFrom(), which is reset
> for each tuple (see copyfrom.c, about line 1123 as of HEAD).
> If there were a leak here, I would blame the COPY logic not
> record_in().
>
> > WDYT?
>
> If I were to do anything at all to that code, it'd be to remove
> the retail pfree's in the non-error path. They're misleading, and
> per the above argument they're likely a net drag on performance
> (but probably not enough to measure easily).
>
> regards, tom lane

Thank you for your clarification.

Indeed, if `record_in` leaks, we need to blame the caller function for
not doing MemoryContextReset.

Yes, almost all coverity reports about memory leaks are garbage.

--
Best regards,
Kirill Reshke

In response to

Browse pgsql-hackers by date

  From Date Subject
Previous Message Tom Lane 2026-01-17 18:15:07 Re: Cleanup palloc'd structs on soft error path in `record_in`