aboutsummaryrefslogtreecommitdiff
path: root/src/backend/storage/buffer
diff options
context:
space:
mode:
authorAndres Freund <andres@anarazel.de>2014-06-20 11:06:42 +0200
committerAndres Freund <andres@anarazel.de>2014-06-20 11:09:17 +0200
commit3bdcf6a5a7555035810e2ba2b8a0fb04dc5c66b8 (patch)
treeca974ea8843cb59f1ea8616f0f742e9faa5e8dbe /src/backend/storage/buffer
parent45b0f357235236dd3198f8abcca277adc0d7459a (diff)
downloadpostgresql-3bdcf6a5a7555035810e2ba2b8a0fb04dc5c66b8.tar.gz
postgresql-3bdcf6a5a7555035810e2ba2b8a0fb04dc5c66b8.zip
Don't allow to disable backend assertions via the debug_assertions GUC.
The existance of the assert_enabled variable (backing the debug_assertions GUC) reduced the amount of knowledge some static code checkers (like coverity and various compilers) could infer from the existance of the assertion. That could have been solved by optionally removing the assertion_enabled variable from the Assert() et al macros at compile time when some special macro is defined, but the resulting complication doesn't seem to be worth the gain from having debug_assertions. Recompiling is fast enough. The debug_assertions GUC is still available, but readonly, as it's useful when diagnosing problems. The commandline/client startup option -A, which previously also allowed to enable/disable assertions, has been removed as it doesn't serve a purpose anymore. While at it, reduce code duplication in bufmgr.c and localbuf.c assertions checking for spurious buffer pins. That code had to be reindented anyway to cope with the assert_enabled removal.
Diffstat (limited to 'src/backend/storage/buffer')
-rw-r--r--src/backend/storage/buffer/bufmgr.c62
-rw-r--r--src/backend/storage/buffer/localbuf.c51
2 files changed, 49 insertions, 64 deletions
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index c0702789446..07ea6652190 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -109,6 +109,7 @@ static volatile BufferDesc *BufferAlloc(SMgrRelation smgr,
bool *foundPtr);
static void FlushBuffer(volatile BufferDesc *buf, SMgrRelation reln);
static void AtProcExit_Buffers(int code, Datum arg);
+static void CheckForBufferLeaks(void);
static int rnode_comparator(const void *p1, const void *p2);
@@ -1699,34 +1700,13 @@ SyncOneBuffer(int buf_id, bool skip_recently_used)
return result | BUF_WRITTEN;
}
-
/*
* AtEOXact_Buffers - clean up at end of transaction.
- *
- * As of PostgreSQL 8.0, buffer pins should get released by the
- * ResourceOwner mechanism. This routine is just a debugging
- * cross-check that no pins remain.
*/
void
AtEOXact_Buffers(bool isCommit)
{
-#ifdef USE_ASSERT_CHECKING
- if (assert_enabled)
- {
- int RefCountErrors = 0;
- Buffer b;
-
- for (b = 1; b <= NBuffers; b++)
- {
- if (PrivateRefCount[b - 1] != 0)
- {
- PrintBufferLeakWarning(b);
- RefCountErrors++;
- }
- }
- Assert(RefCountErrors == 0);
- }
-#endif
+ CheckForBufferLeaks();
AtEOXact_LocalBuffers(isCommit);
}
@@ -1756,26 +1736,36 @@ AtProcExit_Buffers(int code, Datum arg)
AbortBufferIO();
UnlockBuffers();
+ CheckForBufferLeaks();
+
+ /* localbuf.c needs a chance too */
+ AtProcExit_LocalBuffers();
+}
+
+/*
+ * CheckForBufferLeaks - ensure this backend holds no buffer pins
+ *
+ * As of PostgreSQL 8.0, buffer pins should get released by the
+ * ResourceOwner mechanism. This routine is just a debugging
+ * cross-check that no pins remain.
+ */
+static void
+CheckForBufferLeaks(void)
+{
#ifdef USE_ASSERT_CHECKING
- if (assert_enabled)
- {
- int RefCountErrors = 0;
- Buffer b;
+ int RefCountErrors = 0;
+ Buffer b;
- for (b = 1; b <= NBuffers; b++)
+ for (b = 1; b <= NBuffers; b++)
+ {
+ if (PrivateRefCount[b - 1] != 0)
{
- if (PrivateRefCount[b - 1] != 0)
- {
- PrintBufferLeakWarning(b);
- RefCountErrors++;
- }
+ PrintBufferLeakWarning(b);
+ RefCountErrors++;
}
- Assert(RefCountErrors == 0);
}
+ Assert(RefCountErrors == 0);
#endif
-
- /* localbuf.c needs a chance too */
- AtProcExit_LocalBuffers();
}
/*
diff --git a/src/backend/storage/buffer/localbuf.c b/src/backend/storage/buffer/localbuf.c
index 3135c5cf156..6c81be42377 100644
--- a/src/backend/storage/buffer/localbuf.c
+++ b/src/backend/storage/buffer/localbuf.c
@@ -491,15 +491,15 @@ GetLocalBufferStorage(void)
}
/*
- * AtEOXact_LocalBuffers - clean up at end of transaction.
+ * CheckForLocalBufferLeaks - ensure this backend holds no local buffer pins
*
- * This is just like AtEOXact_Buffers, but for local buffers.
+ * This is just like CheckBufferLeaks(), but for local buffers.
*/
-void
-AtEOXact_LocalBuffers(bool isCommit)
+static void
+CheckForLocalBufferLeaks(void)
{
#ifdef USE_ASSERT_CHECKING
- if (assert_enabled && LocalRefCount)
+ if (LocalRefCount)
{
int RefCountErrors = 0;
int i;
@@ -520,33 +520,28 @@ AtEOXact_LocalBuffers(bool isCommit)
}
/*
+ * AtEOXact_LocalBuffers - clean up at end of transaction.
+ *
+ * This is just like AtEOXact_Buffers, but for local buffers.
+ */
+void
+AtEOXact_LocalBuffers(bool isCommit)
+{
+ CheckForLocalBufferLeaks();
+}
+
+/*
* AtProcExit_LocalBuffers - ensure we have dropped pins during backend exit.
*
- * This is just like AtProcExit_Buffers, but for local buffers. We shouldn't
- * be holding any remaining pins; if we are, and assertions aren't enabled,
- * we'll fail later in DropRelFileNodeBuffers while trying to drop the temp
- * rels.
+ * This is just like AtProcExit_Buffers, but for local buffers.
*/
void
AtProcExit_LocalBuffers(void)
{
-#ifdef USE_ASSERT_CHECKING
- if (assert_enabled && LocalRefCount)
- {
- int RefCountErrors = 0;
- int i;
-
- for (i = 0; i < NLocBuffer; i++)
- {
- if (LocalRefCount[i] != 0)
- {
- Buffer b = -i - 1;
-
- PrintBufferLeakWarning(b);
- RefCountErrors++;
- }
- }
- Assert(RefCountErrors == 0);
- }
-#endif
+ /*
+ * We shouldn't be holding any remaining pins; if we are, and assertions
+ * aren't enabled, we'll fail later in DropRelFileNodeBuffers while trying
+ * to drop the temp rels.
+ */
+ CheckForLocalBufferLeaks();
}