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.

4 files changed, 47 insertions, 19 deletions

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 43beb6b6066..3a992f6ccfe 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 f48c8389ce6..98ad33866be 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 d7fd83775b1..b95a233bac7 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;
+
+		expected_refcnt = relation->rd_isnailed ? 1 : 0;
 
-		if (relation->rd_isnailed)
-			RelationSetReferenceCount(relation, 1);
+		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 9cf3eee3bde..fd22a65296d 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