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

Commit1a917ae

Browse files
committed
Fix race when updating a tuple concurrently locked by another process
If a tuple is locked, and this lock is later upgraded either to anupdate or to a stronger lock, and in the meantime some other processtries to lock, update or delete the same tuple, it (the tuple) could endup being updated twice, or having conflicting locks held.The reason for this is that the second updater checks for a change inXmax value, or in the HEAP_XMAX_IS_MULTI infomask bit, after noticingthe first lock; and if there's a change, it restarts and re-evaluatesits ability to update the tuple. But it neglected to check for changesin lock strength or in lock-vs-update status when those two propertiesstayed the same. This would lead it to take the wrong decision andcontinue with its own update, when in reality it shouldn't do so butinstead restart from the top.This could lead to either an assertion failure much later (when amultixact containing multiple updates is detected), or duplicate copiesof tuples.To fix, make sure to compare the other relevant infomask bits alongsidethe Xmax value and HEAP_XMAX_IS_MULTI bit, and restart from the top ifnecessary.Also, in the belt-and-suspenders spirit, add a check toMultiXactCreateFromMembers that a multixact being created does not havetwo or more members that are claimed to be updates. This should protectagainst other bugs that might cause similar bogus situations.Backpatch to 9.3, where the possibility of multixacts containing updateswas introduced. (In prior versions it was possible to have the tuplelock upgraded from shared to exclusive, and an update would not restartfrom the top; yet we're protected against a bug there because there'salways a sleep to wait for the locking transaction to complete beforecontinuing to do anything. Really, the fact that tuple locks alwaysconflicted with concurrent updates is what protected against bugs here.)Per report from Andrew Dunstan and Josh Berkus in thread athttp://www.postgresql.org/message-id/534C8B33.9050807@pgexperts.comBug analysis by Andres Freund.
1 parentd19bd29 commit1a917ae

File tree

3 files changed

+51
-13
lines changed

3 files changed

+51
-13
lines changed

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

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -190,9 +190,6 @@ static const int MultiXactStatusLock[MaxMultiXactStatus + 1] =
190190
/* Get the LockTupleMode for a given MultiXactStatus */
191191
#defineTUPLOCK_from_mxstatus(status) \
192192
(MultiXactStatusLock[(status)])
193-
/* Get the is_update bit for a given MultiXactStatus */
194-
#defineISUPDATE_from_mxstatus(status) \
195-
((status) > MultiXactStatusForUpdate)
196193

197194
/* ----------------------------------------------------------------
198195
* heap support routines
@@ -2588,6 +2585,27 @@ compute_infobits(uint16 infomask, uint16 infomask2)
25882585
XLHL_KEYS_UPDATED :0);
25892586
}
25902587

2588+
/*
2589+
* Given two versions of the same t_infomask for a tuple, compare them and
2590+
* return whether the relevant status for a tuple Xmax has changed. This is
2591+
* used after a buffer lock has been released and reacquired: we want to ensure
2592+
* that the tuple state continues to be the same it was when we previously
2593+
* examined it.
2594+
*
2595+
* Note the Xmax field itself must be compared separately.
2596+
*/
2597+
staticinlinebool
2598+
xmax_infomask_changed(uint16new_infomask,uint16old_infomask)
2599+
{
2600+
constuint16interesting=
2601+
HEAP_XMAX_IS_MULTI |HEAP_XMAX_LOCK_ONLY |HEAP_LOCK_MASK;
2602+
2603+
if ((new_infomask&interesting)!= (old_infomask&interesting))
2604+
return true;
2605+
2606+
return false;
2607+
}
2608+
25912609
/*
25922610
*heap_delete - delete a tuple
25932611
*
@@ -2725,7 +2743,7 @@ heap_delete(Relation relation, ItemPointer tid,
27252743
* update this tuple before we get to this point. Check for xmax
27262744
* change, and start over if so.
27272745
*/
2728-
if (!(tp.t_data->t_infomask&HEAP_XMAX_IS_MULTI)||
2746+
if (xmax_infomask_changed(tp.t_data->t_infomask,infomask)||
27292747
!TransactionIdEquals(HeapTupleHeaderGetRawXmax(tp.t_data),
27302748
xwait))
27312749
gotol1;
@@ -2751,7 +2769,7 @@ heap_delete(Relation relation, ItemPointer tid,
27512769
* other xact could update this tuple before we get to this point.
27522770
* Check for xmax change, and start over if so.
27532771
*/
2754-
if ((tp.t_data->t_infomask&HEAP_XMAX_IS_MULTI)||
2772+
if (xmax_infomask_changed(tp.t_data->t_infomask,infomask)||
27552773
!TransactionIdEquals(HeapTupleHeaderGetRawXmax(tp.t_data),
27562774
xwait))
27572775
gotol1;
@@ -3278,7 +3296,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
32783296
* update this tuple before we get to this point. Check for xmax
32793297
* change, and start over if so.
32803298
*/
3281-
if (!(oldtup.t_data->t_infomask&HEAP_XMAX_IS_MULTI)||
3299+
if (xmax_infomask_changed(oldtup.t_data->t_infomask,infomask)||
32823300
!TransactionIdEquals(HeapTupleHeaderGetRawXmax(oldtup.t_data),
32833301
xwait))
32843302
gotol2;
@@ -3332,7 +3350,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
33323350
* recheck the locker; if someone else changed the tuple while
33333351
* we weren't looking, start over.
33343352
*/
3335-
if ((oldtup.t_data->t_infomask&HEAP_XMAX_IS_MULTI)||
3353+
if (xmax_infomask_changed(oldtup.t_data->t_infomask,infomask)||
33363354
!TransactionIdEquals(
33373355
HeapTupleHeaderGetRawXmax(oldtup.t_data),
33383356
xwait))
@@ -3353,7 +3371,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
33533371
* some other xact could update this tuple before we get to
33543372
* this point. Check for xmax change, and start over if so.
33553373
*/
3356-
if ((oldtup.t_data->t_infomask&HEAP_XMAX_IS_MULTI)||
3374+
if (xmax_infomask_changed(oldtup.t_data->t_infomask,infomask)||
33573375
!TransactionIdEquals(
33583376
HeapTupleHeaderGetRawXmax(oldtup.t_data),
33593377
xwait))
@@ -4357,7 +4375,7 @@ heap_lock_tuple(Relation relation, HeapTuple tuple,
43574375
* over.
43584376
*/
43594377
LockBuffer(*buffer,BUFFER_LOCK_EXCLUSIVE);
4360-
if (!(tuple->t_data->t_infomask&HEAP_XMAX_IS_MULTI)||
4378+
if (xmax_infomask_changed(tuple->t_data->t_infomask,infomask)||
43614379
!TransactionIdEquals(HeapTupleHeaderGetRawXmax(tuple->t_data),
43624380
xwait))
43634381
{
@@ -4376,7 +4394,7 @@ heap_lock_tuple(Relation relation, HeapTuple tuple,
43764394
LockBuffer(*buffer,BUFFER_LOCK_EXCLUSIVE);
43774395

43784396
/* if the xmax changed in the meantime, start over */
4379-
if ((tuple->t_data->t_infomask&HEAP_XMAX_IS_MULTI)||
4397+
if (xmax_infomask_changed(tuple->t_data->t_infomask,infomask)||
43804398
!TransactionIdEquals(
43814399
HeapTupleHeaderGetRawXmax(tuple->t_data),
43824400
xwait))
@@ -4444,7 +4462,7 @@ heap_lock_tuple(Relation relation, HeapTuple tuple,
44444462
* could update this tuple before we get to this point. Check
44454463
* for xmax change, and start over if so.
44464464
*/
4447-
if (!(tuple->t_data->t_infomask&HEAP_XMAX_IS_MULTI)||
4465+
if (xmax_infomask_changed(tuple->t_data->t_infomask,infomask)||
44484466
!TransactionIdEquals(
44494467
HeapTupleHeaderGetRawXmax(tuple->t_data),
44504468
xwait))
@@ -4500,7 +4518,7 @@ heap_lock_tuple(Relation relation, HeapTuple tuple,
45004518
* some other xact could update this tuple before we get to
45014519
* this point.Check for xmax change, and start over if so.
45024520
*/
4503-
if ((tuple->t_data->t_infomask&HEAP_XMAX_IS_MULTI)||
4521+
if (xmax_infomask_changed(tuple->t_data->t_infomask,infomask)||
45044522
!TransactionIdEquals(
45054523
HeapTupleHeaderGetRawXmax(tuple->t_data),
45064524
xwait))

‎src/backend/access/transam/multixact.c

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -457,7 +457,7 @@ MultiXactIdExpand(MultiXactId multi, TransactionId xid, MultiXactStatus status)
457457
for (i=0,j=0;i<nmembers;i++)
458458
{
459459
if (TransactionIdIsInProgress(members[i].xid)||
460-
((members[i].status>MultiXactStatusForUpdate)&&
460+
(ISUPDATE_from_mxstatus(members[i].status)&&
461461
TransactionIdDidCommit(members[i].xid)))
462462
{
463463
newMembers[j].xid=members[i].xid;
@@ -713,6 +713,22 @@ MultiXactIdCreateFromMembers(int nmembers, MultiXactMember *members)
713713
returnmulti;
714714
}
715715

716+
/* Verify that there is a single update Xid among the given members. */
717+
{
718+
inti;
719+
boolhas_update= false;
720+
721+
for (i=0;i<nmembers;i++)
722+
{
723+
if (ISUPDATE_from_mxstatus(members[i].status))
724+
{
725+
if (has_update)
726+
elog(ERROR,"new multixact has more than one updating member");
727+
has_update= true;
728+
}
729+
}
730+
}
731+
716732
/*
717733
* Assign the MXID and offsets range to use, and make sure there is space
718734
* in the OFFSETs and MEMBERs files. NB: this routine does

‎src/include/access/multixact.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,10 @@ typedef enum
4848

4949
#defineMaxMultiXactStatus MultiXactStatusUpdate
5050

51+
/* does a status value correspond to a tuple update? */
52+
#defineISUPDATE_from_mxstatus(status) \
53+
((status) > MultiXactStatusForUpdate)
54+
5155

5256
typedefstructMultiXactMember
5357
{

0 commit comments

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