mirror of
https://github.com/postgres/postgres.git
synced 2025-04-22 23:02:54 +03:00
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
This commit is contained in:
parent
8a0258c318
commit
d25fbf9f3e
@ -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));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
Loading…
x
Reference in New Issue
Block a user