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 1a89d78ba95060f33b9f784efdb56713dfd9e8ea..f74a941f66e024d43d54ecf56845c274b5b04d45 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.281 2010/01/16 10:05:50 sriggs Exp $
+ *       $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.282 2010/01/24 21:49:17 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -2170,37 +2170,40 @@ 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);
-       AtEOXact_Files();
-       AtEOXact_ComboCid();
-       AtEOXact_HashTables(false);
-       AtEOXact_PgStat(false);
-       AtEOXact_Snapshot(false);
-       pgstat_report_xact_timestamp(0);
+               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);
+               AtEOXact_Files();
+               AtEOXact_ComboCid();
+               AtEOXact_HashTables(false);
+               AtEOXact_PgStat(false);
+               AtEOXact_Snapshot(false);
+               pgstat_report_xact_timestamp(0);
+       }
 
        /*
         * State remains TRANS_ABORT until CleanupTransaction().

diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index e585f1517ad320308825200bbf5caa0e483dcae5..cec815cf608358191e17e40d0d25e43f41a944a6 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.259 2010/01/17 22:56:21 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.260 2010/01/24 21:49:17 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -3690,10 +3690,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)
@@ -3706,8 +3705,16 @@ AfterTriggerEndSubXact(bool isCommit)
        else
        {
                /*
-                * Aborting.  Release any event lists from queries being aborted, and
-                * restore query_depth to its pre-subxact value.
+                * 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;
+
+               /*
+                * Release any event lists from queries being aborted, and restore
+                * query_depth to its pre-subxact value.
                 */
                while (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 6846d13a9c7fae4f92d5560829fc806324d17b72..ab1b84ba607bb854d22cdfc56cc9aa5380f6d44b 100644 (file)
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -10,7 +10,7 @@
  * Written by Peter Eisentraut <peter_e@gmx.net>.
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.534 2010/01/23 16:37:12 sriggs Exp $
+ *       $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.535 2010/01/24 21:49:17 tgl Exp $
  *
  *--------------------------------------------------------------------
  */
@@ -3904,7 +3904,14 @@ AtEOXact_GUC(bool isCommit, int nestLevel)
        bool            still_dirty;
        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)