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;