Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitb04aeb0

Browse files
committed
Add assertions that we hold some relevant lock during relation open.
Opening a relation with no lock at all is unsafe; there's no guaranteethat we'll see a consistent state of the relevant catalog entries.While use of MVCC scans to read the catalogs partially addresses thatcomplaint, it's still possible to switch to a new catalog snapshotpartway through loading the relcache entry. Moreover, whether or notyou trust the reasoning behind sometimes using less thanAccessExclusiveLock for ALTER TABLE, that reasoning is certainly notvalid if concurrent users of the table don't hold a lock correspondingto the operation they want to perform.Hence, add some assertion-build-only checks that require any callerof relation_open(x, NoLock) to hold at least AccessShareLock. Thisisn't a full solution, since we can't verify that the lock level issemantically appropriate for the action --- but it's definitely ofsome 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 LaneDiscussion:https://postgr.es/m/16565.1538327894@sss.pgh.pa.us
1 parentb66827c commitb04aeb0

File tree

7 files changed

+95
-3
lines changed

7 files changed

+95
-3
lines changed

‎src/backend/access/heap/heapam.c

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1137,6 +1137,14 @@ relation_open(Oid relationId, LOCKMODE lockmode)
11371137
if (!RelationIsValid(r))
11381138
elog(ERROR,"could not open relation with OID %u",relationId);
11391139

1140+
/*
1141+
* If we didn't get the lock ourselves, assert that caller holds one,
1142+
* except in bootstrap mode where no locks are used.
1143+
*/
1144+
Assert(lockmode!=NoLock||
1145+
IsBootstrapProcessingMode()||
1146+
CheckRelationLockedByMe(r,AccessShareLock, true));
1147+
11401148
/* Make note that we've accessed a temporary relation */
11411149
if (RelationUsesLocalBuffers(r))
11421150
MyXactFlags |=XACT_FLAGS_ACCESSEDTEMPREL;
@@ -1183,6 +1191,10 @@ try_relation_open(Oid relationId, LOCKMODE lockmode)
11831191
if (!RelationIsValid(r))
11841192
elog(ERROR,"could not open relation with OID %u",relationId);
11851193

1194+
/* If we didn't get the lock ourselves, assert that caller holds one */
1195+
Assert(lockmode!=NoLock||
1196+
CheckRelationLockedByMe(r,AccessShareLock, true));
1197+
11861198
/* Make note that we've accessed a temporary relation */
11871199
if (RelationUsesLocalBuffers(r))
11881200
MyXactFlags |=XACT_FLAGS_ACCESSEDTEMPREL;
@@ -8084,6 +8096,8 @@ ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_changed, bool *
80848096

80858097
idx_rel=RelationIdGetRelation(replidindex);
80868098

8099+
Assert(CheckRelationLockedByMe(idx_rel,AccessShareLock, true));
8100+
80878101
/* deform tuple, so we have fast access to columns */
80888102
heap_deform_tuple(tp,desc,values,nulls);
80898103

‎src/backend/parser/parse_relation.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
#include"parser/parse_enr.h"
2929
#include"parser/parse_relation.h"
3030
#include"parser/parse_type.h"
31+
#include"storage/lmgr.h"
3132
#include"utils/builtins.h"
3233
#include"utils/lsyscache.h"
3334
#include"utils/rel.h"
@@ -1272,7 +1273,7 @@ addRangeTableEntry(ParseState *pstate,
12721273
*
12731274
* lockmode is the lock type required for query execution; it must be one
12741275
* of AccessShareLock, RowShareLock, or RowExclusiveLock depending on the
1275-
* RTE's role within the query. The callershould always hold that lock mode
1276+
* RTE's role within the query. The callermust hold that lock mode
12761277
* or a stronger one.
12771278
*
12781279
* Note: properly, lockmode should be declared LOCKMODE not int, but that
@@ -1295,6 +1296,7 @@ addRangeTableEntryForRelation(ParseState *pstate,
12951296
Assert(lockmode==AccessShareLock||
12961297
lockmode==RowShareLock||
12971298
lockmode==RowExclusiveLock);
1299+
Assert(CheckRelationLockedByMe(rel,lockmode, true));
12981300

12991301
rte->rtekind=RTE_RELATION;
13001302
rte->alias=alias;

‎src/backend/storage/lmgr/lmgr.c

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,51 @@ UnlockRelation(Relation relation, LOCKMODE lockmode)
287287
LockRelease(&tag,lockmode, false);
288288
}
289289

290+
/*
291+
*CheckRelationLockedByMe
292+
*
293+
* Returns true if current transaction holds a lock on 'relation' of mode
294+
* 'lockmode'. If 'orstronger' is true, a stronger lockmode is also OK.
295+
* ("Stronger" is defined as "numerically higher", which is a bit
296+
* semantically dubious but is OK for the purposes we use this for.)
297+
*/
298+
bool
299+
CheckRelationLockedByMe(Relationrelation,LOCKMODElockmode,boolorstronger)
300+
{
301+
LOCKTAGtag;
302+
303+
SET_LOCKTAG_RELATION(tag,
304+
relation->rd_lockInfo.lockRelId.dbId,
305+
relation->rd_lockInfo.lockRelId.relId);
306+
307+
if (LockHeldByMe(&tag,lockmode))
308+
return true;
309+
310+
if (orstronger)
311+
{
312+
LOCKMODEslockmode;
313+
314+
for (slockmode=lockmode+1;
315+
slockmode <=MaxLockMode;
316+
slockmode++)
317+
{
318+
if (LockHeldByMe(&tag,slockmode))
319+
{
320+
#ifdefNOT_USED
321+
/* Sometimes this might be useful for debugging purposes */
322+
elog(WARNING,"lock mode %s substituted for %s on relation %s",
323+
GetLockmodeName(tag.locktag_lockmethodid,slockmode),
324+
GetLockmodeName(tag.locktag_lockmethodid,lockmode),
325+
RelationGetRelationName(relation));
326+
#endif
327+
return true;
328+
}
329+
}
330+
}
331+
332+
return false;
333+
}
334+
290335
/*
291336
*LockHasWaitersRelation
292337
*

‎src/backend/storage/lmgr/lock.c

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -563,6 +563,30 @@ DoLockModesConflict(LOCKMODE mode1, LOCKMODE mode2)
563563
return false;
564564
}
565565

566+
/*
567+
* LockHeldByMe -- test whether lock 'locktag' is held with mode 'lockmode'
568+
*by the current transaction
569+
*/
570+
bool
571+
LockHeldByMe(constLOCKTAG*locktag,LOCKMODElockmode)
572+
{
573+
LOCALLOCKTAGlocaltag;
574+
LOCALLOCK*locallock;
575+
576+
/*
577+
* See if there is a LOCALLOCK entry for this lock and lockmode
578+
*/
579+
MemSet(&localtag,0,sizeof(localtag));/* must clear padding */
580+
localtag.lock=*locktag;
581+
localtag.mode=lockmode;
582+
583+
locallock= (LOCALLOCK*)hash_search(LockMethodLocalHash,
584+
(void*)&localtag,
585+
HASH_FIND,NULL);
586+
587+
return (locallock&&locallock->nLocks>0);
588+
}
589+
566590
/*
567591
* LockHasWaiters -- look up 'locktag' and check if releasing this
568592
*lock would wake up other processes waiting for it.

‎src/include/storage/lmgr.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ extern void UnlockRelationOid(Oid relid, LOCKMODE lockmode);
4545
externvoidLockRelation(Relationrelation,LOCKMODElockmode);
4646
externboolConditionalLockRelation(Relationrelation,LOCKMODElockmode);
4747
externvoidUnlockRelation(Relationrelation,LOCKMODElockmode);
48+
externboolCheckRelationLockedByMe(Relationrelation,LOCKMODElockmode,
49+
boolorstronger);
4850
externboolLockHasWaitersRelation(Relationrelation,LOCKMODElockmode);
4951

5052
externvoidLockRelationIdForSession(LockRelId*relid,LOCKMODElockmode);

‎src/include/storage/lock.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -540,6 +540,7 @@ extern void LockReleaseAll(LOCKMETHODID lockmethodid, bool allLocks);
540540
externvoidLockReleaseSession(LOCKMETHODIDlockmethodid);
541541
externvoidLockReleaseCurrentOwner(LOCALLOCK**locallocks,intnlocks);
542542
externvoidLockReassignCurrentOwner(LOCALLOCK**locallocks,intnlocks);
543+
externboolLockHeldByMe(constLOCKTAG*locktag,LOCKMODElockmode);
543544
externboolLockHasWaiters(constLOCKTAG*locktag,
544545
LOCKMODElockmode,boolsessionLock);
545546
externVirtualTransactionId*GetLockConflicts(constLOCKTAG*locktag,

‎src/include/storage/lockdefs.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,15 @@ typedef int LOCKMODE;
4545
#defineAccessExclusiveLock8/* ALTER TABLE, DROP TABLE, VACUUM FULL,
4646
* and unqualified LOCK TABLE */
4747

48+
#defineMaxLockMode8
49+
50+
51+
/* WAL representation of an AccessExclusiveLock on a table */
4852
typedefstructxl_standby_lock
4953
{
5054
TransactionIdxid;/* xid of holder of AccessExclusiveLock */
51-
OiddbOid;
52-
OidrelOid;
55+
OiddbOid;/* DB containing table */
56+
OidrelOid;/* OID of table */
5357
}xl_standby_lock;
5458

5559
#endif/* LOCKDEF_H_ */

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp