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.)

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 4687578feddde4427f6e466458b6a0f32fb10ade..a4d13c5beda98eae3a0541a208852799a4acdddf 100644 (file)
--- 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();
 
    /*