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 2c440ca1faca0d152ec494fd91fee4dcecc4c50a..cc38c6ff131cb91e913af8dca740366fa27945ca 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.257.2.5 2009/12/09 21:58:16 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.257.2.6 2010/01/24 21:49:39 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -2097,38 +2097,41 @@ 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);
+               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_xml();
-       AtEOXact_on_commit_actions(false);
-       AtEOXact_Namespace(false);
-       smgrabort();
-       AtEOXact_Files();
-       AtEOXact_ComboCid();
-       AtEOXact_HashTables(false);
-       AtEOXact_PgStat(false);
-       pgstat_report_xact_timestamp(0);
+               AtEOXact_GUC(false, 1);
+               AtEOXact_SPI(false);
+               AtEOXact_xml();
+               AtEOXact_on_commit_actions(false);
+               AtEOXact_Namespace(false);
+               smgrabort();
+               AtEOXact_Files();
+               AtEOXact_ComboCid();
+               AtEOXact_HashTables(false);
+               AtEOXact_PgStat(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 07033dc02fb5e634837df95280b0d479c47930f7..51ffef01bf0b13c953e3479a45ba44f47a042c0e 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.227.2.3 2009/10/27 20:14:39 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.227.2.4 2010/01/24 21:49:39 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -3060,10 +3060,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)
@@ -3095,7 +3094,15 @@ AfterTriggerEndSubXact(bool isCommit)
        else
        {
                /*
-                * Aborting.  We don't really need to release the subxact's event_cxt,
+                * 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;
+
+               /*
+                * We don't really need to release the subxact's event_cxt,
                 * since it will go away anyway when CurTransactionContext gets reset,
                 * but doing so early in subxact abort helps free space we might need.
                 *

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index df52fb8b098c0f1379203196b0bbb239f916292c..2c86df6b564c41b555e6d4a0a8fe8f6598fbd738 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.432.2.5 2009/12/09 21:58:17 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.432.2.6 2010/01/24 21:49:39 tgl Exp $
  *
  *--------------------------------------------------------------------
  */
@@ -3518,7 +3518,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)