Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit da63fec

Browse files
committed
Add missing buffer lock acquisition in GetTupleForTrigger().
If we had not been holding buffer pin continuously since the tuple was initially fetched by the UPDATE or DELETE query, it would be possible for VACUUM or a page-prune operation to move the tuple while we're trying to copy it. This would result in a garbage "old" tuple value being passed to an AFTER ROW UPDATE or AFTER ROW DELETE trigger. The preconditions for this are somewhat improbable, and the timing constraints are very tight; so it's not so surprising that this hasn't been reported from the field, even though the bug has been there a long time. Problem found by Andres Freund. Back-patch to all active branches.
1 parent abece8a commit da63fec

File tree

1 file changed

+12
-0
lines changed

1 file changed

+12
-0
lines changed

src/backend/commands/trigger.c

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2662,6 +2662,16 @@ ltrmark:;
26622662

26632663
buffer = ReadBuffer(relation, ItemPointerGetBlockNumber(tid));
26642664

2665+
/*
2666+
* Although we already know this tuple is valid, we must lock the
2667+
* buffer to ensure that no one has a buffer cleanup lock; otherwise
2668+
* they might move the tuple while we try to copy it. But we can
2669+
* release the lock before actually doing the heap_copytuple call,
2670+
* since holding pin is sufficient to prevent anyone from getting a
2671+
* cleanup lock they don't already hold.
2672+
*/
2673+
LockBuffer(buffer, BUFFER_LOCK_SHARE);
2674+
26652675
page = BufferGetPage(buffer);
26662676
lp = PageGetItemId(page, ItemPointerGetOffsetNumber(tid));
26672677

@@ -2671,6 +2681,8 @@ ltrmark:;
26712681
tuple.t_len = ItemIdGetLength(lp);
26722682
tuple.t_self = *tid;
26732683
tuple.t_tableOid = RelationGetRelid(relation);
2684+
2685+
LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
26742686
}
26752687

26762688
result = heap_copytuple(&tuple);

0 commit comments

Comments
 (0)