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:04:00 -0700 |
| commit | 163b0993a162ebae00fe5de8f593a5da28821a1b (patch) | |
| tree | c3ae00157bf9fedf60f36e20357b5a32e5d86bc2 /src | |
| parent | 790bf615ddba8a0ba3b870915d03438ce9cb7860 (diff) | |
| download | postgresql-163b0993a162ebae00fe5de8f593a5da28821a1b.tar.gz postgresql-163b0993a162ebae00fe5de8f593a5da28821a1b.zip | |
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 eb811d751e5..75b214824df 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -2711,6 +2711,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 @@ -2724,15 +2733,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) @@ -2791,8 +2791,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; @@ -2824,8 +2828,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; |