Re: heap_lock_updated_tuple_rec can leak a buffer refcount

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Subject: Re: heap_lock_updated_tuple_rec can leak a buffer refcount
Date: 2018-02-20 12:39:41
Message-ID: CAA4eK1JWcdot-vg580G-SVO7vvkVK9xSZAN8fsXoD-zgcrapjw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Feb 13, 2018 at 10:11 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> It seems to me that heap_lock_updated_tuple_rec can lead to a buffer
> refcount leak while locking an updated tuple by an aborted
> transaction. In commit - 5c609a74, we have added the code to deal
> with aborted transactions as below:
>
> heap_lock_updated_tuple_rec()
> {
> ..
>
> if (PageIsAllVisible(BufferGetPage(buf)))
> visibilitymap_pin(rel, block, &vmbuffer);
> else
> vmbuffer = InvalidBuffer;
>
> LockBuffer(buf, BUFFER_LOCK_EXCLUSIVE);
> ..
> -------------------------- below code is added by commit -5c609a74 -----------
> if (TransactionIdDidAbort(HeapTupleHeaderGetXmin(mytup.t_data)))
> {
> UnlockReleaseBuffer(buf);
> return HeapTupleMayBeUpdated;
> }
> -------------------------------------------------------------
>
> I think the above code forgets to deal with vmbuffer and can lead to a
> leak of the same. Attached patch ensures that it deals with vmbuffer
> when required.
>

Registered the patch for next CF:
https://commitfest.postgresql.org/17/1531/

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Petr Jelinek 2018-02-20 13:03:58 Re: ALTER TABLE ADD COLUMN fast default
Previous Message Amit Kapila 2018-02-20 11:43:03 Re: [HACKERS] why not parallel seq scan for slow functions