Re: strong memory leak in plpgsql from handled rollback and lazy cast

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: strong memory leak in plpgsql from handled rollback and lazy cast
Date: 2019-09-23 00:16:15
Message-ID: 22663.1569197775@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Andres Freund <andres(at)anarazel(dot)de> writes:
> On 2019-09-22 18:43:23 -0400, Tom Lane wrote:
>> I'm not convinced that it'd be safe to re-use an ExprState after a
>> previous execution failed (though perhaps Andres has a different
>> opinion?)

> I don't immediately see why it'd be problematic to reuse at a later
> time, as long as it's guaranteed that a) there's only one execution
> happening at a time b) the failure wasn't in the middle of writing a
> value. a) would be problematic regardless of reuse-after-failure, and
> b) should be satisfied by only failing at ereport etc.

I think you're considering a much smaller slice of the system than
what seems to me to be at risk here. As an example, an ExprState
tree would also include any fn_extra-linked state that individual
functions might've set up. We've got very little control over what
the validity requirements are for those or how robust the code that
creates them is. I *think* that most of the core code that makes
such things is written in a way that it doesn't leave partially-valid
cache state if setup fails partway through ... but I wouldn't swear
that it all is, and I'd certainly bet money on there being third-party
code that isn't careful about that.

> Hm. I interestingly am working on a patch that merges all the memory
> allocations done for an ExprState into one or two allocations (by
> basically doing the traversal twice). Then it'd be feasible to just
> pfree() the memory, if that helps.

Again, that'd do nothing to clean up subsidiary fn_extra state.
If we want no leaks, we need a separate context for the tree
to live in.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2019-09-23 01:59:40 Re: strong memory leak in plpgsql from handled rollback and lazy cast
Previous Message Andres Freund 2019-09-22 23:57:28 Re: strong memory leak in plpgsql from handled rollback and lazy cast