Re: Crash in record_type_typmod_compare

Hi,

On 2021-03-31 13:26:34 +0000, Sait Talha Nisanci wrote:
> diff --git a/src/backend/utils/cache/typcache.c b/src/backend/utils/cache/typcache.c
> index 4915ef5934..4757e8fa80 100644
> --- a/src/backend/utils/cache/typcache.c
> +++ b/src/backend/utils/cache/typcache.c
> @@ -1970,18 +1970,16 @@ assign_record_type_typmod(TupleDesc tupDesc)
>  			CreateCacheMemoryContext();
>  	}
>  
> -	/* Find or create a hashtable entry for this tuple descriptor */
> +	/* Find a hashtable entry for this tuple descriptor */
>  	recentry = (RecordCacheEntry *) hash_search(RecordCacheHash,
>  												(void *) &tupDesc,
> -												HASH_ENTER, &found);
> +												HASH_FIND, &found);
>  	if (found && recentry->tupdesc != NULL)
>  	{
>  		tupDesc->tdtypmod = recentry->tupdesc->tdtypmod;
>  		return;
>  	}
>  
> -	/* Not present, so need to manufacture an entry */
> -	recentry->tupdesc = NULL;
>  	oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
>  
>  	/* Look in the SharedRecordTypmodRegistry, if attached */
> @@ -1995,6 +1993,10 @@ assign_record_type_typmod(TupleDesc tupDesc)
>  	}
>  	ensure_record_cache_typmod_slot_exists(entDesc->tdtypmod);
>  	RecordCacheArray[entDesc->tdtypmod] = entDesc;
> +	/* Not present, so need to manufacture an entry */
> +	recentry = (RecordCacheEntry *) hash_search(RecordCacheHash,
> +												(void *) &tupDesc,
> +												HASH_ENTER, NULL);
>  	recentry->tupdesc = entDesc;

ISTM that the ensure_record_cache_typmod_slot_exists() should be moved
to before find_or_make_matching_shared_tupledesc /
CreateTupleDescCopy. Otherwise we can leak if the CreateTupleDescCopy()
succeeds but ensure_record_cache_typmod_slot_exists() fails. Conversely,
if ensure_record_cache_typmod_slot_exists() succeeds, but
CreateTupleDescCopy() we won't leak, since the former is just
repalloc()ing allocations.

Greetings,

Andres Freund