Re: Problem with tupdesc in jsonb_to_recordset

On Tue, Jul 10, 2018 at 10:39:28PM +0200, Dmitry Dolgov wrote:
> I've found out that currently in some situations jsonb_to_recordset can lead to
> a crash. Minimal example that I've managed to create looks like this:

It would be better not to send the same email across multiple mailing
lists.

> After brief investigation it looks like an issue with tupdesc from the function
> cache:
> 
>     if (!cache)
>     {
>         fcinfo->flinfo->fn_extra = cache =
>             MemoryContextAllocZero(fcinfo->flinfo->fn_mcxt, sizeof(*cache));
>     //...
> 
>     rsi->setDesc = cache->c.io.composite.tupdesc;
> 
> Then after the first call of populate_recordset_worker rsi->setDesc is being
> reset since we never increased tdrefcount and the next call will use wrong
> cache data. Apparently, it can be fixed by incrementing a tdrefcount (see the
> attached patch).

This is new as of REL_11_STABLE, so I am adding an open item.  Bisecting
the error, I have the following commit pointed out as the culprit:
commit: 37a795a60b4f4b1def11c615525ec5e0e9449e05
committer: Tom Lane 
date: Thu, 26 Oct 2017 13:47:45 -0400
Support domains over composite types.

> diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
> index e358b5a..9250646 100644
> --- a/src/backend/utils/adt/jsonfuncs.c
> +++ b/src/backend/utils/adt/jsonfuncs.c
> @@ -3728,6 +3728,7 @@ populate_recordset_worker(FunctionCallInfo fcinfo, const char *funcname,
>  
>  	rsi->setResult = state->tuple_store;
>  	rsi->setDesc = cache->c.io.composite.tupdesc;
> +	rsi->setDesc->tdrefcount = 1;
>  
>  	PG_RETURN_NULL();
>  }

I don't think that your solution is correct.  From my read of 37a795a6,
the tuple descriptor is moved from the query-lifespan memory context
(ecxt_per_query_memory) to a function-level context, which could cause
the descriptor to become busted as your test is pointing out.  Tom?
--
Michael