database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-06-27 23:21:58 +03:00
Code Activity

Add assertions that we hold some relevant lock during relation open.

Browse Source 
Opening a relation with no lock at all is unsafe; there's no guarantee
that we'll see a consistent state of the relevant catalog entries.
While use of MVCC scans to read the catalogs partially addresses that
complaint, it's still possible to switch to a new catalog snapshot
partway through loading the relcache entry.  Moreover, whether or not
you trust the reasoning behind sometimes using less than
AccessExclusiveLock for ALTER TABLE, that reasoning is certainly not
valid if concurrent users of the table don't hold a lock corresponding
to the operation they want to perform.

Hence, add some assertion-build-only checks that require any caller
of relation_open(x, NoLock) to hold at least AccessShareLock.  This
isn't a full solution, since we can't verify that the lock level is
semantically appropriate for the action --- but it's definitely of
some use, because it's already caught two bugs.

We can also assert that callers of addRangeTableEntryForRelation()
hold at least the lock level specified for the new RTE.

Amit Langote and Tom Lane

Discussion: https://postgr.es/m/16565.1538327894@sss.pgh.pa.us
This commit is contained in:
Tom Lane
2018-10-01 12:43:21 -04:00
parent b66827ca7c
commit b04aeb0a05
7 changed files with 95 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
src/backend/access/heap/heapam.c
View File

@ -1137,6 +1137,14 @@ relation_open(Oid relationId, LOCKMODE lockmode)
if (!RelationIsValid(r))
elog(ERROR, "could not open relation with OID %u", relationId);
/*
* If we didn't get the lock ourselves, assert that caller holds one,
* except in bootstrap mode where no locks are used.
*/
Assert(lockmode != NoLock ||
IsBootstrapProcessingMode() ||
CheckRelationLockedByMe(r, AccessShareLock, true));
/* Make note that we've accessed a temporary relation */
if (RelationUsesLocalBuffers(r))
MyXactFlags |= XACT_FLAGS_ACCESSEDTEMPREL;
@ -1183,6 +1191,10 @@ try_relation_open(Oid relationId, LOCKMODE lockmode)
if (!RelationIsValid(r))
elog(ERROR, "could not open relation with OID %u", relationId);
/* If we didn't get the lock ourselves, assert that caller holds one */
Assert(lockmode != NoLock ||
CheckRelationLockedByMe(r, AccessShareLock, true));
/* Make note that we've accessed a temporary relation */
if (RelationUsesLocalBuffers(r))
MyXactFlags |= XACT_FLAGS_ACCESSEDTEMPREL;
@ -8084,6 +8096,8 @@ ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_changed, bool *
idx_rel = RelationIdGetRelation(replidindex);
Assert(CheckRelationLockedByMe(idx_rel, AccessShareLock, true));
/* deform tuple, so we have fast access to columns */
heap_deform_tuple(tp, desc, values, nulls);