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