From 30963fc20009b98a7a2b2dcd1f2ec559d58ac6d5 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Tue, 22 Oct 2002 22:44:36 +0000
Subject: [PATCH] Perform transaction cleanup operations in a less ad-hoc, more
 principled order; in particular ensure that all shared resources are released
 before we release transaction locks.  The code used to release locks before
 buffer pins, which might explain an ancient note I have about a bufmgr
 assertion failure I'd seen once several years ago, and been unable to
 reproduce since.  (Theory: someone trying to drop a relation might be able to
 reach FlushRelationBuffers before the last user of the relation had gotten
 around to dropping his buffer pins.)

---
 src/backend/access/transam/xact.c | 40 ++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 14 deletions(-)

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 4687578fed..a4d13c5bed 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.134 2002/10/21 22:06:18 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/access/transam/xact.c,v 1.135 2002/10/22 22:44:36 tgl Exp $
  *
  * NOTES
  *		Transaction aborts can now occur two ways:
@@ -1005,9 +1005,20 @@ CommitTransaction(void)
 	 * This is all post-commit cleanup.  Note that if an error is raised
 	 * here, it's too late to abort the transaction.  This should be just
 	 * noncritical resource releasing.
+	 *
+	 * The ordering of operations is not entirely random.  The idea is:
+	 * release resources visible to other backends (eg, files, buffer pins);
+	 * then release locks; then release backend-local resources.  We want
+	 * to release locks at the point where any backend waiting for us will
+	 * see our transaction as being fully cleaned up.
 	 */
 
 	smgrDoPendingDeletes(true);
+	AtCommit_Cache();
+	AtEOXact_Buffers(true);
+	/* smgrcommit already done */
+
+	AtCommit_Locks();
 
 	AtEOXact_GUC(true);
 	AtEOXact_SPI();
@@ -1016,15 +1027,10 @@ CommitTransaction(void)
 	AtEOXact_nbtree();
 	AtEOXact_rtree();
 	AtEOXact_Namespace(true);
-	AtCommit_Cache();
-	AtCommit_Locks();
 	AtEOXact_CatCache(true);
-	AtCommit_Memory();
-	AtEOXact_Buffers(true);
 	AtEOXact_Files();
-
-	/* Count transaction commit in statistics collector */
 	pgstat_count_xact_commit();
+	AtCommit_Memory();
 
 	/*
 	 * done with commit processing, set current transaction state back to
@@ -1078,6 +1084,9 @@ AbortTransaction(void)
 	 */
 	s->state = TRANS_ABORT;
 
+	/* Make sure we are in a valid memory context */
+	AtAbort_Memory();
+
 	/*
 	 * Reset user id which might have been changed transiently
 	 */
@@ -1109,7 +1118,17 @@ AbortTransaction(void)
 		LWLockRelease(SInvalLock);
 	}
 
+	/*
+	 * Post-abort cleanup.  See notes in CommitTransaction() concerning
+	 * ordering.
+	 */
+
 	smgrDoPendingDeletes(false);
+	AtAbort_Cache();
+	AtEOXact_Buffers(false);
+	smgrabort();
+
+	AtAbort_Locks();
 
 	AtEOXact_GUC(false);
 	AtEOXact_SPI();
@@ -1118,15 +1137,8 @@ AbortTransaction(void)
 	AtEOXact_nbtree();
 	AtEOXact_rtree();
 	AtEOXact_Namespace(false);
-	AtAbort_Cache();
 	AtEOXact_CatCache(false);
-	AtAbort_Memory();
-	AtEOXact_Buffers(false);
-	smgrabort();
 	AtEOXact_Files();
-	AtAbort_Locks();
-
-	/* Count transaction abort in statistics collector */
 	pgstat_count_xact_rollback();
 
 	/*