Improve error reporting in code that checks for buffer refcount leaks.

tglsfdc · tglsfdc · commit dcafdbcde1ba · 2013-03-15T12:26:26.000-04:00
Formerly we just Assert'ed that each refcount was zero, which was quick
and easy but failed to provide a good overview of what was wrong.
Change the code so that we'll call PrintBufferLeakWarning() for each
buffer with a nonzero refcount, and then Assert at the end of the loop.
This costs nothing in runtime and might ease diagnosis of some bugs.

Greg Smith, reviewed by Satoshi Nagayasu, further tweaked by me
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
@@ -1699,12 +1699,18 @@ AtEOXact_Buffers(bool isCommit)
 #ifdef USE_ASSERT_CHECKING
 	if (assert_enabled)
 	{
-		int			i;
+		int			RefCountErrors = 0;
+		Buffer		b;
 
-		for (i = 0; i < NBuffers; i++)
+		for (b = 1; b <= NBuffers; b++)
 		{
-			Assert(PrivateRefCount[i] == 0);
+			if (PrivateRefCount[b - 1] != 0)
+			{
+				PrintBufferLeakWarning(b);
+				RefCountErrors++;
+			}
 		}
+		Assert(RefCountErrors == 0);
 	}
 #endif
 
@@ -1739,12 +1745,18 @@ AtProcExit_Buffers(int code, Datum arg)
 #ifdef USE_ASSERT_CHECKING
 	if (assert_enabled)
 	{
-		int			i;
+		int			RefCountErrors = 0;
+		Buffer		b;
 
-		for (i = 0; i < NBuffers; i++)
+		for (b = 1; b <= NBuffers; b++)
 		{
-			Assert(PrivateRefCount[i] == 0);
+			if (PrivateRefCount[b - 1] != 0)
+			{
+				PrintBufferLeakWarning(b);
+				RefCountErrors++;
+			}
 		}
+		Assert(RefCountErrors == 0);
 	}
 #endif
 
diff --git a/src/backend/storage/buffer/localbuf.c b/src/backend/storage/buffer/localbuf.c
@@ -497,14 +497,22 @@ void
 AtEOXact_LocalBuffers(bool isCommit)
 {
 #ifdef USE_ASSERT_CHECKING
-	if (assert_enabled)
+	if (assert_enabled && LocalRefCount)
 	{
+		int			RefCountErrors = 0;
 		int			i;
 
 		for (i = 0; i < NLocBuffer; i++)
 		{
-			Assert(LocalRefCount[i] == 0);
+			if (LocalRefCount[i] != 0)
+			{
+				Buffer	b = -i - 1;
+
+				PrintBufferLeakWarning(b);
+				RefCountErrors++;
+			}
 		}
+		Assert(RefCountErrors == 0);
 	}
 #endif
 }
@@ -523,12 +531,20 @@ AtProcExit_LocalBuffers(void)
 #ifdef USE_ASSERT_CHECKING
 	if (assert_enabled && LocalRefCount)
 	{
+		int			RefCountErrors = 0;
 		int			i;
 
 		for (i = 0; i < NLocBuffer; i++)
 		{
-			Assert(LocalRefCount[i] == 0);
+			if (LocalRefCount[i] != 0)
+			{
+				Buffer	b = -i - 1;
+
+				PrintBufferLeakWarning(b);
+				RefCountErrors++;
+			}
 		}
+		Assert(RefCountErrors == 0);
 	}
 #endif
 }

Original file line number	Diff line number	Diff line change
`@@ -1699,12 +1699,18 @@ AtEOXact_Buffers(bool isCommit)`
`1699`	`1699`	`#ifdef USE_ASSERT_CHECKING`
`1700`	`1700`	`if (assert_enabled)`
`1701`	`1701`	`{`
`1702`		`- int i;`
	`1702`	`+ int RefCountErrors = 0;`
	`1703`	`+ Buffer b;`
`1703`	`1704`
`1704`		`- for (i = 0; i < NBuffers; i++)`
	`1705`	`+ for (b = 1; b <= NBuffers; b++)`
`1705`	`1706`	`{`
`1706`		`- Assert(PrivateRefCount[i] == 0);`
	`1707`	`+ if (PrivateRefCount[b - 1] != 0)`
	`1708`	`+ {`
	`1709`	`+ PrintBufferLeakWarning(b);`
	`1710`	`+ RefCountErrors++;`
	`1711`	`+ }`
`1707`	`1712`	`}`
	`1713`	`+ Assert(RefCountErrors == 0);`
`1708`	`1714`	`}`
`1709`	`1715`	`#endif`
`1710`	`1716`
`@@ -1739,12 +1745,18 @@ AtProcExit_Buffers(int code, Datum arg)`
`1739`	`1745`	`#ifdef USE_ASSERT_CHECKING`
`1740`	`1746`	`if (assert_enabled)`
`1741`	`1747`	`{`
`1742`		`- int i;`
	`1748`	`+ int RefCountErrors = 0;`
	`1749`	`+ Buffer b;`
`1743`	`1750`
`1744`		`- for (i = 0; i < NBuffers; i++)`
	`1751`	`+ for (b = 1; b <= NBuffers; b++)`
`1745`	`1752`	`{`
`1746`		`- Assert(PrivateRefCount[i] == 0);`
	`1753`	`+ if (PrivateRefCount[b - 1] != 0)`
	`1754`	`+ {`
	`1755`	`+ PrintBufferLeakWarning(b);`
	`1756`	`+ RefCountErrors++;`
	`1757`	`+ }`
`1747`	`1758`	`}`
	`1759`	`+ Assert(RefCountErrors == 0);`
`1748`	`1760`	`}`
`1749`	`1761`	`#endif`
`1750`	`1762`
Original file line number	Diff line number	Diff line change
`@@ -497,14 +497,22 @@ void`
`497`	`497`	`AtEOXact_LocalBuffers(bool isCommit)`
`498`	`498`	`{`
`499`	`499`	`#ifdef USE_ASSERT_CHECKING`
`500`		`- if (assert_enabled)`
	`500`	`+ if (assert_enabled && LocalRefCount)`
`501`	`501`	`{`
	`502`	`+ int RefCountErrors = 0;`
`502`	`503`	`int i;`
`503`	`504`
`504`	`505`	`for (i = 0; i < NLocBuffer; i++)`
`505`	`506`	`{`
`506`		`- Assert(LocalRefCount[i] == 0);`
	`507`	`+ if (LocalRefCount[i] != 0)`
	`508`	`+ {`
	`509`	`+ Buffer b = -i - 1;`
	`510`	`+`
	`511`	`+ PrintBufferLeakWarning(b);`
	`512`	`+ RefCountErrors++;`
	`513`	`+ }`
`507`	`514`	`}`
	`515`	`+ Assert(RefCountErrors == 0);`
`508`	`516`	`}`
`509`	`517`	`#endif`
`510`	`518`	`}`
`@@ -523,12 +531,20 @@ AtProcExit_LocalBuffers(void)`
`523`	`531`	`#ifdef USE_ASSERT_CHECKING`
`524`	`532`	`if (assert_enabled && LocalRefCount)`
`525`	`533`	`{`
	`534`	`+ int RefCountErrors = 0;`
`526`	`535`	`int i;`
`527`	`536`
`528`	`537`	`for (i = 0; i < NLocBuffer; i++)`
`529`	`538`	`{`
`530`		`- Assert(LocalRefCount[i] == 0);`
	`539`	`+ if (LocalRefCount[i] != 0)`
	`540`	`+ {`
	`541`	`+ Buffer b = -i - 1;`
	`542`	`+`
	`543`	`+ PrintBufferLeakWarning(b);`
	`544`	`+ RefCountErrors++;`
	`545`	`+ }`
`531`	`546`	`}`
	`547`	`+ Assert(RefCountErrors == 0);`
`532`	`548`	`}`
`533`	`549`	`#endif`
`534`	`550`	`}`