Horton

inplace070-rel-locks-missing-v1.patch

text/plain

Download
Filename: inplace070-rel-locks-missing-v1.patch
Type: text/plain
Part: 5
Message: Re: race condition in pg_class

Patch

Same data as JSON: GET /api/v1/attachments/:id/patch the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes. API reference →
Format: unified
Series: patch v1
File+−
src/backend/catalog/heap.c 2 0
src/backend/catalog/index.c 1 0
src/backend/commands/indexcmds.c 4 0
src/backend/commands/sequence.c 1 0 
Author:     Noah Misch <noah@leadboat.com>
Commit:     Noah Misch <noah@leadboat.com>

    Add relation locks where existing standards would have them.
    
    The SetRelationHasSubclass() header comment says to hold this
    ShareUpdateExclusiveLock lock, and it's conventional for the sort of DDL
    done there and in IndexSetParentIndex().  SequenceChangePersistence()
    and heap_create() are deep surgery warranting AccessExclusiveLock.  This
    might cause more deadlocks.  I didn't run down exact user-visible bugs
    reachable via these gaps.  Back-patch to v12 (all supported versions).
    
    Reviewed by FIXME.
    
    Discussion: https://postgr.es/m/FIXME

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 922ba79..2652405 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -407,6 +407,8 @@ heap_create(const char *relname,
 	/* ensure that stats are dropped if transaction aborts */
 	pgstat_create_relation(rel);
 
+	LockRelationOid(relid, AccessExclusiveLock);
+
 	return rel;
 }
 
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 5a8568c..5c48e57 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -1058,6 +1058,7 @@ index_create(Relation heapRelation,
 	if (OidIsValid(parentIndexRelid))
 	{
 		StoreSingleInheritance(indexRelationId, parentIndexRelid, 1);
+		LockRelationOid(parentIndexRelid, ShareUpdateExclusiveLock);
 		SetRelationHasSubclass(parentIndexRelid, true);
 	}
 
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index d9016ef..a28ce72 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -4475,7 +4475,10 @@ IndexSetParentIndex(Relation partitionIdx, Oid parentOid)
 
 	/* set relhassubclass if an index partition has been added to the parent */
 	if (OidIsValid(parentOid))
+	{
+		LockRelationOid(parentOid, ShareUpdateExclusiveLock);
 		SetRelationHasSubclass(parentOid, true);
+	}
 
 	/* set relispartition correctly on the partition */
 	update_relispartition(partRelid, OidIsValid(parentOid));
@@ -4527,6 +4530,7 @@ update_relispartition(Oid relationId, bool newval)
 	HeapTuple	tup;
 	Relation	classRel;
 
+	LockRelationOid(relationId, ShareUpdateExclusiveLock);
 	classRel = table_open(RelationRelationId, RowExclusiveLock);
 	tup = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(relationId));
 	if (!HeapTupleIsValid(tup))
diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index 4610356..a60d6e7 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -545,6 +545,7 @@ SequenceChangePersistence(Oid relid, char newrelpersistence)
 	Buffer		buf;
 	HeapTupleData seqdatatuple;
 
+	LockRelationOid(relid, AccessExclusiveLock);
 	init_sequence(relid, &elm, &seqrel);
 
 	/* check the comment above nextval_internal()'s equivalent call. */