mirror of
https://git.postgresql.org/git/postgresql.git
synced 2024-12-09 08:10:09 +08:00
Avoid potential deadlock in InitCatCachePhase2().
Opening a catcache's index could require reading from that cache's own catalog, which of course would acquire AccessShareLock on the catalog. So the original coding here risks locking index before heap, which could deadlock against another backend trying to get exclusive locks in the normal order. Because InitCatCachePhase2 is only called when a backend has to start up without a relcache init file, the deadlock was seldom seen in the field. (And by the same token, there's no need to worry about any performance disadvantage; so not much point in trying to distinguish exactly which catalogs have the risk.) Bug report, diagnosis, and patch by Nikhil Sontakke. Additional commentary by me. Back-patch to all supported branches.
This commit is contained in:
parent
6e8e7cc580
commit
1192ba8b67
9
src/backend/utils/cache/catcache.c
vendored
9
src/backend/utils/cache/catcache.c
vendored
@ -26,6 +26,7 @@
|
||||
#ifdef CATCACHE_STATS
|
||||
#include "storage/ipc.h" /* for on_proc_exit */
|
||||
#endif
|
||||
#include "storage/lmgr.h"
|
||||
#include "utils/builtins.h"
|
||||
#include "utils/fmgroids.h"
|
||||
#include "utils/inval.h"
|
||||
@ -967,8 +968,16 @@ InitCatCachePhase2(CatCache *cache, bool touch_index)
|
||||
{
|
||||
Relation idesc;
|
||||
|
||||
/*
|
||||
* We must lock the underlying catalog before opening the index to
|
||||
* avoid deadlock, since index_open could possibly result in reading
|
||||
* this same catalog, and if anyone else is exclusive-locking this
|
||||
* catalog and index they'll be doing it in that order.
|
||||
*/
|
||||
LockRelationOid(cache->cc_reloid, AccessShareLock);
|
||||
idesc = index_open(cache->cc_indexoid, AccessShareLock);
|
||||
index_close(idesc, AccessShareLock);
|
||||
UnlockRelationOid(cache->cc_reloid, AccessShareLock);
|
||||
}
|
||||
}
|
||||
|
||||
|
10
src/backend/utils/cache/relcache.c
vendored
10
src/backend/utils/cache/relcache.c
vendored
@ -1651,6 +1651,12 @@ RelationClose(Relation relation)
|
||||
* 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.)
|
||||
*
|
||||
* If the target index is an index on pg_class or pg_index, we'd better have
|
||||
* previously gotten at least AccessShareLock on its underlying catalog,
|
||||
* else we are at risk of deadlock against someone trying to exclusive-lock
|
||||
* the heap and index in that order. This is ensured in current usage by
|
||||
* only applying this to indexes being opened or having positive refcount.
|
||||
*/
|
||||
static void
|
||||
RelationReloadIndexInfo(Relation relation)
|
||||
@ -3611,6 +3617,10 @@ RelationGetIndexPredicate(Relation relation)
|
||||
* Attribute numbers are offset by FirstLowInvalidHeapAttributeNumber so that
|
||||
* we can include system attributes (e.g., OID) in the bitmap representation.
|
||||
*
|
||||
* Caller had better hold at least RowExclusiveLock on the target relation
|
||||
* to ensure that it has a stable set of indexes. This also makes it safe
|
||||
* (deadlock-free) for us to take locks on the relation's indexes.
|
||||
*
|
||||
* The returned result is palloc'd in the caller's memory context and should
|
||||
* be bms_free'd when not needed anymore.
|
||||
*/
|
||||
|
Loading…
Reference in New Issue
Block a user