Hi Peter,

Thanks for your feedback; Dmitry has followed-up with some additional questions.

On 1/20/20 8:05 PM, Peter Geoghegan wrote:
This is definitely not okay.

I suggest that you find a way to add assertions to code like
_bt_readpage() that verify that we do in fact have the buffer content
lock.

If you apply the attached patch on master it will fail the test suite; did you mean something else ?

Best regards,
 Jesper
diff --git a/src/backend/access/nbtree/nbtsearch.c 
b/src/backend/access/nbtree/nbtsearch.c
index 4189730f3a..57882f0b8d 100644
--- a/src/backend/access/nbtree/nbtsearch.c
+++ b/src/backend/access/nbtree/nbtsearch.c
@@ -1721,6 +1721,7 @@ _bt_readpage(IndexScanDesc scan, ScanDirection dir, 
OffsetNumber offnum)
         * used here; this function is what makes it good for currPos.
         */
        Assert(BufferIsValid(so->currPos.buf));
+       Assert(IsBufferPinnedAndLocked(so->currPos.buf));
 
        page = BufferGetPage(so->currPos.buf);
        opaque = (BTPageOpaque) PageGetSpecialPointer(page);
diff --git a/src/backend/storage/buffer/bufmgr.c 
b/src/backend/storage/buffer/bufmgr.c
index aba3960481..f29f40f9b6 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -3774,6 +3774,23 @@ HoldingBufferPinThatDelaysRecovery(void)
        return false;
 }
 
+/*
+ * Assert that the buffer is pinned and locked
+ */
+bool
+IsBufferPinnedAndLocked(Buffer buffer)
+{
+       BufferDesc *bufHdr;
+
+       Assert(BufferIsPinned(buffer));
+
+       bufHdr = GetBufferDescriptor(buffer - 1);
+
+       Assert(LWLockHeldByMe(BufferDescriptorGetContentLock(bufHdr)));
+
+       return true;
+}
+
 /*
  * ConditionalLockBufferForCleanup - as above, but don't wait to get the lock
  *
diff --git a/src/include/storage/bufmgr.h b/src/include/storage/bufmgr.h
index 73c7e9ba38..46a6aa6560 100644
--- a/src/include/storage/bufmgr.h
+++ b/src/include/storage/bufmgr.h
@@ -217,6 +217,7 @@ extern void LockBufferForCleanup(Buffer buffer);
 extern bool ConditionalLockBufferForCleanup(Buffer buffer);
 extern bool IsBufferCleanupOK(Buffer buffer);
 extern bool HoldingBufferPinThatDelaysRecovery(void);
+extern bool IsBufferPinnedAndLocked(Buffer buffer);
 
 extern void AbortBufferIO(void);
 

Reply via email to