Thread: Re: Proposal for fixing intra-query memory leaks

Re: Proposal for fixing intra-query memory leaks

From
Peter Eisentraut
Date:
Thanks for the timely introduction to memory contexts btw. :)

> 1. There will be two kinds of contexts, "permanent" and "temporary".

Rather than making this an explicit distinction you could simply (hah)
make the transaction initiating command create a memory context under Top
and store a pointer to it in a global variable. Then per tuple contexts,
etc. are created as a child thereof. The transaction ending commands would
then destroy that context again. I guess this is sort of what you were
planning anyway but I just wanted to throw in that specially treating this
"everything lives and dies with the transaction" attitude is not the
be-all-end-all, IMHO. ("Statement" would be better than "transaction" in
many cases anyway.)

> Functions that return pass-by-reference values will be required always
> to palloc the returned space in the caller's memory context (ie, the
> context that was CurrentMemoryContext at the time of call).  It is not
> OK to pass back an input pointer, even if we are returning an input value
> verbatim, because we do not know the lifespan of the context the input
> pointer points to.

ISTM that you can have the compiler help you here if you separate input
and output values in the function manager design. E.g., if you define the
function signature like
void my_func(const fmgr_in_t * in, fmgr_out_t * out);

then you establish the fact that copying is required.


-- 
Peter Eisentraut                  Sernanders väg 10:115
peter_e@gmx.net                   75262 Uppsala
http://yi.org/peter-e/            Sweden



Re: Proposal for fixing intra-query memory leaks

From
Tom Lane
Date:
Peter Eisentraut <peter_e@gmx.net> writes:
>> 1. There will be two kinds of contexts, "permanent" and "temporary".

> Rather than making this an explicit distinction you could simply (hah)
> make the transaction initiating command create a memory context under Top
> and store a pointer to it in a global variable. Then per tuple contexts,
> etc. are created as a child thereof. The transaction ending commands would
> then destroy that context again. I guess this is sort of what you were
> planning anyway

Yes, it was.  You're right that the lifespan of a context will be
determined by usage; there's not any real distinction between permanent
and temporary contexts as far as the mechanism goes.  I thought it would
be clearer to describe it that way, but maybe not.

Also, yes, the topmost contexts in the tree of contexts will be
referenced via pointers in global variables.  An alternative approach
would be to provide some kind of lookup-by-name facility (like portals
have) but I think that'd likely be overkill.  We've gotten along fine
with pointers for TopMemoryContext, CacheMemoryContext, etc, so it seems
that's good enough.

>> Functions that return pass-by-reference values will be required always
>> to palloc the returned space in the caller's memory context (ie, the
>> context that was CurrentMemoryContext at the time of call).  It is not
>> OK to pass back an input pointer, even if we are returning an input value
>> verbatim, because we do not know the lifespan of the context the input
>> pointer points to.

> ISTM that you can have the compiler help you here if you separate input
> and output values in the function manager design. E.g., if you define the
> function signature like
>     void my_func(const fmgr_in_t * in, fmgr_out_t * out);
> then you establish the fact that copying is required.

Given the amount of casting that we do between Datum and other types,
I don't think that const decorations would actually help much :-(.
On most compilers you can cast away const without noticing.

Jan is of the opinion that this you-must-palloc-your-result rule is
wrongheaded anyway, so the issue may go away if he persuades me...
        regards, tom lane