PortalRun must guard against the possibility that the portal it's

running contains VACUUM or a similar command that will internally start
and commit transactions.  In such a case, the original caller values of
CurrentMemoryContext and CurrentResourceOwner will point to objects that
will be destroyed by the internal commit.  We must restore these pointers
to point to the newly-manufactured transaction context and resource owner,
rather than possibly pointing to deleted memory.
Also tweak xact.c so that AbortTransaction and AbortSubTransaction
forcibly restore a sane value for CurrentResourceOwner, much as they
have always done for CurrentMemoryContext.  I'm not certain this is
necessary but I'm feeling paranoid today.
Responds to Sean Chittenden's bug report of 4-Oct.

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 17db7dd78d52f2126b669b1c9e88ade46b35ae23..321a86f30c22f6c33ac05098d603ba19330771b3 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.190 2004/09/16 20:17:16 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.191 2004/10/04 21:52:14 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -205,6 +205,7 @@ static void AssignSubTransactionId(TransactionState s);
 static void AbortTransaction(void);
 static void AtAbort_Memory(void);
 static void AtCleanup_Memory(void);
+static void AtAbort_ResourceOwner(void);
 static void AtCommit_LocalCache(void);
 static void AtCommit_Memory(void);
 static void AtStart_Cache(void);
@@ -229,6 +230,7 @@ static void PopTransaction(void);
 
 static void AtSubAbort_Memory(void);
 static void AtSubCleanup_Memory(void);
+static void AtSubAbort_ResourceOwner(void);
 static void AtSubCommit_Memory(void);
 static void AtSubStart_Memory(void);
 static void AtSubStart_ResourceOwner(void);
@@ -1103,7 +1105,6 @@ AtAbort_Memory(void)
                MemoryContextSwitchTo(TopMemoryContext);
 }
 
-
 /*
  * AtSubAbort_Memory
  */
@@ -1115,6 +1116,33 @@ AtSubAbort_Memory(void)
        MemoryContextSwitchTo(TopTransactionContext);
 }
 
+
+/*
+ *     AtAbort_ResourceOwner
+ */
+static void
+AtAbort_ResourceOwner(void)
+{
+       /*
+        * Make sure we have a valid ResourceOwner, if possible (else it
+        * will be NULL, which is OK)
+        */
+       CurrentResourceOwner = TopTransactionResourceOwner;
+}
+
+/*
+ * AtSubAbort_ResourceOwner
+ */
+static void
+AtSubAbort_ResourceOwner(void)
+{
+       TransactionState s = CurrentTransactionState;
+
+       /* Make sure we have a valid ResourceOwner */
+       CurrentResourceOwner = s->curTransactionOwner;
+}
+
+
 /*
  * AtSubAbort_childXids
  */
@@ -1598,8 +1626,9 @@ AbortTransaction(void)
         */
        s->state = TRANS_ABORT;
 
-       /* Make sure we are in a valid memory context */
+       /* Make sure we have a valid memory context and resource owner */
        AtAbort_Memory();
+       AtAbort_ResourceOwner();
 
        /*
         * Reset user id which might have been changed transiently.  We cannot
@@ -3338,6 +3367,7 @@ AbortSubTransaction(void)
         * do abort processing
         */
        AtSubAbort_Memory();
+       AtSubAbort_ResourceOwner();
 
        /*
         * We can skip all this stuff if the subxact failed before creating

diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c
index 5a1c8b4867b39c36bec0c6f7551bc6beda0e8e30..2c4c2f3c7e5d6122dce0f583def04b824ecd1081 100644 (file)
--- a/src/backend/tcop/pquery.c
+++ b/src/backend/tcop/pquery.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/tcop/pquery.c,v 1.87 2004/09/13 20:07:05 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/tcop/pquery.c,v 1.88 2004/10/04 21:52:15 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -491,12 +491,14 @@ PortalRun(Portal portal, long count,
                  char *completionTag)
 {
        bool            result;
+       ResourceOwner saveTopTransactionResourceOwner;
+       MemoryContext saveTopTransactionContext;
        Portal          saveActivePortal;
        Snapshot        saveActiveSnapshot;
        ResourceOwner saveResourceOwner;
        MemoryContext savePortalContext;
        MemoryContext saveQueryContext;
-       MemoryContext oldContext;
+       MemoryContext saveMemoryContext;
 
        AssertArg(PortalIsValid(portal));
 
@@ -523,12 +525,26 @@ PortalRun(Portal portal, long count,
 
        /*
         * Set up global portal context pointers.
+        *
+        * We have to play a special game here to support utility commands like
+        * VACUUM and CLUSTER, which internally start and commit transactions.
+        * When we are called to execute such a command, CurrentResourceOwner
+        * will be pointing to the TopTransactionResourceOwner --- which will
+        * be destroyed and replaced in the course of the internal commit and
+        * restart.  So we need to be prepared to restore it as pointing to
+        * the exit-time TopTransactionResourceOwner.  (Ain't that ugly?  This
+        * idea of internally starting whole new transactions is not good.)
+        * CurrentMemoryContext has a similar problem, but the other pointers
+        * we save here will be NULL or pointing to longer-lived objects.
         */
+       saveTopTransactionResourceOwner = TopTransactionResourceOwner;
+       saveTopTransactionContext = TopTransactionContext;
        saveActivePortal = ActivePortal;
        saveActiveSnapshot = ActiveSnapshot;
        saveResourceOwner = CurrentResourceOwner;
        savePortalContext = PortalContext;
        saveQueryContext = QueryContext;
+       saveMemoryContext = CurrentMemoryContext;
        PG_TRY();
        {
                ActivePortal = portal;
@@ -537,7 +553,7 @@ PortalRun(Portal portal, long count,
                PortalContext = PortalGetHeapMemory(portal);
                QueryContext = portal->queryContext;
 
-               oldContext = MemoryContextSwitchTo(PortalContext);
+               MemoryContextSwitchTo(PortalContext);
 
                switch (portal->strategy)
                {
@@ -620,9 +636,16 @@ PortalRun(Portal portal, long count,
                portal->status = PORTAL_FAILED;
 
                /* Restore global vars and propagate error */
+               if (saveMemoryContext == saveTopTransactionContext)
+                       MemoryContextSwitchTo(TopTransactionContext);
+               else
+                       MemoryContextSwitchTo(saveMemoryContext);
                ActivePortal = saveActivePortal;
                ActiveSnapshot = saveActiveSnapshot;
-               CurrentResourceOwner = saveResourceOwner;
+               if (saveResourceOwner == saveTopTransactionResourceOwner)
+                       CurrentResourceOwner = TopTransactionResourceOwner;
+               else
+                       CurrentResourceOwner = saveResourceOwner;
                PortalContext = savePortalContext;
                QueryContext = saveQueryContext;
 
@@ -630,11 +653,16 @@ PortalRun(Portal portal, long count,
        }
        PG_END_TRY();
 
-       MemoryContextSwitchTo(oldContext);
-
+       if (saveMemoryContext == saveTopTransactionContext)
+               MemoryContextSwitchTo(TopTransactionContext);
+       else
+               MemoryContextSwitchTo(saveMemoryContext);
        ActivePortal = saveActivePortal;
        ActiveSnapshot = saveActiveSnapshot;
-       CurrentResourceOwner = saveResourceOwner;
+       if (saveResourceOwner == saveTopTransactionResourceOwner)
+               CurrentResourceOwner = TopTransactionResourceOwner;
+       else
+               CurrentResourceOwner = saveResourceOwner;
        PortalContext = savePortalContext;
        QueryContext = saveQueryContext;