Make type cache initialization more resilient on re-entry after OOM

An out-of-memory failure while initializing the type cache hash tables
would issue an ERROR and leave a backend in a partially inconsistent
state.  Without assertions, the server would crash with a NULL pointer
dereference on initialization re-entry when doing a type lookup due to
one or both hash tables missing.  An assertion would trigger if these
are enabled in the build.

This commit changes the ordering of the type cache initialization to
become more robust on re-entry after an in-flight allocation failure:
- The two hash tables are initialized first, and can only be initialized
once.
- The initialization is considered as done once the in-progress list is
allocated in the CacheMemoryContext.  This is now the last allocation
step.
- Last, the callbacks are registered.  These can only fail with a FATAL
error, taking down the process so leaving the process in a non-complete
state is fine.

This is in the same spirit as b85f9c00fb88 and 29fb598b9cad, where
random allocation failures can make the backend go crazy in the code
paths fixed due to the static states becoming inconsistent.  Like the
other fixes, this is unlikely going to show up in practice, so no
backpatch is done.

Reported-by: Alexander Lakhin <exclusion@gmail.com>
Author: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Matthias van de Meent <boekewurm+postgres@gmail.com>
Discussion: https://postgr.es/m/e77acaac-a1b3-40b3-99ee-5769b4e453e4@gmail.com

diff --git a/src/backend/utils/cache/typcache.c b/src/backend/utils/cache/typcache.c
index da91a2ff1dd8872aad4a8e8446846fcfe9bb3d04..650b5d9bad90d338f42bbe462523a96260f1b2d4 100644 (file)
--- a/src/backend/utils/cache/typcache.c
+++ b/src/backend/utils/cache/typcache.c
@@ -392,50 +392,59 @@ lookup_type_cache(Oid type_id, int flags)
        bool            found;
        int                     in_progress_offset;
 
-       if (TypeCacheHash == NULL)
+       if (in_progress_list == NULL)
        {
                /* First time through: initialize the hash table */
                HASHCTL         ctl;
                int                     allocsize;
 
-               ctl.keysize = sizeof(Oid);
-               ctl.entrysize = sizeof(TypeCacheEntry);
-
-               /*
-                * TypeCacheEntry takes hash value from the system cache. For
-                * TypeCacheHash we use the same hash in order to speedup search by
-                * hash value. This is used by hash_seq_init_with_hash_value().
-                */
-               ctl.hash = type_cache_syshash;
-
-               TypeCacheHash = hash_create("Type information cache", 64,
-                                                                       &ctl, HASH_ELEM | HASH_FUNCTION);
+               if (TypeCacheHash == NULL)
+               {
+                       ctl.keysize = sizeof(Oid);
+                       ctl.entrysize = sizeof(TypeCacheEntry);
 
-               Assert(RelIdToTypeIdCacheHash == NULL);
+                       /*
+                        * TypeCacheEntry takes hash value from the system cache. For
+                        * TypeCacheHash we use the same hash in order to speedup search
+                        * by hash value. This is used by hash_seq_init_with_hash_value().
+                        */
+                       ctl.hash = type_cache_syshash;
 
-               ctl.keysize = sizeof(Oid);
-               ctl.entrysize = sizeof(RelIdToTypeIdCacheEntry);
-               RelIdToTypeIdCacheHash = hash_create("Map from relid to OID of cached composite type", 64,
-                                                                                        &ctl, HASH_ELEM | HASH_BLOBS);
+                       TypeCacheHash = hash_create("Type information cache", 64,
+                                                                               &ctl, HASH_ELEM | HASH_FUNCTION);
+               }
 
-               /* Also set up callbacks for SI invalidations */
-               CacheRegisterRelcacheCallback(TypeCacheRelCallback, (Datum) 0);
-               CacheRegisterSyscacheCallback(TYPEOID, TypeCacheTypCallback, (Datum) 0);
-               CacheRegisterSyscacheCallback(CLAOID, TypeCacheOpcCallback, (Datum) 0);
-               CacheRegisterSyscacheCallback(CONSTROID, TypeCacheConstrCallback, (Datum) 0);
+               if (RelIdToTypeIdCacheHash == NULL)
+               {
+                       ctl.keysize = sizeof(Oid);
+                       ctl.entrysize = sizeof(RelIdToTypeIdCacheEntry);
+                       RelIdToTypeIdCacheHash = hash_create("Map from relid to OID of cached composite type", 64,
+                                                                                                &ctl, HASH_ELEM | HASH_BLOBS);
+               }
 
                /* Also make sure CacheMemoryContext exists */
                if (!CacheMemoryContext)
                        CreateCacheMemoryContext();
 
                /*
-                * reserve enough in_progress_list slots for many cases
+                * Reserve enough in_progress_list slots for many cases.  This is the
+                * last allocation on purpose, done after the two others.
                 */
                allocsize = 4;
                in_progress_list =
                        MemoryContextAlloc(CacheMemoryContext,
                                                           allocsize * sizeof(*in_progress_list));
                in_progress_list_maxlen = allocsize;
+
+               /*
+                * Set up callbacks for SI invalidations.  These steps are done last,
+                * once all the other initializations are done, and can fail only with
+                * a FATAL error.
+                */
+               CacheRegisterRelcacheCallback(TypeCacheRelCallback, (Datum) 0);
+               CacheRegisterSyscacheCallback(TYPEOID, TypeCacheTypCallback, (Datum) 0);
+               CacheRegisterSyscacheCallback(CLAOID, TypeCacheOpcCallback, (Datum) 0);
+               CacheRegisterSyscacheCallback(CONSTROID, TypeCacheConstrCallback, (Datum) 0);
        }
 
        Assert(TypeCacheHash != NULL && RelIdToTypeIdCacheHash != NULL);