On Fri, Apr 9, 2021 at 10:31 AM Tom Lane <[email protected]> wrote:
> Michael Paquier <[email protected]> writes:
> > On Tue, Apr 06, 2021 at 11:09:13AM +0530, Rohit Bhogate wrote:
> >> I found the below reference leak on master.
>
> > Thanks for the report. This is indeed a new problem as of HEAD,
>
> Just for the record, it's not new. The issue is (I think) that
> the tupledesc refcount created by get_cached_rowtype is being
> logged in the wrong ResourceOwner. Other cases that use
> get_cached_rowtype, such as IS NOT NULL on a composite value,
> reproduce the same type of failure back to v11:
>
> create type float_rec_typ as (i float8);
>
> do $$
> declare
> f float_rec_typ := row(42);
> r bool;
> begin
> r := f is not null;
> commit;
> end
> $$;
>
> WARNING: TupleDesc reference leak: TupleDesc 0x7f5f549809d8 (53719,-1)
> still referenced
> ERROR: tupdesc reference 0x7f5f549809d8 is not owned by resource owner
> TopTransaction
>
> Still poking at a suitable fix.
>
> regards, tom lane
>
>
> Hi,
I think I have some idea about the cause for the 'resource owner' error.
When commit results in calling exec_stmt_commit(), the ResourceOwner
switches to a new one.
Later, when ResourceOwnerForgetTupleDesc() is called, we get the error
since owner->tupdescarr doesn't carry the tuple Desc to be forgotten.
One potential fix is to add the following to resowner.c
/*
* Transfer resources from resarr1 to resarr2
*/
static void
ResourceArrayTransfer(ResourceArray *resarr1, ResourceArray *resarr2)
{
}
In exec_stmt_commit(), we save reference to the old ResourceOwner before
calling SPI_commit() (line 4824).
Then after the return from SPI_start_transaction(), ResourceArrayTransfer()
is called to transfer remaining items in tupdescarr from old ResourceOwner
to the current ResourceOwner.
I want to get some opinion on the feasibility of this route.
It seems ResourceOwner is opaque inside exec_stmt_commit(). And
no ResourceArrayXX call exists in pl_exec.c
So I am still looking for the proper structure of the solution.
Cheers