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 <tgl(at)sss(dot)pgh(dot)pa(dot)us>
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