From 22c64f18349dfba623f9c844363b484475d3c45e Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 2 Aug 2002 22:36:05 +0000
Subject: [PATCH] When compiling with --enable-cassert, check for reference
count leaks in the relcache. It's rather silly that we have reference count
leak checks in bufmgr and in catcache, but not in relcache which will
normally have many fewer entries. Chris K-L would have caught at least one
bug in his recent DROP patch if he'd had this.
---
src/backend/access/transam/xact.c | 6 ++--
src/backend/bootstrap/bootstrap.c | 7 ++---
src/backend/utils/cache/relcache.c | 49 +++++++++++++++++++++++-------
src/include/utils/relcache.h | 4 +--
4 files changed, 47 insertions(+), 19 deletions(-)
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 43beb6b606..3a992f6ccf 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
- * $Header: /cvsroot/pgsql/src/backend/access/transam/xact.c,v 1.128 2002/06/20 20:29:25 momjian Exp $
+ * $Header: /cvsroot/pgsql/src/backend/access/transam/xact.c,v 1.129 2002/08/02 22:36:05 tgl Exp $
*
* NOTES
* Transaction aborts can now occur two ways:
@@ -615,6 +615,8 @@ RecordTransactionCommit(void)
static void
AtCommit_Cache(void)
{
+ /* Check for relcache reference-count leaks */
+ AtEOXactRelationCache(true);
/*
* Make catalog changes visible to all backends.
*/
@@ -741,7 +743,7 @@ RecordTransactionAbort(void)
static void
AtAbort_Cache(void)
{
- RelationCacheAbort();
+ AtEOXactRelationCache(false);
AtEOXactInvalidationMessages(false);
}
diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index f48c8389ce..98ad33866b 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -8,7 +8,7 @@
* Portions Copyright (c) 1994, Regents of the University of California
*
* IDENTIFICATION
- * $Header: /cvsroot/pgsql/src/backend/bootstrap/bootstrap.c,v 1.134 2002/08/02 18:15:04 tgl Exp $
+ * $Header: /cvsroot/pgsql/src/backend/bootstrap/bootstrap.c,v 1.135 2002/08/02 22:36:05 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -550,7 +550,6 @@ closerel(char *name)
else
elog(ERROR, "closerel: close of '%s' before any relation was opened",
name);
-
}
if (boot_reldesc == NULL)
@@ -822,8 +821,8 @@ cleanup()
elog(FATAL, "Memory manager fault: cleanup called twice.\n");
proc_exit(1);
}
- if (boot_reldesc != (Relation) NULL)
- heap_close(boot_reldesc, NoLock);
+ if (boot_reldesc != NULL)
+ closerel(NULL);
CommitTransactionCommand();
proc_exit(Warnings);
}
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index d7fd83775b..b95a233bac 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
- * $Header: /cvsroot/pgsql/src/backend/utils/cache/relcache.c,v 1.168 2002/07/20 05:16:58 momjian Exp $
+ * $Header: /cvsroot/pgsql/src/backend/utils/cache/relcache.c,v 1.169 2002/08/02 22:36:05 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -1968,34 +1968,61 @@ RelationCacheInvalidate(void)
}
/*
- * RelationCacheAbort
+ * AtEOXactRelationCache
*
- * Clean up the relcache at transaction abort.
+ * Clean up the relcache at transaction commit or abort.
*
- * What we need to do here is reset relcache entry ref counts to
- * their normal not-in-a-transaction state. A ref count may be
+ * During transaction abort, we must reset relcache entry ref counts
+ * to their normal not-in-a-transaction state. A ref count may be
* too high because some routine was exited by elog() between
* incrementing and decrementing the count.
*
- * XXX Maybe we should do this at transaction commit, too, in case
- * someone forgets to decrement a refcount in a non-error path?
+ * During commit, we should not have to do this, but it's useful to
+ * check that the counts are correct to catch missed relcache closes.
+ * Since that's basically a debugging thing, only pay the cost when
+ * assert checking is enabled.
+ *
+ * In bootstrap mode, forget the debugging checks --- the bootstrap code
+ * expects relations to stay open across start/commit transaction calls.
*/
void
-RelationCacheAbort(void)
+AtEOXactRelationCache(bool commit)
{
HASH_SEQ_STATUS status;
RelIdCacheEnt *idhentry;
+#ifdef USE_ASSERT_CHECKING
+ if (commit && IsBootstrapProcessingMode())
+ return;
+#else
+ if (commit)
+ return;
+#endif
+
hash_seq_init(&status, RelationIdCache);
while ((idhentry = (RelIdCacheEnt *) hash_seq_search(&status)) != NULL)
{
Relation relation = idhentry->reldesc;
+ int expected_refcnt;
- if (relation->rd_isnailed)
- RelationSetReferenceCount(relation, 1);
+ expected_refcnt = relation->rd_isnailed ? 1 : 0;
+
+ if (commit)
+ {
+ if (relation->rd_refcnt != expected_refcnt)
+ {
+ elog(WARNING, "Relcache reference leak: relation \"%s\" has refcnt %d instead of %d",
+ RelationGetRelationName(relation),
+ relation->rd_refcnt, expected_refcnt);
+ RelationSetReferenceCount(relation, expected_refcnt);
+ }
+ }
else
- RelationSetReferenceCount(relation, 0);
+ {
+ /* abort case, just reset it quietly */
+ RelationSetReferenceCount(relation, expected_refcnt);
+ }
}
}
diff --git a/src/include/utils/relcache.h b/src/include/utils/relcache.h
index 9cf3eee3bd..fd22a65296 100644
--- a/src/include/utils/relcache.h
+++ b/src/include/utils/relcache.h
@@ -7,7 +7,7 @@
* Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
*
- * $Id: relcache.h,v 1.32 2002/06/20 20:29:53 momjian Exp $
+ * $Id: relcache.h,v 1.33 2002/08/02 22:36:05 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -63,7 +63,7 @@ extern void RelationCacheInvalidate(void);
extern void RelationPurgeLocalRelation(bool xactComitted);
-extern void RelationCacheAbort(void);
+extern void AtEOXactRelationCache(bool commit);
/*
* Routines to help manage rebuilding of relcache init file