From 0f45d1eb08d0916f8569644223fe0c11390aae0a Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 16 Apr 2008 18:23:04 +0000 Subject: [PATCH] Fix LOAD_CRIT_INDEX() macro to take out AccessShareLock on the system index it is trying to build a relcache entry for. This is an oversight in my 8.2 patch that tried to ensure we always took a lock on a relation before trying to build its relcache entry. The implication is that if someone committed a reindex of a critical system index at about the same time that some other backend were starting up without a valid pg_internal.init file, the second one might PANIC due to not seeing any valid version of the index's pg_class row. Improbable case, but definitely not impossible. --- src/backend/utils/cache/relcache.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 9a32fd5845..b199a478df 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.270 2008/04/01 00:48:33 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/utils/cache/relcache.c,v 1.271 2008/04/16 18:23:04 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -220,8 +220,11 @@ static OpClassCacheEnt *LookupOpclassInfo(Oid operatorClassOid, /* * ScanPgRelation * - * this is used by RelationBuildDesc to find a pg_class - * tuple matching targetRelId. + * This is used by RelationBuildDesc to find a pg_class + * tuple matching targetRelId. The caller must hold at least + * AccessShareLock on the target relid to prevent concurrent-update + * scenarios --- else our SnapshotNow scan might fail to find any + * version that it thinks is live. * * NB: the returned tuple has been copied into palloc'd storage * and must eventually be freed with heap_freetuple. @@ -773,18 +776,18 @@ equalRuleLocks(RuleLock *rlock1, RuleLock *rlock2) } -/* ---------------------------------- +/* * RelationBuildDesc * * Build a relation descriptor --- either a new one, or by * recycling the given old relation object. The latter case * supports rebuilding a relcache entry without invalidating - * pointers to it. + * pointers to it. The caller must hold at least + * AccessShareLock on the target relid. * * Returns NULL if no pg_class row could be found for the given relid * (suggesting we are trying to access a just-deleted relation). * Any other error is reported via elog. - * -------------------------------- */ static Relation RelationBuildDesc(Oid targetRelId, Relation oldrelation) @@ -1608,6 +1611,10 @@ RelationClose(Relation relation) * RelationClearRelation just marks the entry as invalid by setting * rd_isvalid to false. This routine is called to fix the entry when it * is next needed. + * + * We assume that at the time we are called, we have at least AccessShareLock + * on the target index. (Note: in the calls from RelationClearRelation, + * this is legitimate because we know the rel has positive refcount.) */ static void RelationReloadIndexInfo(Relation relation) @@ -1692,6 +1699,9 @@ RelationReloadIndexInfo(Relation relation) * usually used when we are notified of a change to an open relation * (one with refcount > 0). However, this routine just does whichever * it's told to do; callers must determine which they want. + * + * NB: when rebuilding, we'd better hold some lock on the relation. + * In current usages this is presumed true because it has refcnt > 0. */ static void RelationClearRelation(Relation relation, bool rebuild) @@ -2542,12 +2552,14 @@ RelationCacheInitializePhase2(void) #define LOAD_CRIT_INDEX(indexoid) \ do { \ + LockRelationOid(indexoid, AccessShareLock); \ ird = RelationBuildDesc(indexoid, NULL); \ 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);