Fix assorted core dumps and Assert failures that could occur during

AbortTransaction or AbortSubTransaction, when trying to clean up after an
error that prevented (sub)transaction start from completing:
* access to TopTransactionResourceOwner that might not exist
* assert failure in AtEOXact_GUC, if AtStart_GUC not called yet
* assert failure or core dump in AfterTriggerEndSubXact, if
  AfterTriggerBeginSubXact not called yet

Per testing by injecting elog(ERROR) at successive steps in StartTransaction
and StartSubTransaction.  It's not clear whether all of these cases could
really occur in the field, but at least one of them is easily exposed by
simple stress testing, as per my accidental discovery yesterday.

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 55a46cef66891b791d127c23c778a9a4e26a9b81..33a862c8ef61fd2cfdbaa4191c7996e693d2ac5f 100644 (file)
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.215.2.6 2009/12/09 21:58:41 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.215.2.7 2010/01/24 21:49:58 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1919,35 +1919,38 @@ AbortTransaction(void)
 
    /*
     * Post-abort cleanup.  See notes in CommitTransaction() concerning
-    * ordering.
+    * ordering.  We can skip all of it if the transaction failed before
+    * creating a resource owner.
     */
+   if (TopTransactionResourceOwner != NULL)
+   {
+       CallXactCallbacks(XACT_EVENT_ABORT);
 
-   CallXactCallbacks(XACT_EVENT_ABORT);
-
-   ResourceOwnerRelease(TopTransactionResourceOwner,
-                        RESOURCE_RELEASE_BEFORE_LOCKS,
-                        false, true);
-   AtEOXact_Buffers(false);
-   AtEOXact_RelationCache(false);
-   AtEOXact_Inval(false);
-   smgrDoPendingDeletes(false);
-   AtEOXact_MultiXact();
-   ResourceOwnerRelease(TopTransactionResourceOwner,
-                        RESOURCE_RELEASE_LOCKS,
-                        false, true);
-   ResourceOwnerRelease(TopTransactionResourceOwner,
-                        RESOURCE_RELEASE_AFTER_LOCKS,
-                        false, true);
-   AtEOXact_CatCache(false);
-
-   AtEOXact_GUC(false, 1);
-   AtEOXact_SPI(false);
-   AtEOXact_on_commit_actions(false);
-   AtEOXact_Namespace(false);
-   smgrabort();
-   AtEOXact_Files();
-   AtEOXact_HashTables(false);
-   pgstat_count_xact_rollback();
+       ResourceOwnerRelease(TopTransactionResourceOwner,
+                            RESOURCE_RELEASE_BEFORE_LOCKS,
+                            false, true);
+       AtEOXact_Buffers(false);
+       AtEOXact_RelationCache(false);
+       AtEOXact_Inval(false);
+       smgrDoPendingDeletes(false);
+       AtEOXact_MultiXact();
+       ResourceOwnerRelease(TopTransactionResourceOwner,
+                            RESOURCE_RELEASE_LOCKS,
+                            false, true);
+       ResourceOwnerRelease(TopTransactionResourceOwner,
+                            RESOURCE_RELEASE_AFTER_LOCKS,
+                            false, true);
+       AtEOXact_CatCache(false);
+
+       AtEOXact_GUC(false, 1);
+       AtEOXact_SPI(false);
+       AtEOXact_on_commit_actions(false);
+       AtEOXact_Namespace(false);
+       smgrabort();
+       AtEOXact_Files();
+       AtEOXact_HashTables(false);
+       pgstat_count_xact_rollback();
+   }
 
    /*
     * State remains TRANS_ABORT until CleanupTransaction().

diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 5c97e5feabe8df4b3881e215846dbab9f3355a0e..b82125eb33776293d06426245cf3f5de946d0bbd 100644 (file)
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.195.2.7 2009/10/27 20:14:56 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.195.2.8 2010/01/24 21:49:58 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -2742,10 +2742,9 @@ AfterTriggerEndSubXact(bool isCommit)
    /*
     * Pop the prior state if needed.
     */
-   Assert(my_level < afterTriggers->maxtransdepth);
-
    if (isCommit)
    {
+       Assert(my_level < afterTriggers->maxtransdepth);
        /* If we saved a prior state, we don't need it anymore */
        state = afterTriggers->state_stack[my_level];
        if (state != NULL)
@@ -2758,7 +2757,15 @@ AfterTriggerEndSubXact(bool isCommit)
    else
    {
        /*
-        * Aborting --- restore the pointers from the stacks.
+        * Aborting.  It is possible subxact start failed before calling
+        * AfterTriggerBeginSubXact, in which case we mustn't risk touching
+        * stack levels that aren't there.
+        */
+       if (my_level >= afterTriggers->maxtransdepth)
+           return;
+
+       /*
+        * Restore the pointers from the stacks.
         */
        afterTriggers->events = afterTriggers->events_stack[my_level];
        afterTriggers->query_depth = afterTriggers->depth_stack[my_level];

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 751e79215122579e241ea3f7bebeaa696b76182b..ed734975854ef076669a616dbab4e1c5e2cd2683 100644 (file)
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -10,7 +10,7 @@
  * Written by Peter Eisentraut <[email protected]>.
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.299.2.6 2009/12/09 21:58:43 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.299.2.7 2010/01/24 21:49:58 tgl Exp $
  *
  *--------------------------------------------------------------------
  */
@@ -3109,7 +3109,14 @@ AtEOXact_GUC(bool isCommit, int nestLevel)
 {
    int         i;
 
-   Assert(nestLevel > 0 && nestLevel <= GUCNestLevel);
+   /*
+    * Note: it's possible to get here with GUCNestLevel == nestLevel-1 during
+    * abort, if there is a failure during transaction start before
+    * AtStart_GUC is called.
+    */
+   Assert(nestLevel > 0 &&
+          (nestLevel <= GUCNestLevel ||
+           (nestLevel == GUCNestLevel + 1 && !isCommit)));
 
    /* Quick exit if nothing's changed in this transaction */
    if (!guc_dirty)