When loading critical system indexes into the relcache, ensure we lock the

underlying catalog not only the index itself.  Otherwise, if the cache
load process touches the catalog (which will happen for many though not
all of these indexes), we are locking index before parent table, which can
result in a deadlock against processes that are trying to lock them in the
normal order.  Per today's failure on buildfarm member gothic_moth; it's
surprising the problem hadn't been identified before.

Back-patch to 8.2.  Earlier releases didn't have the issue because they
didn't try to lock these indexes during load (instead assuming that they
couldn't change schema at all during multiuser operation).

diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 8235d93d8b6ecef56db7cd00edada54d43cbb0f0..99a7482fb931e264d75099ee5656cc89eeb96b45 100644 (file)
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/utils/cache/relcache.c,v 1.250.2.4 2010/01/12 18:12:40 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/utils/cache/relcache.c,v 1.250.2.5 2010/01/13 23:07:28 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -43,8 +43,10 @@
 #include "catalog/pg_constraint.h"
 #include "catalog/pg_namespace.h"
 #include "catalog/pg_opclass.h"
+#include "catalog/pg_operator.h"
 #include "catalog/pg_proc.h"
 #include "catalog/pg_rewrite.h"
+#include "catalog/pg_trigger.h"
 #include "catalog/pg_type.h"
 #include "commands/trigger.h"
 #include "miscadmin.h"
@@ -194,6 +196,7 @@ static void RelationParseRelOptions(Relation relation, HeapTuple tuple);
 static void RelationBuildTupleDesc(Relation relation);
 static Relation RelationBuildDesc(Oid targetRelId, bool insertIt);
 static void RelationInitPhysicalAddr(Relation relation);
+static void load_critical_index(Oid indexoid, Oid heapoid);
 static TupleDesc GetPgClassDescriptor(void);
 static TupleDesc GetPgIndexDescriptor(void);
 static void AttrDefaultFetch(Relation relation);
@@ -2451,28 +2454,22 @@ RelationCacheInitializePhase2(void)
         */
        if (!criticalRelcachesBuilt)
        {
-               Relation        ird;
-
-#define LOAD_CRIT_INDEX(indexoid) \
-               do { \
-                       LockRelationOid(indexoid, AccessShareLock); \
-                       ird = RelationBuildDesc(indexoid, true); \
-                       if (ird == NULL) \
-                               elog(PANIC, "could not open critical system index %u", \
-                                        indexoid); \
-                       ird->rd_isnailed = true; \
-                       ird->rd_refcnt = 1; \
-                       UnlockRelationOid(indexoid, AccessShareLock); \
-               } while (0)
-
-               LOAD_CRIT_INDEX(ClassOidIndexId);
-               LOAD_CRIT_INDEX(AttributeRelidNumIndexId);
-               LOAD_CRIT_INDEX(IndexRelidIndexId);
-               LOAD_CRIT_INDEX(AccessMethodStrategyIndexId);
-               LOAD_CRIT_INDEX(AccessMethodProcedureIndexId);
-               LOAD_CRIT_INDEX(OperatorOidIndexId);
-               LOAD_CRIT_INDEX(RewriteRelRulenameIndexId);
-               LOAD_CRIT_INDEX(TriggerRelidNameIndexId);
+               load_critical_index(ClassOidIndexId,
+                                                       RelationRelationId);
+               load_critical_index(AttributeRelidNumIndexId,
+                                                       AttributeRelationId);
+               load_critical_index(IndexRelidIndexId,
+                                                       IndexRelationId);
+               load_critical_index(AccessMethodStrategyIndexId,
+                                                       AccessMethodOperatorRelationId);
+               load_critical_index(AccessMethodProcedureIndexId,
+                                                       AccessMethodProcedureRelationId);
+               load_critical_index(OperatorOidIndexId,
+                                                       OperatorRelationId);
+               load_critical_index(RewriteRelRulenameIndexId,
+                                                       RewriteRelationId);
+               load_critical_index(TriggerRelidNameIndexId,
+                                                       TriggerRelationId);
 
 #define NUM_CRITICAL_INDEXES   8               /* fix if you change list above */
 
@@ -2606,6 +2603,34 @@ RelationCacheInitializePhase2(void)
        }
 }
 
+/*
+ * Load one critical system index into the relcache
+ *
+ * indexoid is the OID of the target index, heapoid is the OID of the catalog
+ * it belongs to.
+ */
+static void
+load_critical_index(Oid indexoid, Oid heapoid)
+{
+       Relation        ird;
+
+       /*
+        * We must lock the underlying catalog before locking the index to avoid
+        * deadlock, since RelationBuildDesc might well need to read the catalog,
+        * and if anyone else is exclusive-locking this catalog and index they'll
+        * be doing it in that order.
+        */
+       LockRelationOid(heapoid, AccessShareLock);
+       LockRelationOid(indexoid, AccessShareLock);
+       ird = RelationBuildDesc(indexoid, true);
+       if (ird == NULL)
+               elog(PANIC, "could not open critical system index %u", indexoid);
+       ird->rd_isnailed = true;
+       ird->rd_refcnt = 1;
+       UnlockRelationOid(indexoid, AccessShareLock);
+       UnlockRelationOid(heapoid, AccessShareLock);
+}
+
 /*
  * GetPgClassDescriptor -- get a predefined tuple descriptor for pg_class
  * GetPgIndexDescriptor -- get a predefined tuple descriptor for pg_index