Re: SRF memory mgmt patch (was [HACKERS] Concern about memory management - Mailing list pgsql-patches

From Joe Conway
Subject Re: SRF memory mgmt patch (was [HACKERS] Concern about memory management
Date
Msg-id 3D6EBA98.9050601@joeconway.com
Whole thread Raw
In response to SRF memory mgmt patch (was [HACKERS] Concern about memory management with SRFs)  (Joe Conway <mail@joeconway.com>)
Responses Re: SRF memory mgmt patch (was [HACKERS] Concern about memory management with SRFs)
List pgsql-patches
Tom Lane wrote:
> Joe Conway <mail@joeconway.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



pgsql-patches by date:

Previous
From: Bruce Momjian
Date:
Subject: Re: [HACKERS] Proposed GUC Variable
Next
From: Tom Lane
Date:
Subject: Re: revised patch for PL/PgSQL table functions