diff options
| author | Jeff Davis <jdavis@postgresql.org> | 2022-09-22 10:58:49 -0700 |
|---|---|---|
| committer | Jeff Davis <jdavis@postgresql.org> | 2022-09-22 11:06:42 -0700 |
| commit | 410c422b75ac53a4da438c1a4b0e1fa421db0d59 (patch) | |
| tree | bee5f8f8df6fe46094a627bd53b1b7477095cfd2 /src | |
| parent | 5f9dda4c0664faf36fdf931f00e2206dad2d3066 (diff) | |
| download | postgresql-410c422b75ac53a4da438c1a4b0e1fa421db0d59.tar.gz | |
Fix race condition where heap_delete() fails to pin VM page.
Similar to 5f12bc94dc, the code must re-check PageIsAllVisible() after
buffer lock is re-acquired. Backpatching to the same version, 12.
Discussion: https://postgr.es/m/CAEP4nAw9jYQDKd_5Y+-s2E4YiUJq1vqiikFjYGpLShtp-K3gag@mail.gmail.com
Reported-by: Robins Tharakan
Reviewed-by: Robins Tharakan
Backpatch-through: 12
Diffstat (limited to 'src')
| -rw-r--r-- | src/backend/access/heap/heapam.c | 30 |
1 files changed, 19 insertions, 11 deletions
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 22babd9b84..0a3ebecd22 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -2500,6 +2500,15 @@ heap_delete(Relation relation, ItemPointer tid, LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE); + lp = PageGetItemId(page, ItemPointerGetOffsetNumber(tid)); + Assert(ItemIdIsNormal(lp)); + + tp.t_tableOid = RelationGetRelid(relation); + tp.t_data = (HeapTupleHeader) PageGetItem(page, lp); + tp.t_len = ItemIdGetLength(lp); + tp.t_self = *tid; + +l1: /* * If we didn't pin the visibility map page and the page has become all * visible while we were busy locking the buffer, we'll have to unlock and @@ -2513,15 +2522,6 @@ heap_delete(Relation relation, ItemPointer tid, LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE); } - lp = PageGetItemId(page, ItemPointerGetOffsetNumber(tid)); - Assert(ItemIdIsNormal(lp)); - - tp.t_tableOid = RelationGetRelid(relation); - tp.t_data = (HeapTupleHeader) PageGetItem(page, lp); - tp.t_len = ItemIdGetLength(lp); - tp.t_self = *tid; - -l1: result = HeapTupleSatisfiesUpdate(&tp, cid, buffer); if (result == TM_Invisible) @@ -2580,8 +2580,12 @@ l1: * If xwait had just locked the tuple then some other xact * could update this tuple before we get to this point. Check * for xmax change, and start over if so. + * + * We also must start over if we didn't pin the VM page, and + * the page has become all visible. */ - if (xmax_infomask_changed(tp.t_data->t_infomask, infomask) || + if ((vmbuffer == InvalidBuffer && PageIsAllVisible(page)) || + xmax_infomask_changed(tp.t_data->t_infomask, infomask) || !TransactionIdEquals(HeapTupleHeaderGetRawXmax(tp.t_data), xwait)) goto l1; @@ -2613,8 +2617,12 @@ l1: * xwait is done, but if xwait had just locked the tuple then some * other xact could update this tuple before we get to this point. * Check for xmax change, and start over if so. + * + * We also must start over if we didn't pin the VM page, and the + * page has become all visible. */ - if (xmax_infomask_changed(tp.t_data->t_infomask, infomask) || + if ((vmbuffer == InvalidBuffer && PageIsAllVisible(page)) || + xmax_infomask_changed(tp.t_data->t_infomask, infomask) || !TransactionIdEquals(HeapTupleHeaderGetRawXmax(tp.t_data), xwait)) goto l1; |