Fix exception safety bug in typcache.c.

If an out-of-memory error was thrown at an unfortunate time,
ensure_record_cache_typmod_slot_exists() could leak memory and leave
behind a global state that produced an infinite loop on the next call.

Fix by merging RecordCacheArray and RecordIdentifierArray into a single
array.  With only one allocation or re-allocation, there is no
intermediate state.

Back-patch to all supported releases.

Reported-by: "James Pang (chaolpan)" <chaolpan@cisco.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/PH0PR11MB519113E738814BDDA702EDADD6EFA%40PH0PR11MB5191.namprd11.prod.outlook.com
This commit is contained in:
Thomas Munro 2023-09-13 14:32:24 +12:00
parent 522a31ac87
commit 3acd0599bd
2 changed files with 28 additions and 21 deletions

View File

@ -273,10 +273,15 @@ static const dshash_parameters srtr_typmod_table_params = {
/* hashtable for recognizing registered record types */ /* hashtable for recognizing registered record types */
static HTAB *RecordCacheHash = NULL; static HTAB *RecordCacheHash = NULL;
/* arrays of info about registered record types, indexed by assigned typmod */ typedef struct RecordCacheArrayEntry
static TupleDesc *RecordCacheArray = NULL; {
static uint64 *RecordIdentifierArray = NULL; uint64 id;
static int32 RecordCacheArrayLen = 0; /* allocated length of above arrays */ TupleDesc tupdesc;
} RecordCacheArrayEntry;
/* array of info about registered record types, indexed by assigned typmod */
static RecordCacheArrayEntry *RecordCacheArray = NULL;
static int32 RecordCacheArrayLen = 0; /* allocated length of above array */
static int32 NextRecordTypmod = 0; /* number of entries used */ static int32 NextRecordTypmod = 0; /* number of entries used */
/* /*
@ -1703,10 +1708,9 @@ ensure_record_cache_typmod_slot_exists(int32 typmod)
{ {
if (RecordCacheArray == NULL) if (RecordCacheArray == NULL)
{ {
RecordCacheArray = (TupleDesc *) RecordCacheArray = (RecordCacheArrayEntry *)
MemoryContextAllocZero(CacheMemoryContext, 64 * sizeof(TupleDesc)); MemoryContextAllocZero(CacheMemoryContext,
RecordIdentifierArray = (uint64 *) 64 * sizeof(RecordCacheArrayEntry));
MemoryContextAllocZero(CacheMemoryContext, 64 * sizeof(uint64));
RecordCacheArrayLen = 64; RecordCacheArrayLen = 64;
} }
@ -1714,8 +1718,10 @@ ensure_record_cache_typmod_slot_exists(int32 typmod)
{ {
int32 newlen = pg_nextpower2_32(typmod + 1); int32 newlen = pg_nextpower2_32(typmod + 1);
RecordCacheArray = repalloc0_array(RecordCacheArray, TupleDesc, RecordCacheArrayLen, newlen); RecordCacheArray = repalloc0_array(RecordCacheArray,
RecordIdentifierArray = repalloc0_array(RecordIdentifierArray, uint64, RecordCacheArrayLen, newlen); RecordCacheArrayEntry,
RecordCacheArrayLen,
newlen);
RecordCacheArrayLen = newlen; RecordCacheArrayLen = newlen;
} }
} }
@ -1753,8 +1759,8 @@ lookup_rowtype_tupdesc_internal(Oid type_id, int32 typmod, bool noError)
{ {
/* It is already in our local cache? */ /* It is already in our local cache? */
if (typmod < RecordCacheArrayLen && if (typmod < RecordCacheArrayLen &&
RecordCacheArray[typmod] != NULL) RecordCacheArray[typmod].tupdesc != NULL)
return RecordCacheArray[typmod]; return RecordCacheArray[typmod].tupdesc;
/* Are we attached to a shared record typmod registry? */ /* Are we attached to a shared record typmod registry? */
if (CurrentSession->shared_typmod_registry != NULL) if (CurrentSession->shared_typmod_registry != NULL)
@ -1780,19 +1786,19 @@ lookup_rowtype_tupdesc_internal(Oid type_id, int32 typmod, bool noError)
* Our local array can now point directly to the TupleDesc * Our local array can now point directly to the TupleDesc
* in shared memory, which is non-reference-counted. * in shared memory, which is non-reference-counted.
*/ */
RecordCacheArray[typmod] = tupdesc; RecordCacheArray[typmod].tupdesc = tupdesc;
Assert(tupdesc->tdrefcount == -1); Assert(tupdesc->tdrefcount == -1);
/* /*
* We don't share tupdesc identifiers across processes, so * We don't share tupdesc identifiers across processes, so
* assign one locally. * assign one locally.
*/ */
RecordIdentifierArray[typmod] = ++tupledesc_id_counter; RecordCacheArray[typmod].id = ++tupledesc_id_counter;
dshash_release_lock(CurrentSession->shared_typmod_table, dshash_release_lock(CurrentSession->shared_typmod_table,
entry); entry);
return RecordCacheArray[typmod]; return RecordCacheArray[typmod].tupdesc;
} }
} }
} }
@ -2005,10 +2011,10 @@ assign_record_type_typmod(TupleDesc tupDesc)
ensure_record_cache_typmod_slot_exists(entDesc->tdtypmod); ensure_record_cache_typmod_slot_exists(entDesc->tdtypmod);
} }
RecordCacheArray[entDesc->tdtypmod] = entDesc; RecordCacheArray[entDesc->tdtypmod].tupdesc = entDesc;
/* Assign a unique tupdesc identifier, too. */ /* Assign a unique tupdesc identifier, too. */
RecordIdentifierArray[entDesc->tdtypmod] = ++tupledesc_id_counter; RecordCacheArray[entDesc->tdtypmod].id = ++tupledesc_id_counter;
/* Fully initialized; create the hash table entry */ /* Fully initialized; create the hash table entry */
recentry = (RecordCacheEntry *) hash_search(RecordCacheHash, recentry = (RecordCacheEntry *) hash_search(RecordCacheHash,
@ -2057,10 +2063,10 @@ assign_record_type_identifier(Oid type_id, int32 typmod)
* It's a transient record type, so look in our record-type table. * It's a transient record type, so look in our record-type table.
*/ */
if (typmod >= 0 && typmod < RecordCacheArrayLen && if (typmod >= 0 && typmod < RecordCacheArrayLen &&
RecordCacheArray[typmod] != NULL) RecordCacheArray[typmod].tupdesc != NULL)
{ {
Assert(RecordIdentifierArray[typmod] != 0); Assert(RecordCacheArray[typmod].id != 0);
return RecordIdentifierArray[typmod]; return RecordCacheArray[typmod].id;
} }
/* For anonymous or unrecognized record type, generate a new ID */ /* For anonymous or unrecognized record type, generate a new ID */
@ -2140,7 +2146,7 @@ SharedRecordTypmodRegistryInit(SharedRecordTypmodRegistry *registry,
TupleDesc tupdesc; TupleDesc tupdesc;
bool found; bool found;
tupdesc = RecordCacheArray[typmod]; tupdesc = RecordCacheArray[typmod].tupdesc;
if (tupdesc == NULL) if (tupdesc == NULL)
continue; continue;

View File

@ -2251,6 +2251,7 @@ ReadLocalXLogPageNoWaitPrivate
ReadReplicationSlotCmd ReadReplicationSlotCmd
ReassignOwnedStmt ReassignOwnedStmt
RecheckForeignScan_function RecheckForeignScan_function
RecordCacheArrayEntry
RecordCacheEntry RecordCacheEntry
RecordCompareData RecordCompareData
RecordIOData RecordIOData