From ac0146edd7548ad6283b74c9c6b7c1847cca4b6f Mon Sep 17 00:00:00 2001
From: Asim R P <apraveen@pivotal.io>
Date: Wed, 8 Nov 2017 14:46:29 -0800
Subject: [PATCH 2/2] PageGetLSN: assert that locks are properly held

To help developers find issues, assert that the proper locks are held
for shared pages inside PageGetLSN. The implementation for this check is
added in a helper function, AssertPageIsLockedForLSN, which is called
from PageGetLSN whenever backend code is built with assertions enabled.
This introduces a slight source incompatibility: previously, both Pages
and PageHeaders could be passed to the macro, but now callers must
explicitly pass a Page.
---
 contrib/pageinspect/rawpage.c       |  2 +-
 src/backend/storage/buffer/bufmgr.c | 37 +++++++++++++++++++++++++++++++++++++
 src/include/storage/bufpage.h       | 17 +++++++++++++++--
 3 files changed, 53 insertions(+), 3 deletions(-)

diff --git a/contrib/pageinspect/rawpage.c b/contrib/pageinspect/rawpage.c
index 25af22f453..e3a1ca0e3b 100644
--- a/contrib/pageinspect/rawpage.c
+++ b/contrib/pageinspect/rawpage.c
@@ -250,7 +250,7 @@ page_header(PG_FUNCTION_ARGS)
 
 	/* Extract information from the page header */
 
-	lsn = PageGetLSN(page);
+	lsn = PageGetLSN((Page) page);
 
 	/* pageinspect >= 1.2 uses pg_lsn instead of text for the LSN field. */
 	if (TupleDescAttr(tupdesc, 0)->atttypid == TEXTOID)
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 15795b0c5a..df27936943 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -4355,3 +4355,40 @@ TestForOldSnapshot_impl(Snapshot snapshot, Relation relation)
 				(errcode(ERRCODE_SNAPSHOT_TOO_OLD),
 				 errmsg("snapshot too old")));
 }
+
+#if defined(USE_ASSERT_CHECKING) && !defined(FRONTEND)
+void
+AssertPageIsLockedForLSN(Page page)
+{
+	char *pagePtr = page;
+
+	/*
+	 * We only want to assert that we hold a lock on the page contents if the
+	 * page is shared (i.e. it is one of the BufferBlocks).  Crash recovery or
+	 * WAL replay is exempt from this assertion because page acecss is
+	 * guaranteed to be serialized.
+	 */
+	if (!InRecovery && BufferBlocks <= pagePtr
+		&& pagePtr < (BufferBlocks + NBuffers * BLCKSZ))
+	{
+		ptrdiff_t bufId = (pagePtr - BufferBlocks) / BLCKSZ;
+		BufferDesc *buf = GetBufferDescriptor(bufId);
+		LWLock *content_lock = BufferDescriptorGetContentLock(buf);
+		uint32 buf_state = pg_atomic_read_u32(&buf->state);
+
+		/*
+		 * Either:
+		 * 1) we hold the exclusive lock for the buffer contents, so reading is
+		 *    always safe, or
+		 * 2) we hold the shared lock and the header spinlock, in which case we
+		 *    are protected against other exclusive lock holders and other WAL
+		 *    log hint writers, or
+		 * 3) XLog hint bits aren't enabled (so there are no WAL log hint
+		 *    writers) and we just need the shared lock by itself.
+		 */
+		Assert(LWLockHeldByMeInMode(content_lock, LW_EXCLUSIVE)
+			   || (LWLockHeldByMeInMode(content_lock, LW_SHARED)
+				   && (!XLogHintBitIsNeeded() || (buf_state & BM_LOCKED))));
+	}
+}
+#endif
diff --git a/src/include/storage/bufpage.h b/src/include/storage/bufpage.h
index 50c72a3c8d..db94310586 100644
--- a/src/include/storage/bufpage.h
+++ b/src/include/storage/bufpage.h
@@ -359,8 +359,21 @@ PageValidateSpecialPointer(Page page)
  * Additional macros for access to page headers. (Beware multiple evaluation
  * of the arguments!)
  */
-#define PageGetLSN(page) \
-	PageXLogRecPtrGet(((PageHeader) (page))->pd_lsn)
+
+#if defined(USE_ASSERT_CHECKING) && !defined(FRONTEND)
+/* in bufmgr.c; used in PageGetLSN */
+extern void AssertPageIsLockedForLSN(Page page);
+#else
+#define AssertPageIsLockedForLSN(page)
+#endif
+
+static inline XLogRecPtr
+PageGetLSN(Page page)
+{
+	AssertPageIsLockedForLSN(page);
+	return PageXLogRecPtrGet(((PageHeader) page)->pd_lsn);
+}
+
 #define PageSetLSN(page, lsn) \
 	PageXLogRecPtrSet(((PageHeader) (page))->pd_lsn, lsn)
 
-- 
2.13.5 (Apple Git-94)