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

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index eb811d751e5534cd83c691e28f2feb4f80b56a3c..75b214824dfa90dc68621f55b93eb1bd2c72f835 100644 (file)
--- 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;