Re: SRF memory mgmt patch (was [HACKERS] Concern about memory management

Tom Lane wrote:
> Joe Conway <mail(at)joeconway(dot)com> writes:
>
>>As you said, if the next ExecStoreTuple will try to do an
>>ExecClearTuple(), ISTM that it should be removed from
>>per_MultiFuncCall()/SRF_PERCALL_SETUP(). Or am I crazy?
>
>
> Actually ... on second thought ...
>
> I bet the real issue here is that we have a long-lived TupleTableSlot
> pointing at a short-lived tuple. (I assume you're just forming the
> tuple in the function's working context, no?)

yep

> When ExecClearTuple is called on the next time through, it tries to
> pfree a tuple that has already been recycled along with the rest of
> the short-term context. Result: coredump.
>
> However, if that were the story then *none* of the SRFs returning
> tuple should work, and they do. So I'm still confused.

That's what had me confused.

I have found the smoking gun, however. I had changed this function from
returning setof text, to returning setof
two_column_named_composite_type. *However*. I had not dropped and
recreated the function with the proper declaration. Once I redeclared
the function properly, the coredump went away, even with current
per_MultiFuncCall() code.

The way I found this was by removing ExecClearTuple() from
per_MultiFuncCall(). That allowed the function to return without core
dumping, but it gave me one column of garbage -- that finally clued me in.

> But I suspect that what we want to do is take management of the tuples
> away from the Slot: pass should_free = FALSE to ExecStoreTuple in the
> TupleGetDatum macro. The ClearTuple call *is* appropriate when you do
> that, because it will reset the Slot to empty rather than leaving it
> containing a dangling pointer to a long-since-freed tuple.

OK. I'll make that change and leave ExecClearTuple() in
per_MultiFuncCall(). Sound like a plan?

Joe