Commit 7981c342 authored by Tom Lane's avatar Tom Lane

Force READY portals into FAILED state when a transaction or subtransaction

is aborted, if they were created within the failed xact.  This prevents
ExecutorEnd from being run on them, which is a good idea because they may
contain references to tables or other objects that no longer exist.
In particular this is hazardous when auto_explain is active, but it's
really rather surprising that nobody has seen an issue with this before.
I'm back-patching this to 8.4, since that's the first version that contains
auto_explain or an ExecutorEnd hook, but I wonder whether we shouldn't
back-patch further.
parent c0d5be5d
...@@ -12,7 +12,7 @@ ...@@ -12,7 +12,7 @@
* Portions Copyright (c) 1994, Regents of the University of California * Portions Copyright (c) 1994, Regents of the University of California
* *
* IDENTIFICATION * IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/utils/mmgr/portalmem.c,v 1.116 2010/01/18 02:30:25 tgl Exp $ * $PostgreSQL: pgsql/src/backend/utils/mmgr/portalmem.c,v 1.117 2010/02/18 03:06:46 tgl Exp $
* *
*------------------------------------------------------------------------- *-------------------------------------------------------------------------
*/ */
...@@ -668,6 +668,7 @@ AtAbort_Portals(void) ...@@ -668,6 +668,7 @@ AtAbort_Portals(void)
{ {
Portal portal = hentry->portal; Portal portal = hentry->portal;
/* Any portal that was actually running has to be considered broken */
if (portal->status == PORTAL_ACTIVE) if (portal->status == PORTAL_ACTIVE)
portal->status = PORTAL_FAILED; portal->status = PORTAL_FAILED;
...@@ -677,6 +678,15 @@ AtAbort_Portals(void) ...@@ -677,6 +678,15 @@ AtAbort_Portals(void)
if (portal->createSubid == InvalidSubTransactionId) if (portal->createSubid == InvalidSubTransactionId)
continue; continue;
/*
* If it was created in the current transaction, we can't do normal
* shutdown on a READY portal either; it might refer to objects
* created in the failed transaction. See comments in
* AtSubAbort_Portals.
*/
if (portal->status == PORTAL_READY)
portal->status = PORTAL_FAILED;
/* let portalcmds.c clean up the state it knows about */ /* let portalcmds.c clean up the state it knows about */
if (PointerIsValid(portal->cleanup)) if (PointerIsValid(portal->cleanup))
{ {
...@@ -789,36 +799,17 @@ AtSubAbort_Portals(SubTransactionId mySubid, ...@@ -789,36 +799,17 @@ AtSubAbort_Portals(SubTransactionId mySubid,
continue; continue;
/* /*
* Force any active portals of my own transaction into FAILED state. * Force any live portals of my own subtransaction into FAILED state.
* This is mostly to ensure that a portal running a FETCH will go * We have to do this because they might refer to objects created or
* FAILED if the underlying cursor fails. (Note we do NOT want to do * changed in the failed subtransaction, leading to crashes if
* this to upper-level portals, since they may be able to continue.) * execution is resumed, or even if we just try to run ExecutorEnd.
* * (Note we do NOT do this to upper-level portals, since they cannot
* This is only needed to dodge the sanity check in PortalDrop. * have such references and hence may be able to continue.)
*/ */
if (portal->status == PORTAL_ACTIVE) if (portal->status == PORTAL_READY ||
portal->status == PORTAL_ACTIVE)
portal->status = PORTAL_FAILED; portal->status = PORTAL_FAILED;
/*
* If the portal is READY then allow it to survive into the parent
* transaction; otherwise shut it down.
*
* Currently, we can't actually support that because the portal's
* query might refer to objects created or changed in the failed
* subtransaction, leading to crashes if execution is resumed. So,
* even READY portals are deleted. It would be nice to detect whether
* the query actually depends on any such object, instead.
*/
#ifdef NOT_USED
if (portal->status == PORTAL_READY)
{
portal->createSubid = parentSubid;
if (portal->resowner)
ResourceOwnerNewParent(portal->resowner, parentXactOwner);
}
else
#endif
{
/* let portalcmds.c clean up the state it knows about */ /* let portalcmds.c clean up the state it knows about */
if (PointerIsValid(portal->cleanup)) if (PointerIsValid(portal->cleanup))
{ {
...@@ -844,7 +835,6 @@ AtSubAbort_Portals(SubTransactionId mySubid, ...@@ -844,7 +835,6 @@ AtSubAbort_Portals(SubTransactionId mySubid,
*/ */
MemoryContextDeleteChildren(PortalGetHeapMemory(portal)); MemoryContextDeleteChildren(PortalGetHeapMemory(portal));
} }
}
} }
/* /*
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment