Re: memory leak in pgoutput

On Mon, Nov 18, 2024 at 02:53:52AM +0000, Zhijie Hou (Fujitsu) wrote:
> But I think there is an issue in the attached patch:
>
> + FreeTupleDesc(entry->old_slot->tts_tupleDescriptor);
> ExecDropSingleTupleTableSlot(entry->old_slot);
>
> Here, after freeing the tupledesc, the ExecDropSingleTupleTableSlot will still
> access the freed tupledesc->tdrefcount which is an illegal memory access.
>
> I think we can do something like below instead:
>
> + TupleDesc desc = entry->old_slot->tts_tupleDescriptor;
> +
> + Assert(desc->tdrefcount == -1);
> +
> ExecDropSingleTupleTableSlot(entry->old_slot);
> + FreeTupleDesc(desc);

Yep, obviously.

I was first surprised that the DecrTupleDescRefCount() done in
ExecDropSingleTupleTableSlot() would not be enough to free the
TupleDesc.

We do some allocations for dynamically-allocated resources like what
pgoutput does in the SRF code, if I recall correctly, as these need
are a problem across multiple calls and the query-level memory context
would not do this cleanup..
--
Michael