Re: Reference Leak with type

From: Zhihong Yu <zyu(at)yugabyte(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, Rohit Bhogate <rohit(dot)bhogate(at)enterprisedb(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Reference Leak with type
Date: 2021-04-10 21:12:32
Message-ID: CALNJ-vTYtXCiyab3mMm3XYEo7peLAxrBJR+BKE0s0QYMEZ_sAg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Apr 9, 2021 at 10:31 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Michael Paquier <michael(at)paquier(dot)xyz> 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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2021-04-10 21:22:53 Re: pgsql: autovacuum: handle analyze for partitioned tables
Previous Message Noah Misch 2021-04-10 20:03:26 Re: SQL-standard function body