Odd code around ginScanToDelete - Mailing list pgsql-hackers

Hi,

While looking at converting more places to UnlockReleaseBuffer(), in the
course of making UnlockReleaseBuffer() faster than the two separate
operations, I found this code:

static bool
ginScanToDelete(GinVacuumState *gvs, BlockNumber blkno, bool isRoot,
                DataPageDeleteStack *parent, OffsetNumber myoff)
...

    if (!meDelete)
    {
        if (BufferIsValid(me->leftBuffer))
            UnlockReleaseBuffer(me->leftBuffer);
        me->leftBuffer = buffer;
    }
    else
    {
        if (!isRoot)
            LockBuffer(buffer, GIN_UNLOCK);

        ReleaseBuffer(buffer);
    }

    if (isRoot)
        ReleaseBuffer(buffer);


Which sure looks like it'd release buffer twice if isRoot is set?  I guess
that's not reachable, because presumably the root page will always go down the
!meDelete path. But it sure made me wonder if there's a hard to reach bug.

This code was introduced in
  commit e14641197a5
  Author: Alexander Korotkov <akorotkov@postgresql.org>
  Date:   2019-11-19 23:07:36 +0300

      Fix deadlock between ginDeletePage() and ginStepRight()

I didn't trace it further to see if it existed before that in some fashion.



There's another oddity here: ginScanToDelete() requires that the root page has
been locked by the caller already, but will afaict re-read the root page? But
then have code to avoid locking it again, because that would not have worked?
Seems odd.


Greetings,

Andres Freund