Re: Possible duplicate release of buffer lock.

Поиск
Список
Период
Сортировка
От Tom Lane
Тема Re: Possible duplicate release of buffer lock.
Дата
Msg-id 16748.1470501932@sss.pgh.pa.us
обсуждение исходный текст
Ответ на Re: Possible duplicate release of buffer lock.  (Amit Kapila <amit.kapila16@gmail.com>)
Ответы Re: Possible duplicate release of buffer lock.  (Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>)
Список pgsql-hackers
Amit Kapila <amit.kapila16@gmail.com> writes:
> Isn't the problem here is that due to some reason (like concurrent
> split), the code in question (loop --
> while (P_ISDELETED(opaque) || opaque->btpo_next != target)) releases
> the lock on target buffer and the caller again tries to release the
> lock on target buffer when false is returned.

Yeah.  I do not think there is anything wrong with the loop-to-find-
current-leftsib logic.  The problem is lack of care about what
resources are held on failure return.  The sole caller thinks it
should do "_bt_relbuf(rel, buf)" --- that is, drop lock and pin on
what _bt_unlink_halfdead_page calls the leafbuf.  But that function
already dropped the lock (line 1582 in 9.4), and won't have reacquired
it unless target != leafblkno.  Another problem is that if the target
is different from leafblkno, we'll hold a pin on the target page, which
is leaked entirely in this code path.

The caller knows nothing of the "target" block so it can't reasonably
deal with all these cases.  I'm inclined to change the function's API
spec to say that on failure return, it's responsible for dropping
lock and pin on the passed buffer.  We could alternatively reacquire
lock before returning, but that seems pointless.

Another thing that looks fishy is at line 1611:
    leftsib = opaque->btpo_prev;

At this point we already released lock on leafbuf so it seems pretty
unsafe to fetch its left-link.  And it's unnecessary cause we already
did; the line should be just
    leftsib = leafleftsib;
        regards, tom lane



В списке pgsql-hackers по дате отправления:

Предыдущее
От: Bruce Momjian
Дата:
Сообщение: Re: No longer possible to query catalogs for index capabilities?
Следующее
От: Andrew Borodin
Дата:
Сообщение: Re: Re: GiST optimizing memmoves in gistplacetopage for fixed-size updates [PoC]