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 068bc4e1eb32b4558d7bc33756518bbd54778b9c..f3ba0cfc4257f422c92addb4a49b7223619b465a 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.274.2.2 2009/12/09 21:58:04 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.274.2.3 2010/01/24 21:49:31 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -2056,37 +2056,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 2505de062305a232e025b7b71ae4f25c27c82499..09199ed27be754f7bfbe9adc62401d53aea6b871 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.248.2.1 2009/10/27 20:14:33 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.248.2.2 2010/01/24 21:49:31 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -3296,10 +3296,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)
@@ -3312,8 +3311,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 8f9a96722e99fe83ebbd7d84721292df7e45e9d5..fe81112ca00a21ee9d6a128d68d154c025173dbf 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.505.2.3 2009/12/09 21:58:04 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.505.2.4 2010/01/24 21:49:31 tgl Exp $
  *
  *--------------------------------------------------------------------
  */
@@ -3786,7 +3786,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)