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).

1 files changed, 50 insertions, 24 deletions

diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 7e4b375a1bf..9a24141a213 100644
--- 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.287.2.2 2010/01/12 18:12:26 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/cache/relcache.c,v 1.287.2.3 2010/01/13 23:07:15 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -45,8 +45,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"
@@ -202,6 +204,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);
@@ -2626,29 +2629,24 @@ 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(OpclassOidIndexId);
-		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(OpclassOidIndexId,
+							OperatorClassRelationId);
+		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	9		/* fix if you change list above */
 
@@ -2783,6 +2781,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
  *