Commit 0f45d1eb authored by Tom Lane's avatar Tom Lane

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.
parent a9b3e4fa
...@@ -8,7 +8,7 @@ ...@@ -8,7 +8,7 @@
* *
* *
* IDENTIFICATION * 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, ...@@ -220,8 +220,11 @@ static OpClassCacheEnt *LookupOpclassInfo(Oid operatorClassOid,
/* /*
* ScanPgRelation * ScanPgRelation
* *
* this is used by RelationBuildDesc to find a pg_class * This is used by RelationBuildDesc to find a pg_class
* tuple matching targetRelId. * 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 * NB: the returned tuple has been copied into palloc'd storage
* and must eventually be freed with heap_freetuple. * and must eventually be freed with heap_freetuple.
...@@ -773,18 +776,18 @@ equalRuleLocks(RuleLock *rlock1, RuleLock *rlock2) ...@@ -773,18 +776,18 @@ equalRuleLocks(RuleLock *rlock1, RuleLock *rlock2)
} }
/* ---------------------------------- /*
* RelationBuildDesc * RelationBuildDesc
* *
* Build a relation descriptor --- either a new one, or by * Build a relation descriptor --- either a new one, or by
* recycling the given old relation object. The latter case * recycling the given old relation object. The latter case
* supports rebuilding a relcache entry without invalidating * 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 * Returns NULL if no pg_class row could be found for the given relid
* (suggesting we are trying to access a just-deleted relation). * (suggesting we are trying to access a just-deleted relation).
* Any other error is reported via elog. * Any other error is reported via elog.
* --------------------------------
*/ */
static Relation static Relation
RelationBuildDesc(Oid targetRelId, Relation oldrelation) RelationBuildDesc(Oid targetRelId, Relation oldrelation)
...@@ -1608,6 +1611,10 @@ RelationClose(Relation relation) ...@@ -1608,6 +1611,10 @@ RelationClose(Relation relation)
* RelationClearRelation just marks the entry as invalid by setting * RelationClearRelation just marks the entry as invalid by setting
* rd_isvalid to false. This routine is called to fix the entry when it * rd_isvalid to false. This routine is called to fix the entry when it
* is next needed. * 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 static void
RelationReloadIndexInfo(Relation relation) RelationReloadIndexInfo(Relation relation)
...@@ -1692,6 +1699,9 @@ RelationReloadIndexInfo(Relation relation) ...@@ -1692,6 +1699,9 @@ RelationReloadIndexInfo(Relation relation)
* usually used when we are notified of a change to an open relation * usually used when we are notified of a change to an open relation
* (one with refcount > 0). However, this routine just does whichever * (one with refcount > 0). However, this routine just does whichever
* it's told to do; callers must determine which they want. * 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 static void
RelationClearRelation(Relation relation, bool rebuild) RelationClearRelation(Relation relation, bool rebuild)
...@@ -2542,12 +2552,14 @@ RelationCacheInitializePhase2(void) ...@@ -2542,12 +2552,14 @@ RelationCacheInitializePhase2(void)
#define LOAD_CRIT_INDEX(indexoid) \ #define LOAD_CRIT_INDEX(indexoid) \
do { \ do { \
LockRelationOid(indexoid, AccessShareLock); \
ird = RelationBuildDesc(indexoid, NULL); \ ird = RelationBuildDesc(indexoid, NULL); \
if (ird == NULL) \ if (ird == NULL) \
elog(PANIC, "could not open critical system index %u", \ elog(PANIC, "could not open critical system index %u", \
indexoid); \ indexoid); \
ird->rd_isnailed = true; \ ird->rd_isnailed = true; \
ird->rd_refcnt = 1; \ ird->rd_refcnt = 1; \
UnlockRelationOid(indexoid, AccessShareLock); \
} while (0) } while (0)
LOAD_CRIT_INDEX(ClassOidIndexId); LOAD_CRIT_INDEX(ClassOidIndexId);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment