Given the severity of the (never released) bug that commit 0bada39 fixed, I
felt I owed improvements to our ability to detect the next bug like it. The
attached patch adds pertinent assertions. For obstacles to the original bug
detection, see Appendix 1.
One of the exceptions this patch allows is new for v18. I think that
exception is okay, but I'm starting this thread to invite other opinions. In
v17, varstr_cmp() avoids catcache searches for collation "C", so catalog index
maintenance doesn't do catcache searches. Non-catalog, non-"C" index
maintenance does COLLID catcache searches in v17 (likely in all supported
versions). v18 additionally reaches those COLLID catcache searches when
updating catalog indexes having text columns:
[local] test=*# select attrelid::regclass, attname , attcollation::regcollation
from pg_attribute a join pg_index i on attrelid = i.indexrelid where atttypid =
'text'::regtype and indisunique;
attrelid │ attname │ attcollation
────────────────────────────────────┼──────────┼──────────────
pg_replication_origin_roname_index │ roname │ "C"
pg_seclabel_object_index │ provider │ "C"
pg_shseclabel_object_index │ provider │ "C"
pg_parameter_acl_parname_index │ parname │ "C"
(4 rows)
See Appendix 2 for a stack trace from an earlier version of the patch.
relcache builds don't use those indexes, and that's not likely to change.
Hence, I don't see a need to move or avoid these catcache searches. We'd have
problems if we later make a text column part of a UNIQUE key of, say,
pg_class. An INSERT of pg_class would then do a COLLID search while we hold
an exclusive lock on a pg_class index page. If the COLLID search reached a
relcache build that attempts to read that index page, the outcome would be an
undetected lwlock deadlock.
Separately, the catcache.c part of the attached patch makes a questionable
allowance for TypeCacheRelCallback() to reach GetSysCacheHashValue1(TYPEOID)
when not in a transaction. I suspect that's buggy after an OOM at
lookup_type_cache() -> CreateCacheMemoryContext() leaves the callback
registered without having initialized the TYPEOID catcache. Other than that
improbable outcome, it seems fine. Unlike the main $SUBJECT, this is ancient.
==== Appendix 1: difficulty of original commit 0bada39 bug detection
It required a relcache inval in this call stack:
PrepareToInvalidateCacheTuple() [common]
CatalogCacheInitializeCache() [common: happens *unless* debug_discard_caches]
table_open(cache->cc_reloid, AccessShareLock) [always]
RelationBuildDesc() [rare: only on cache miss / inval]
debug_discard_caches doesn't reproduce it, because it reaches
CatalogCacheInitializeCache() earlier, via this code:
if (needNewCacheFile)
{
/*
* Force all the catcaches to finish initializing and thereby
open the
* catalogs and indexes they use. This will preload the
relcache with
* entries for all the most important system catalogs and
indexes, so
* that the init files will be most useful for future backends.
*/
InitCatalogCachePhase2();
==== Appendix 2: stack of COLLID search while catalog buffer exclusive-locked
#0 0x00007f07a3241387 in raise () from /lib64/libc.so.6
#1 0x00007f07a3242a78 in abort () from /lib64/libc.so.6
#2 0x0000000000c08133 in ExceptionalCondition (
conditionName=0xe4a298 "tag.spcOid != GLOBALTABLESPACE_OID",
fileName=0xe49640 "bufmgr.c", lineNumber=4119) at assert.c:66
#3 0x00000000009d90f0 in AssertNotCatalogBufferLock (lock=0x7f07921e4ab0,
unused_context=0x0) at bufmgr.c:4119
#4 0x0000000000a10849 in ForEachLWLockHeldByMeInMode (mode=LW_EXCLUSIVE,
callback=0x9d906f <AssertNotCatalogBufferLock>, context=0x0)
at lwlock.c:1976
#5 0x00000000009d9165 in AssertNoCatalogBufferLocksInMode (mode=2)
at bufmgr.c:4142
#6 0x0000000000be3770 in ConditionalCatalogCacheInitializeCache (
cache=0x246bd80) at catcache.c:1064
#7 0x0000000000be3d59 in SearchCatCacheInternal (cache=0x246bd80, nkeys=1,
v1=950, v2=0, v3=0, v4=0) at catcache.c:1397
#8 0x0000000000be3c29 in SearchCatCache1 (cache=0x246bd80, v1=950)
at catcache.c:1344
#9 0x0000000000c01064 in SearchSysCache1 (cacheId=16, key1=950)
at syscache.c:228
#10 0x0000000000b4054a in create_pg_locale (collid=950, context=0x24dec20)
at pg_locale.c:1075
#11 0x0000000000b40b1e in pg_newlocale_from_collation (collid=950)
at pg_locale.c:1223
#12 0x0000000000bcd4a4 in varstr_cmp (arg1=0x7f07965dbfe1 "pg_27254", len1=8,
arg2=0x242da19 "pg_34104", len2=8, collid=950) at varlena.c:1617
#13 0x0000000000bcd760 in text_cmp (arg1=0x7f07965dbfe0, arg2=0x242da18,
collid=950) at varlena.c:1671
#14 0x0000000000bce102 in bttextcmp (fcinfo=0x7fff47fe50a0) at varlena.c:1892
#15 0x0000000000c13332 in FunctionCall2Coll (flinfo=0x242c878, collation=950,
arg1=139670564224992, arg2=37935640) at fmgr.c:1161
#16 0x000000000054019a in _bt_compare (rel=0x7f07a4a6b218, key=0x242c850,
page=0x7f07965da000 "", offnum=1) at nbtsearch.c:765
#17 0x000000000053fc17 in _bt_binsrch_insert (rel=0x7f07a4a6b218,
insertstate=0x7fff47fe54c0) at nbtsearch.c:537
#18 0x000000000052dbd8 in _bt_check_unique (rel=0x7f07a4a6b218,
insertstate=0x7fff47fe54c0, heapRel=0x7f07a4a6a338,
checkUnique=UNIQUE_CHECK_YES, is_unique=0x7fff47fe54f1,
speculativeToken=0x7fff47fe54bc) at nbtinsert.c:443
#19 0x000000000052d74f in _bt_doinsert (rel=0x7f07a4a6b218, itup=0x242da10,
checkUnique=UNIQUE_CHECK_YES, indexUnchanged=false, heapRel=0x7f07a4a6a338)
at nbtinsert.c:210
#20 0x000000000053c4e3 in btinsert (rel=0x7f07a4a6b218, values=0x7fff47fe5610,
isnull=0x7fff47fe55f0, ht_ctid=0x242d534, heapRel=0x7f07a4a6a338,
checkUnique=UNIQUE_CHECK_YES, indexUnchanged=false, indexInfo=0x24ea0c8)
at nbtree.c:215
#21 0x0000000000527c5a in index_insert (indexRelation=0x7f07a4a6b218,
values=0x7fff47fe5610, isnull=0x7fff47fe55f0, heap_t_ctid=0x242d534,
heapRelation=0x7f07a4a6a338, checkUnique=UNIQUE_CHECK_YES,
indexUnchanged=false, indexInfo=0x24ea0c8) at indexam.c:230
#22 0x00000000005dd100 in CatalogIndexInsert (indstate=0x243c1e8,
heapTuple=0x242d530, updateIndexes=TU_All) at indexing.c:170
#23 0x00000000005dd21c in CatalogTupleInsert (heapRel=0x7f07a4a6a338,
tup=0x242d530) at indexing.c:243
#24 0x000000000096fbac in replorigin_create (roname=0x7fff47fe5a30 "pg_34104")
at origin.c:324
#25 0x00000000006f88c6 in CreateSubscription (pstate=0x242c710,
stmt=0x2401f80, isTopLevel=true) at subscriptioncmds.c:696
#26 0x0000000000a34d61 in ProcessUtilitySlow (pstate=0x242c710,
pstmt=0x2402030,
queryString=0x2401390 "CREATE SUBSCRIPTION regress_testsub CONNECTION
'dbname=regress_doesnotexist' PUBLICATION testpub WITH (connect = false);",
context=PROCESS_UTILITY_TOPLEVEL, params=0x0, queryEnv=0x0,
dest=0x24023b0, qc=0x7fff47fe6360) at utility.c:1860
#27 0x0000000000a334db in standard_ProcessUtility (pstmt=0x2402030,
queryString=0x2401390 "CREATE SUBSCRIPTION regress_testsub CONNECTION
'dbname=regress_doesnotexist' PUBLICATION testpub WITH (connect = false);",
readOnlyTree=false, context=PROCESS_UTILITY_TOPLEVEL, params=0x0,
queryEnv=0x0, dest=0x24023b0, qc=0x7fff47fe6360) at utility.c:1070
#28 0x0000000000a32674 in ProcessUtility (pstmt=0x2402030,
queryString=0x2401390 "CREATE SUBSCRIPTION regress_testsub CONNECTION
'dbname=regress_doesnotexist' PUBLICATION testpub WITH (connect = false);",
readOnlyTree=false, context=PROCESS_UTILITY_TOPLEVEL, params=0x0,
queryEnv=0x0, dest=0x24023b0, qc=0x7fff47fe6360) at utility.c:523
#29 0x0000000000a31259 in PortalRunUtility (portal=0x2485240, pstmt=0x2402030,
isTopLevel=true, setHoldSnapshot=false, dest=0x24023b0, qc=0x7fff47fe6360)
at pquery.c:1185
#30 0x0000000000a314f5 in PortalRunMulti (portal=0x2485240, isTopLevel=true,
setHoldSnapshot=false, dest=0x24023b0, altdest=0x24023b0,
qc=0x7fff47fe6360) at pquery.c:1349
#31 0x0000000000a309e8 in PortalRun (portal=0x2485240,
count=9223372036854775807, isTopLevel=true, dest=0x24023b0,
altdest=0x24023b0, qc=0x7fff47fe6360) at pquery.c:820
#32 0x0000000000a29c84 in exec_simple_query (
query_string=0x2401390 "CREATE SUBSCRIPTION regress_testsub CONNECTION
'dbname=regress_doesnotexist' PUBLICATION testpub WITH (connect = false);")
at postgres.c:1274
#33 0x0000000000a2ea3d in PostgresMain (dbname=0x243eaf0 "regression",
username=0x243ead8 "nm") at postgres.c:4768
#34 0x0000000000a2599e in BackendMain (startup_data=0x7fff47fe66b0,
startup_data_len=24) at backend_startup.c:124
#35 0x000000000093fe47 in postmaster_child_launch (child_type=B_BACKEND,
child_slot=20, startup_data=0x7fff47fe66b0, startup_data_len=24,
client_sock=0x7fff47fe6700) at launch_backend.c:290
#36 0x0000000000945d55 in BackendStartup (client_sock=0x7fff47fe6700)
at postmaster.c:3580
#37 0x0000000000943694 in ServerLoop () at postmaster.c:1702
#38 0x000000000094305c in PostmasterMain (argc=8, argv=0x23fbb30)
at postmaster.c:1400
#39 0x00000000007f6c6e in main (argc=8, argv=0x23fbb30) at main.c:227
From: Noah Misch <n...@leadboat.com>
Assert lack of hazardous buffer locks before possible catalog read.
Commit 0bada39c83a150079567a6e97b1a25a198f30ea3 fixed a bug of this kind,
which existed in all branches for six days before detection. While the
probability of reaching the trouble was low, the disruption was extreme. No
new backends could start, and service restoration needed an immediate
shutdown. Hence, add this to catch the next bug like it.
The new check in RelationIdGetRelation() suffices to make autovacuum detect
the bug in commit 243e9b40f1b2dd09d6e5bf91ebf6e822a2cd3704 that led to commit
0bada39. This also checks in a number of similar places. It replaces each
Assert(IsTransactionState()) that pertained to a conditional catalog read.
No back-patch for now, but a back-patch of commit 243e9b4 should back-patch
this, too. A back-patch could omit the src/test/regress changes, since back
branches won't gain new index columns.
Reported-by: Alexander Lakhin <exclus...@gmail.com>
Reviewed-by: FIXME
Discussion: https://postgr.es/m/FIXME
Discussion: https://postgr.es/m/10ec0bc3-5933-1189-6bb8-5dec41145...@gmail.com
diff --git a/src/backend/catalog/catalog.c b/src/backend/catalog/catalog.c
index a6edf61..35ebb0c 100644
--- a/src/backend/catalog/catalog.c
+++ b/src/backend/catalog/catalog.c
@@ -34,6 +34,7 @@
#include "catalog/pg_namespace.h"
#include "catalog/pg_parameter_acl.h"
#include "catalog/pg_replication_origin.h"
+#include "catalog/pg_seclabel.h"
#include "catalog/pg_shdepend.h"
#include "catalog/pg_shdescription.h"
#include "catalog/pg_shseclabel.h"
@@ -136,6 +137,36 @@ IsCatalogRelationOid(Oid relid)
}
/*
+ * IsCatalogTextUniqueIndexOid
+ * True iff the relation identified by this OID is a catalog
UNIQUE index
+ * having a column of type "text".
+ *
+ * The relcache must not use these indexes. Inserting into any
UNIQUE
+ * index compares index keys while holding BUFFER_LOCK_EXCLUSIVE.
+ * bttextcmp() can search the COLLID catcache. Depending on
concurrent
+ * invalidation traffic, catcache can reach relcache builds. A
backend
+ * would self-deadlock on LWLocks if the relcache build read the
+ * exclusive-locked buffer.
+ *
+ * To avoid being itself the cause of self-deadlock, this doesn't
read
+ * catalogs. Instead, it uses a hard-coded list with a supporting
+ * regression test.
+ */
+bool
+IsCatalogTextUniqueIndexOid(Oid relid)
+{
+ switch (relid)
+ {
+ case ParameterAclParnameIndexId:
+ case ReplicationOriginNameIndex:
+ case SecLabelObjectIndexId:
+ case SharedSecLabelObjectIndexId:
+ return true;
+ }
+ return false;
+}
+
+/*
* IsInplaceUpdateRelation
* True iff core code performs inplace updates on the relation.
*
diff --git a/src/backend/storage/buffer/bufmgr.c
b/src/backend/storage/buffer/bufmgr.c
index db8f2b1..ed45d90 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -40,6 +40,9 @@
#include "access/tableam.h"
#include "access/xloginsert.h"
#include "access/xlogutils.h"
+#ifdef USE_ASSERT_CHECKING
+#include "catalog/pg_tablespace_d.h"
+#endif
#include "catalog/storage.h"
#include "catalog/storage_xlog.h"
#include "executor/instrument.h"
@@ -541,6 +544,10 @@ static void RelationCopyStorageUsingBuffer(RelFileLocator
srclocator,
ForkNumber forkNum, bool permanent);
static void AtProcExit_Buffers(int code, Datum arg);
static void CheckForBufferLeaks(void);
+#ifdef USE_ASSERT_CHECKING
+static void AssertNotCatalogBufferLock(LWLock *lock, LWLockMode mode,
+ void
*unused_context);
+#endif
static int rlocator_comparator(const void *p1, const void *p2);
static inline int buffertag_comparator(const BufferTag *ba, const BufferTag
*bb);
static inline int ckpt_buforder_comparator(const CkptSortItem *a, const
CkptSortItem *b);
@@ -4097,6 +4104,69 @@ CheckForBufferLeaks(void)
#endif
}
+#ifdef USE_ASSERT_CHECKING
+/*
+ * Check for exclusive-locked catalog buffers. This is the core of
+ * AssertCouldGetRelation().
+ *
+ * A backend would self-deadlock on LWLocks if the catalog scan read the
+ * exclusive-locked buffer. The main threat is exclusive-locked buffers of
+ * catalogs used in relcache, because a catcache search on any catalog may
+ * build that catalog's relcache entry. We don't have an inventory of
+ * catalogs relcache uses, so just check buffers of most catalogs.
+ *
+ * It's better to minimize waits while holding an exclusive buffer lock, so it
+ * would be nice to broaden this check not to be catalog-specific. However,
+ * bttextcmp() accesses pg_collation, and non-core opclasses might similarly
+ * read tables. That is deadlock-free as long as there's no loop in the
+ * dependency graph: modifying table A may cause an opclass to read table B,
+ * but it must not cause a read of table A.
+ */
+void
+AssertBufferLocksPermitCatalogRead(void)
+{
+ ForEachLWLockHeldByMe(AssertNotCatalogBufferLock, NULL);
+}
+
+static void
+AssertNotCatalogBufferLock(LWLock *lock, LWLockMode mode,
+ void *unused_context)
+{
+ BufferDesc *bufHdr;
+ BufferTag tag;
+ Oid relid;
+
+ if (mode != LW_EXCLUSIVE)
+ return;
+
+ if (!((BufferDescPadded *) lock > BufferDescriptors &&
+ (BufferDescPadded *) lock < BufferDescriptors + NBuffers))
+ return; /* not a buffer lock */
+
+ bufHdr = (BufferDesc *)
+ ((char *) lock - offsetof(BufferDesc, content_lock));
+ tag = bufHdr->tag;
+
+ /*
+ * This relNumber==relid assumption holds until a catalog experiences
+ * VACUUM FULL or similar. After a command like that, relNumber will be
+ * in the normal (non-catalog) range, and we lose the ability to detect
+ * hazardous access to that catalog. Calling RelidByRelfilenumber()
would
+ * close that gap, but RelidByRelfilenumber() might then deadlock with a
+ * held lock.
+ */
+ relid = tag.relNumber;
+
+ if (IsCatalogTextUniqueIndexOid(relid)) /* see comments at the callee */
+ return;
+
+ Assert(!IsCatalogRelationOid(relid));
+ /* Shared rels are always catalogs: detect even after VACUUM FULL. */
+ Assert(tag.spcOid != GLOBALTABLESPACE_OID);
+}
+#endif
+
+
/*
* Helper routine to issue warnings when a buffer is unexpectedly pinned
*/
diff --git a/src/backend/storage/lmgr/lwlock.c
b/src/backend/storage/lmgr/lwlock.c
index dc4d96c..f63c1de 100644
--- a/src/backend/storage/lmgr/lwlock.c
+++ b/src/backend/storage/lmgr/lwlock.c
@@ -1962,6 +1962,21 @@ LWLockReleaseAll(void)
/*
+ * ForEachLWLockHeldByMe - run a callback for each held lock
+ *
+ * This is meant as debug support only.
+ */
+void
+ForEachLWLockHeldByMe(void (*callback) (LWLock *, LWLockMode, void *),
+ void *context)
+{
+ int i;
+
+ for (i = 0; i < num_held_lwlocks; i++)
+ callback(held_lwlocks[i].lock, held_lwlocks[i].mode, context);
+}
+
+/*
* LWLockHeldByMe - test whether my process holds a lock in any mode
*
* This is meant as debug support only.
diff --git a/src/backend/utils/adt/pg_locale.c
b/src/backend/utils/adt/pg_locale.c
index 6e43b70..a73aac4 100644
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -1196,6 +1196,8 @@ pg_newlocale_from_collation(Oid collid)
if (!OidIsValid(collid))
elog(ERROR, "cache lookup failed for collation %u", collid);
+ AssertCouldGetRelation();
+
if (last_collation_cache_oid == collid)
return last_collation_cache_locale;
diff --git a/src/backend/utils/cache/catcache.c
b/src/backend/utils/cache/catcache.c
index 9ad7681..9a33ad3 100644
--- a/src/backend/utils/cache/catcache.c
+++ b/src/backend/utils/cache/catcache.c
@@ -1054,6 +1054,32 @@ RehashCatCacheLists(CatCache *cp)
cp->cc_lbucket = newbucket;
}
+pg_attribute_always_inline
+static void
+ConditionalCatalogCacheInitializeCache(CatCache *cache)
+{
+#ifdef USE_ASSERT_CHECKING
+ /*
+ * TypeCacheRelCallback() runs outside transactions and relies on
TYPEOID
+ * for hashing. This isn't ideal. Since lookup_type_cache() both
+ * registers the callback and searches TYPEOID, reaching trouble likely
+ * requires OOM at an unlucky moment.
+ *
+ * InvalidateAttoptCacheCallback() runs outside transactions and
likewise
+ * relies on ATTNUM. InitPostgres() initializes ATTNUM, so it's
reliable.
+ */
+ if (!(cache->id == TYPEOID || cache->id == ATTNUM) ||
+ IsTransactionState())
+ AssertCouldGetRelation();
+ else
+ Assert(cache->cc_tupdesc != NULL);
+#endif
+
+ if (unlikely(cache->cc_tupdesc == NULL))
+ CatalogCacheInitializeCache(cache);
+}
+
+
/*
* CatalogCacheInitializeCache
*
@@ -1194,8 +1220,7 @@ CatalogCacheInitializeCache(CatCache *cache)
void
InitCatCachePhase2(CatCache *cache, bool touch_index)
{
- if (cache->cc_tupdesc == NULL)
- CatalogCacheInitializeCache(cache);
+ ConditionalCatalogCacheInitializeCache(cache);
if (touch_index &&
cache->id != AMOID &&
@@ -1374,16 +1399,12 @@ SearchCatCacheInternal(CatCache *cache,
dlist_head *bucket;
CatCTup *ct;
- /* Make sure we're in an xact, even if this ends up being a cache hit */
- Assert(IsTransactionState());
-
Assert(cache->cc_nkeys == nkeys);
/*
* one-time startup overhead for each cache
*/
- if (unlikely(cache->cc_tupdesc == NULL))
- CatalogCacheInitializeCache(cache);
+ ConditionalCatalogCacheInitializeCache(cache);
#ifdef CATCACHE_STATS
cache->cc_searches++;
@@ -1668,8 +1689,7 @@ GetCatCacheHashValue(CatCache *cache,
/*
* one-time startup overhead for each cache
*/
- if (cache->cc_tupdesc == NULL)
- CatalogCacheInitializeCache(cache);
+ ConditionalCatalogCacheInitializeCache(cache);
/*
* calculate the hash value
@@ -1720,8 +1740,7 @@ SearchCatCacheList(CatCache *cache,
/*
* one-time startup overhead for each cache
*/
- if (unlikely(cache->cc_tupdesc == NULL))
- CatalogCacheInitializeCache(cache);
+ ConditionalCatalogCacheInitializeCache(cache);
Assert(nkeys > 0 && nkeys < cache->cc_nkeys);
@@ -2390,8 +2409,7 @@ PrepareToInvalidateCacheTuple(Relation relation,
continue;
/* Just in case cache hasn't finished initialization yet... */
- if (ccp->cc_tupdesc == NULL)
- CatalogCacheInitializeCache(ccp);
+ ConditionalCatalogCacheInitializeCache(ccp);
hashvalue = CatalogCacheComputeTupleHashValue(ccp,
ccp->cc_nkeys, tuple);
dbid = ccp->cc_relisshared ? (Oid) 0 : MyDatabaseId;
diff --git a/src/backend/utils/cache/inval.c b/src/backend/utils/cache/inval.c
index 4eb6772..96a2e1f 100644
--- a/src/backend/utils/cache/inval.c
+++ b/src/backend/utils/cache/inval.c
@@ -683,7 +683,8 @@ PrepareInvalidationState(void)
{
TransInvalidationInfo *myInfo;
- Assert(IsTransactionState());
+ /* PrepareToInvalidateCacheTuple() needs relcache */
+ AssertCouldGetRelation();
/* Can't queue transactional message while collecting inplace messages.
*/
Assert(inplaceInvalInfo == NULL);
@@ -752,7 +753,7 @@ PrepareInplaceInvalidationState(void)
{
InvalidationInfo *myInfo;
- Assert(IsTransactionState());
+ AssertCouldGetRelation();
/* limit of one inplace update under assembly */
Assert(inplaceInvalInfo == NULL);
@@ -928,6 +929,12 @@ InvalidateSystemCaches(void)
void
AcceptInvalidationMessages(void)
{
+#ifdef USE_ASSERT_CHECKING
+ /* message handlers shall access catalogs only during transactions */
+ if (IsTransactionState())
+ AssertCouldGetRelation();
+#endif
+
ReceiveSharedInvalidMessages(LocalExecuteInvalidationMessage,
InvalidateSystemCaches);
@@ -1436,6 +1443,9 @@ CacheInvalidateHeapTupleCommon(Relation relation,
Oid databaseId;
Oid relationId;
+ /* PrepareToInvalidateCacheTuple() needs relcache */
+ AssertCouldGetRelation();
+
/* Do nothing during bootstrap */
if (IsBootstrapProcessingMode())
return;
diff --git a/src/backend/utils/cache/relcache.c
b/src/backend/utils/cache/relcache.c
index 2905ae8..68ff67d 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -2056,6 +2056,23 @@ formrdesc(const char *relationName, Oid relationReltype,
relation->rd_isvalid = true;
}
+#ifdef USE_ASSERT_CHECKING
+/*
+ * AssertCouldGetRelation
+ *
+ * Check safety of calling RelationIdGetRelation().
+ *
+ * In code that reads catalogs in the event of a cache miss, call
this
+ * before checking the cache.
+ */
+void
+AssertCouldGetRelation(void)
+{
+ Assert(IsTransactionState());
+ AssertBufferLocksPermitCatalogRead();
+}
+#endif
+
/* ----------------------------------------------------------------
* Relation Descriptor Lookup Interface
@@ -2083,8 +2100,7 @@ RelationIdGetRelation(Oid relationId)
{
Relation rd;
- /* Make sure we're in an xact, even if this ends up being a cache hit */
- Assert(IsTransactionState());
+ AssertCouldGetRelation();
/*
* first try to find reldesc in the cache
@@ -2373,8 +2389,7 @@ RelationReloadNailed(Relation relation)
Assert(relation->rd_isnailed);
/* nailed indexes are handled by RelationReloadIndexInfo() */
Assert(relation->rd_rel->relkind == RELKIND_RELATION);
- /* can only reread catalog contents in a transaction */
- Assert(IsTransactionState());
+ AssertCouldGetRelation();
/*
* Redo RelationInitPhysicalAddr in case it is a mapped relation whose
@@ -2570,8 +2585,7 @@ static void
RelationRebuildRelation(Relation relation)
{
Assert(!RelationHasReferenceCountZero(relation));
- /* rebuilding requires access to the catalogs */
- Assert(IsTransactionState());
+ AssertCouldGetRelation();
/* there is no reason to ever rebuild a dropped relation */
Assert(relation->rd_droppedSubid == InvalidSubTransactionId);
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index 885dc11..5ddba5b 100644
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -310,7 +310,7 @@ InitializeClientEncoding(void)
{
Oid utf8_to_server_proc;
- Assert(IsTransactionState());
+ AssertCouldGetRelation();
utf8_to_server_proc =
FindDefaultConversionProc(PG_UTF8,
current_server_encoding);
diff --git a/src/include/catalog/catalog.h b/src/include/catalog/catalog.h
index b3a3acf..9b5e750 100644
--- a/src/include/catalog/catalog.h
+++ b/src/include/catalog/catalog.h
@@ -27,6 +27,7 @@ extern bool IsSystemClass(Oid relid, Form_pg_class reltuple);
extern bool IsToastClass(Form_pg_class reltuple);
extern bool IsCatalogRelationOid(Oid relid);
+extern bool IsCatalogTextUniqueIndexOid(Oid relid);
extern bool IsInplaceUpdateOid(Oid relid);
extern bool IsCatalogNamespace(Oid namespaceId);
diff --git a/src/include/storage/bufmgr.h b/src/include/storage/bufmgr.h
index 96150a6..b8dfa08 100644
--- a/src/include/storage/bufmgr.h
+++ b/src/include/storage/bufmgr.h
@@ -258,6 +258,9 @@ extern Buffer ExtendBufferedRelTo(BufferManagerRelation bmr,
extern void InitBufferManagerAccess(void);
extern void AtEOXact_Buffers(bool isCommit);
+#ifdef USE_ASSERT_CHECKING
+extern void AssertBufferLocksPermitCatalogRead(void);
+#endif
extern char *DebugPrintBufferRefcount(Buffer buffer);
extern void CheckPointBuffers(int flags);
extern BlockNumber BufferGetBlockNumber(Buffer buffer);
diff --git a/src/include/storage/lwlock.h b/src/include/storage/lwlock.h
index d333f33..fb636ad 100644
--- a/src/include/storage/lwlock.h
+++ b/src/include/storage/lwlock.h
@@ -131,6 +131,8 @@ extern void LWLockReleaseClearVar(LWLock *lock,
pg_atomic_uint64 *valptr, uint64
extern void LWLockReleaseAll(void);
extern void LWLockDisown(LWLock *l);
extern void LWLockReleaseDisowned(LWLock *l, LWLockMode mode);
+extern void ForEachLWLockHeldByMe(void (*callback) (LWLock *, LWLockMode, void
*),
+ void
*context);
extern bool LWLockHeldByMe(LWLock *lock);
extern bool LWLockAnyHeldByMe(LWLock *lock, int nlocks, size_t stride);
extern bool LWLockHeldByMeInMode(LWLock *lock, LWLockMode mode);
diff --git a/src/include/utils/relcache.h b/src/include/utils/relcache.h
index a7c55db..3561c6b 100644
--- a/src/include/utils/relcache.h
+++ b/src/include/utils/relcache.h
@@ -37,6 +37,14 @@ typedef Relation *RelationPtr;
/*
* Routines to open (lookup) and close a relcache entry
*/
+#ifdef USE_ASSERT_CHECKING
+extern void AssertCouldGetRelation(void);
+#else
+static inline void
+AssertCouldGetRelation(void)
+{
+}
+#endif
extern Relation RelationIdGetRelation(Oid relationId);
extern void RelationClose(Relation relation);
diff --git a/src/test/regress/expected/type_sanity.out
b/src/test/regress/expected/type_sanity.out
index 8eff3d1..dd0c52a 100644
--- a/src/test/regress/expected/type_sanity.out
+++ b/src/test/regress/expected/type_sanity.out
@@ -11,6 +11,10 @@
-- that is OID or REGPROC fields that are not zero and do not match some
-- row in the linked-to table. However, if we want to enforce that a link
-- field can't be 0, we have to check it here.
+-- directory paths and dlsuffix are passed to us in environment variables
+\getenv libdir PG_LIBDIR
+\getenv dlsuffix PG_DLSUFFIX
+\set regresslib :libdir '/regress' :dlsuffix
-- **************** pg_type ****************
-- Look for illegal values in pg_type fields.
SELECT t1.oid, t1.typname
@@ -587,6 +591,21 @@ WHERE a1.atttypid = t1.oid AND
----------+---------+-----+---------
(0 rows)
+-- Look for IsCatalogTextUniqueIndexOid() omissions.
+CREATE FUNCTION is_catalog_text_unique_index_oid(oid) RETURNS bool
+ AS :'regresslib', 'is_catalog_text_unique_index_oid'
+ LANGUAGE C STRICT;
+SELECT indexrelid::regclass
+FROM pg_index
+WHERE (is_catalog_text_unique_index_oid(indexrelid) <>
+ (indisunique AND
+ indexrelid < 16384 AND
+ EXISTS (SELECT 1 FROM pg_attribute
+ WHERE attrelid = indexrelid AND atttypid = 'text'::regtype)));
+ indexrelid
+------------
+(0 rows)
+
-- **************** pg_range ****************
-- Look for illegal values in pg_range fields.
SELECT r.rngtypid, r.rngsubtype
diff --git a/src/test/regress/regress.c b/src/test/regress/regress.c
index 837fab6..3dbba06 100644
--- a/src/test/regress/regress.c
+++ b/src/test/regress/regress.c
@@ -21,6 +21,7 @@
#include "access/detoast.h"
#include "access/htup_details.h"
+#include "catalog/catalog.h"
#include "catalog/namespace.h"
#include "catalog/pg_operator.h"
#include "catalog/pg_type.h"
@@ -722,6 +723,13 @@ test_fdw_handler(PG_FUNCTION_ARGS)
PG_RETURN_NULL();
}
+PG_FUNCTION_INFO_V1(is_catalog_text_unique_index_oid);
+Datum
+is_catalog_text_unique_index_oid(PG_FUNCTION_ARGS)
+{
+ return IsCatalogTextUniqueIndexOid(PG_GETARG_OID(0));
+}
+
PG_FUNCTION_INFO_V1(test_support_func);
Datum
test_support_func(PG_FUNCTION_ARGS)
diff --git a/src/test/regress/sql/type_sanity.sql
b/src/test/regress/sql/type_sanity.sql
index 303f909..c94dd83 100644
--- a/src/test/regress/sql/type_sanity.sql
+++ b/src/test/regress/sql/type_sanity.sql
@@ -12,6 +12,12 @@
-- row in the linked-to table. However, if we want to enforce that a link
-- field can't be 0, we have to check it here.
+-- directory paths and dlsuffix are passed to us in environment variables
+\getenv libdir PG_LIBDIR
+\getenv dlsuffix PG_DLSUFFIX
+
+\set regresslib :libdir '/regress' :dlsuffix
+
-- **************** pg_type ****************
-- Look for illegal values in pg_type fields.
@@ -425,6 +431,20 @@ WHERE a1.atttypid = t1.oid AND
a1.attbyval != t1.typbyval OR
(a1.attstorage != t1.typstorage AND a1.attstorage != 'p'));
+-- Look for IsCatalogTextUniqueIndexOid() omissions.
+
+CREATE FUNCTION is_catalog_text_unique_index_oid(oid) RETURNS bool
+ AS :'regresslib', 'is_catalog_text_unique_index_oid'
+ LANGUAGE C STRICT;
+
+SELECT indexrelid::regclass
+FROM pg_index
+WHERE (is_catalog_text_unique_index_oid(indexrelid) <>
+ (indisunique AND
+ indexrelid < 16384 AND
+ EXISTS (SELECT 1 FROM pg_attribute
+ WHERE attrelid = indexrelid AND atttypid = 'text'::regtype)));
+
-- **************** pg_range ****************
-- Look for illegal values in pg_range fields.
