Fix two off-by-one errors in bufmgr.c.

In 4b4b680c I passed a buffer index number (starting from 0) instead of
a proper Buffer id (which start from 1 for shared buffers) in two
places.

This wasn't noticed so far as one of those locations isn't compiled at
all (PrintPinnedBufs) and the other one (InvalidBuffer) requires a
unlikely, but possible, set of circumstances to trigger a symptom.

To reduce the likelihood of such incidents a bit also convert existing
open coded mappings from buffer descriptors to buffer ids with
BufferDescriptorGetBuffer().

Author: Qingqing Zhou
Reported-By: Qingqing Zhou
Discussion: CAJjS0u2ai9ooUisKtkV8cuVUtEkMTsbK8c7juNAjv8K11zeCQg@mail.gmail.com
Backpatch: 9.5 where the private ref count infrastructure was introduced

diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index e4b25587e98ab0620a15abab0de49957e8743624..cd3aaad610646ef172a326129f283fd02e5a0d33 100644 (file)
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -1273,7 +1273,7 @@ retry:
                UnlockBufHdr(buf);
                LWLockRelease(oldPartitionLock);
                /* safety check: should definitely not be our *own* pin */
-               if (GetPrivateRefCount(buf->buf_id) > 0)
+               if (GetPrivateRefCount(BufferDescriptorGetBuffer(buf)) > 0)
                        elog(ERROR, "buffer is pinned in InvalidateBuffer");
                WaitIO(buf);
                goto retry;
@@ -1426,16 +1426,16 @@ ReleaseAndReadBuffer(Buffer buffer,
 static bool
 PinBuffer(volatile BufferDesc *buf, BufferAccessStrategy strategy)
 {
-       int                     b = buf->buf_id;
+       Buffer          b = BufferDescriptorGetBuffer(buf);
        bool            result;
        PrivateRefCountEntry *ref;
 
-       ref = GetPrivateRefCountEntry(b + 1, true);
+       ref = GetPrivateRefCountEntry(b, true);
 
        if (ref == NULL)
        {
                ReservePrivateRefCountEntry();
-               ref = NewPrivateRefCountEntry(b + 1);
+               ref = NewPrivateRefCountEntry(b);
 
                LockBufHdr(buf);
                buf->refcount++;
@@ -1460,8 +1460,7 @@ PinBuffer(volatile BufferDesc *buf, BufferAccessStrategy strategy)
 
        ref->refcount++;
        Assert(ref->refcount > 0);
-       ResourceOwnerRememberBuffer(CurrentResourceOwner,
-                                                               BufferDescriptorGetBuffer(buf));
+       ResourceOwnerRememberBuffer(CurrentResourceOwner, b);
        return result;
 }
 
@@ -1489,23 +1488,24 @@ PinBuffer(volatile BufferDesc *buf, BufferAccessStrategy strategy)
 static void
 PinBuffer_Locked(volatile BufferDesc *buf)
 {
-       int                     b = buf->buf_id;
+       Buffer          b;
        PrivateRefCountEntry *ref;
 
        /*
         * As explained, We don't expect any preexisting pins. That allows us to
         * manipulate the PrivateRefCount after releasing the spinlock
         */
-       Assert(GetPrivateRefCountEntry(b + 1, false) == NULL);
+       Assert(GetPrivateRefCountEntry(BufferDescriptorGetBuffer(buf), false) == NULL);
 
        buf->refcount++;
        UnlockBufHdr(buf);
 
-       ref = NewPrivateRefCountEntry(b + 1);
+       b = BufferDescriptorGetBuffer(buf);
+
+       ref = NewPrivateRefCountEntry(b);
        ref->refcount++;
 
-       ResourceOwnerRememberBuffer(CurrentResourceOwner,
-                                                               BufferDescriptorGetBuffer(buf));
+       ResourceOwnerRememberBuffer(CurrentResourceOwner, b);
 }
 
 /*
@@ -1520,14 +1520,14 @@ static void
 UnpinBuffer(volatile BufferDesc *buf, bool fixOwner)
 {
        PrivateRefCountEntry *ref;
+       Buffer          b = BufferDescriptorGetBuffer(buf);
 
        /* not moving as we're likely deleting it soon anyway */
-       ref = GetPrivateRefCountEntry(buf->buf_id + 1, false);
+       ref = GetPrivateRefCountEntry(b, false);
        Assert(ref != NULL);
 
        if (fixOwner)
-               ResourceOwnerForgetBuffer(CurrentResourceOwner,
-                                                                 BufferDescriptorGetBuffer(buf));
+               ResourceOwnerForgetBuffer(CurrentResourceOwner, b);
 
        Assert(ref->refcount > 0);
        ref->refcount--;
@@ -2739,6 +2739,7 @@ PrintBufferDescs(void)
        for (i = 0; i < NBuffers; ++i)
        {
                volatile BufferDesc *buf = GetBufferDescriptor(i);
+               Buffer          b = BufferDescriptorGetBuffer(buf);
 
                /* theoretically we should lock the bufhdr here */
                elog(LOG,
@@ -2747,7 +2748,7 @@ PrintBufferDescs(void)
                         i, buf->freeNext,
                  relpathbackend(buf->tag.rnode, InvalidBackendId, buf->tag.forkNum),
                         buf->tag.blockNum, buf->flags,
-                        buf->refcount, GetPrivateRefCount(i));
+                        buf->refcount, GetPrivateRefCount(b));
        }
 }
 #endif
@@ -2761,8 +2762,9 @@ PrintPinnedBufs(void)
        for (i = 0; i < NBuffers; ++i)
        {
                volatile BufferDesc *buf = GetBufferDescriptor(i);
+               Buffer          b = BufferDescriptorGetBuffer(buf);
 
-               if (GetPrivateRefCount(i + 1) > 0)
+               if (GetPrivateRefCount(b) > 0)
                {
                        /* theoretically we should lock the bufhdr here */
                        elog(LOG,
@@ -2771,7 +2773,7 @@ PrintPinnedBufs(void)
                                 i, buf->freeNext,
                                 relpathperm(buf->tag.rnode, buf->tag.forkNum),
                                 buf->tag.blockNum, buf->flags,
-                                buf->refcount, GetPrivateRefCount(i + 1));
+                                buf->refcount, GetPrivateRefCount(b));
                }
        }
 }