Re: Fix resource leak in FindConflictTuple() retry path

On Thu, Sep 3, 2026 at 9:22 AM Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> wrote:
>
> Hi,
>
> As part of the AI-assisted review of the update_deleted conflict
> detection work [1], a small resource leak was identified in
> FindConflictTuple(), introduced by commit 9758174e2e5.
>
> When should_refetch_tuple() returns true i.e. when the conflicting
> tuple was modified between ExecCheckIndexConstraints() and
> table_tuple_lock(), the function retries from its retry label. On each
> pass it creates a new slot and assigns it to *conflictslot,
> overwriting the previous slot without releasing it. So every retry
> abandons one slot.
>
> The abandoned slot also holds a buffer pin. Since FindConflictTuple()
> does not pass TUPLE_LOCK_FLAG_FIND_LAST_VERSION, heapam_tuple_lock()
> falls through to ExecStorePinnedBufferHeapTuple(), which transfers the
> pin to the slot even for TM_Updated. The heap_lock_tuple() failure
> path releases the content lock but not the pin. Since the slot has a
> NULL reglist, it is not registered in estate->es_tupleTable and is
> therefore not cleaned up by ExecResetTupleTable().
>
> This is mostly harmless in practice: the slot is freed with its memory
> context, and buffer pins are released at transaction end. The usual
> conflict path also aborts the transaction with ERROR, releasing
> everything.
>
> Still, a pinned buffer cannot be evicted, and repeated retries on a
> contended unique key can accumulate pins for the lifetime of the
> transaction, so this seems worth fixing.
>
> The attached patch-001 creates the slot once before the retry loop and
> reuses it. Re-storing a tuple in the same slot releases its previous
> buffer pin, so no slot is abandoned.
>
> Reproducing the issue:
> The window is very narrow and cannot be triggered directly from SQL,
> as the concurrent UPDATE must occur between
> ExecCheckIndexConstraints() and table_tuple_lock() inside
> FindConflictTuple().
>
> I created a small, hacky TAP test (patch-002) with the help of Claude,
> which uses an injection point and elog() to reproduce the issue and
> verify slot reuse. This test is only for demonstrating the problem and
> is not intended for commit.
>
> Since this is an oversight in commit 9758174e2e5, it should be
> backpatched to PG18.
>
> Feedback on the fix and approach is welcome.
>
> [1] https://www.postgresql.org/message-id/TY4PR01MB177182F547A62FC2666EC04EC94B72%40TY4PR01MB17718.jpnprd01.prod.outlook.com
>

I agree with the idea of patch. But the patch can be improved. Before
this patch, slot creation happened after the conflict was found. Now
it happens unconditionally. So every call to FindConflictTuple() now
allocates a slot and immediately tears it down. So most of the cases
which are ocnflict-free now will do slot-allocation. I feel this can
be optimized.

Suggestion:

retry:
if (ExecCheckIndexConstraints(...))
{
if (*conflictslot)
ExecDropSingleTupleTableSlot(*conflictslot);
*conflictslot = NULL;
return false;
}

if (*conflictslot == NULL)
*conflictslot = table_slot_create(rel, NULL);

thanks
Shveta