AfterTriggerEndSubXact(false);
AtSubAbort_Portals(s->subTransactionId,
s->parent->subTransactionId,
+ s->curTransactionOwner,
s->parent->curTransactionOwner);
AtEOSubXact_LargeObject(false, s->subTransactionId,
s->parent->subTransactionId);
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("portal \"%s\" cannot be run", portal->name)));
portal->status = PORTAL_ACTIVE;
+ portal->activeSubid = GetCurrentSubTransactionId();
/*
* Set up global portal context pointers.
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("portal \"%s\" cannot be run", portal->name)));
portal->status = PORTAL_ACTIVE;
+ portal->activeSubid = GetCurrentSubTransactionId();
/*
* Set up global portal context pointers.
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("portal \"%s\" cannot be run", portal->name)));
portal->status = PORTAL_ACTIVE;
+ portal->activeSubid = GetCurrentSubTransactionId();
/*
* Set up global portal context pointers.
{
/*
* As per notes above, a rel to be rebuilt MUST have refcnt > 0; while of
- * course it would be a bad idea to blow away one with nonzero refcnt.
+ * course it would be an equally bad idea to blow away one with nonzero
+ * refcnt, since that would leave someone somewhere with a dangling
+ * pointer. All callers are expected to have verified that this holds.
*/
Assert(rebuild ?
!RelationHasReferenceCountZero(relation) :
{
if (isCommit)
relation->rd_createSubid = InvalidSubTransactionId;
- else
+ else if (RelationHasReferenceCountZero(relation))
{
RelationClearRelation(relation, false);
continue;
}
+ else
+ {
+ /*
+ * Hmm, somewhere there's a (leaked?) reference to the
+ * relation. We daren't remove the entry for fear of
+ * dereferencing a dangling pointer later. Bleat, and mark it
+ * as not belonging to the current transaction. Hopefully
+ * it'll get cleaned up eventually. This must be just a
+ * WARNING to avoid error-during-error-recovery loops.
+ */
+ relation->rd_createSubid = InvalidSubTransactionId;
+ elog(WARNING, "cannot remove relcache entry for \"%s\" because it has nonzero refcount",
+ RelationGetRelationName(relation));
+ }
}
/*
{
if (isCommit)
relation->rd_createSubid = parentSubid;
- else
+ else if (RelationHasReferenceCountZero(relation))
{
RelationClearRelation(relation, false);
continue;
}
+ else
+ {
+ /*
+ * Hmm, somewhere there's a (leaked?) reference to the
+ * relation. We daren't remove the entry for fear of
+ * dereferencing a dangling pointer later. Bleat, and
+ * transfer it to the parent subtransaction so we can try
+ * again later. This must be just a WARNING to avoid
+ * error-during-error-recovery loops.
+ */
+ relation->rd_createSubid = parentSubid;
+ elog(WARNING, "cannot remove relcache entry for \"%s\" because it has nonzero refcount",
+ RelationGetRelationName(relation));
+ }
}
/*
portal->status = PORTAL_NEW;
portal->cleanup = PortalCleanup;
portal->createSubid = GetCurrentSubTransactionId();
+ portal->activeSubid = portal->createSubid;
portal->strategy = PORTAL_MULTI_QUERY;
portal->cursorOptions = CURSOR_OPT_NO_SCROLL;
portal->atStart = true;
* not belonging to this transaction.
*/
portal->createSubid = InvalidSubTransactionId;
+ portal->activeSubid = InvalidSubTransactionId;
result = true;
}
/*
* Pre-subcommit processing for portals.
*
- * Reassign the portals created in the current subtransaction to the parent
- * subtransaction.
+ * Reassign portals created or used in the current subtransaction to the
+ * parent subtransaction.
*/
void
AtSubCommit_Portals(SubTransactionId mySubid,
if (portal->resowner)
ResourceOwnerNewParent(portal->resowner, parentXactOwner);
}
+ if (portal->activeSubid == mySubid)
+ portal->activeSubid = parentSubid;
}
}
/*
* Subtransaction abort handling for portals.
*
- * Deactivate portals created during the failed subtransaction.
- * Note that per AtSubCommit_Portals, this will catch portals created
+ * Deactivate portals created or used during the failed subtransaction.
+ * Note that per AtSubCommit_Portals, this will catch portals created/used
* in descendants of the subtransaction too.
*
* We don't destroy any portals here; that's done in AtSubCleanup_Portals.
void
AtSubAbort_Portals(SubTransactionId mySubid,
SubTransactionId parentSubid,
+ ResourceOwner myXactOwner,
ResourceOwner parentXactOwner)
{
HASH_SEQ_STATUS status;
{
Portal portal = hentry->portal;
+ /* Was it created in this subtransaction? */
if (portal->createSubid != mySubid)
+ {
+ /* No, but maybe it was used in this subtransaction? */
+ if (portal->activeSubid == mySubid)
+ {
+ /* Maintain activeSubid until the portal is removed */
+ portal->activeSubid = parentSubid;
+
+ /*
+ * Upper-level portals that failed while running in this
+ * subtransaction must be forced into FAILED state, for the
+ * same reasons discussed below.
+ *
+ * We assume we can get away without forcing upper-level READY
+ * portals to fail, even if they were run and then suspended.
+ * In theory a suspended upper-level portal could have
+ * acquired some references to objects that are about to be
+ * destroyed, but there should be sufficient defenses against
+ * such cases: the portal's original query cannot contain such
+ * references, and any references within, say, cached plans of
+ * PL/pgSQL functions are not from active queries and should
+ * be protected by revalidation logic.
+ */
+ if (portal->status == PORTAL_ACTIVE)
+ {
+ portal->status = PORTAL_FAILED;
+ /* let portalcmds.c clean up the state it knows about */
+ if (PointerIsValid(portal->cleanup))
+ {
+ (*portal->cleanup) (portal);
+ portal->cleanup = NULL;
+ }
+ }
+
+ /*
+ * Also, if we failed it during the current subtransaction
+ * (either just above, or earlier), reattach its resource
+ * owner to the current subtransaction's resource owner, so
+ * that any resources it still holds will be released while
+ * cleaning up this subtransaction. This prevents some corner
+ * cases wherein we might get Asserts or worse while cleaning
+ * up objects created during the current subtransaction
+ * (because they're still referenced within this portal).
+ */
+ if (portal->status == PORTAL_FAILED && portal->resowner)
+ {
+ ResourceOwnerNewParent(portal->resowner, myXactOwner);
+ portal->resowner = NULL;
+ }
+ }
+ /* Done if it wasn't created in this subtransaction */
continue;
+ }
/*
* Force any live portals of my own subtransaction into FAILED state.
* We have to do this because they might refer to objects created or
- * changed in the failed subtransaction, leading to crashes if
- * 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
- * have such references and hence may be able to continue.)
+ * changed in the failed subtransaction, leading to crashes within
+ * ExecutorEnd when portalcmds.c tries to close down the portal.
*/
if (portal->status == PORTAL_READY ||
portal->status == PORTAL_ACTIVE)
MemoryContext heap; /* subsidiary memory for portal */
ResourceOwner resowner; /* resources owned by portal */
void (*cleanup) (Portal portal); /* cleanup hook */
- SubTransactionId createSubid; /* the ID of the creating subxact */
/*
- * if createSubid is InvalidSubTransactionId, the portal is held over from
- * a previous transaction
+ * State data for remembering which subtransaction(s) the portal was
+ * created or used in. If the portal is held over from a previous
+ * transaction, both subxids are InvalidSubTransactionId. Otherwise,
+ * createSubid is the creating subxact and activeSubid is the last subxact
+ * in which we ran the portal.
*/
+ SubTransactionId createSubid; /* the creating subxact */
/* The query or queries the portal will execute */
const char *sourceText; /* text of query (as of 8.4, never NULL) */
/* Presentation data, primarily used by the pg_cursors system view */
TimestampTz creation_time; /* time at which this portal was defined */
bool visible; /* include this portal in pg_cursors? */
+
+ /*
+ * This field belongs with createSubid, but in pre-9.5 branches, add it
+ * at the end to avoid creating an ABI break for extensions that examine
+ * Portal structs.
+ */
+ SubTransactionId activeSubid; /* the last subxact with activity */
} PortalData;
/*
ResourceOwner parentXactOwner);
extern void AtSubAbort_Portals(SubTransactionId mySubid,
SubTransactionId parentSubid,
+ ResourceOwner myXactOwner,
ResourceOwner parentXactOwner);
extern void AtSubCleanup_Portals(SubTransactionId mySubid);
extern Portal CreatePortal(const char *name, bool allowDup, bool dupSilent);
(1 row)
abort;
+-- Test for proper cleanup after a failure in a cursor portal
+-- that was created in an outer subtransaction
+CREATE FUNCTION invert(x float8) RETURNS float8 LANGUAGE plpgsql AS
+$$ begin return 1/x; end $$;
+CREATE FUNCTION create_temp_tab() RETURNS text
+LANGUAGE plpgsql AS $$
+BEGIN
+ CREATE TEMP TABLE new_table (f1 float8);
+ -- case of interest is that we fail while holding an open
+ -- relcache reference to new_table
+ INSERT INTO new_table SELECT invert(0.0);
+ RETURN 'foo';
+END $$;
+BEGIN;
+DECLARE ok CURSOR FOR SELECT * FROM int8_tbl;
+DECLARE ctt CURSOR FOR SELECT create_temp_tab();
+FETCH ok;
+ q1 | q2
+-----+-----
+ 123 | 456
+(1 row)
+
+SAVEPOINT s1;
+FETCH ok; -- should work
+ q1 | q2
+-----+------------------
+ 123 | 4567890123456789
+(1 row)
+
+FETCH ctt; -- error occurs here
+ERROR: division by zero
+CONTEXT: PL/pgSQL function "invert" line 1 at RETURN
+SQL statement "INSERT INTO new_table SELECT invert(0.0)"
+PL/pgSQL function "create_temp_tab" line 5 at SQL statement
+ROLLBACK TO s1;
+FETCH ok; -- should work
+ q1 | q2
+------------------+-----
+ 4567890123456789 | 123
+(1 row)
+
+FETCH ctt; -- must be rejected
+ERROR: portal "ctt" cannot be run
+COMMIT;
+DROP FUNCTION create_temp_tab();
+DROP FUNCTION invert(x float8);
-- tests for the "tid" type
SELECT '(3, 3)'::tid = '(3, 4)'::tid;
?column?
abort;
+
+-- Test for proper cleanup after a failure in a cursor portal
+-- that was created in an outer subtransaction
+CREATE FUNCTION invert(x float8) RETURNS float8 LANGUAGE plpgsql AS
+$$ begin return 1/x; end $$;
+
+CREATE FUNCTION create_temp_tab() RETURNS text
+LANGUAGE plpgsql AS $$
+BEGIN
+ CREATE TEMP TABLE new_table (f1 float8);
+ -- case of interest is that we fail while holding an open
+ -- relcache reference to new_table
+ INSERT INTO new_table SELECT invert(0.0);
+ RETURN 'foo';
+END $$;
+
+BEGIN;
+DECLARE ok CURSOR FOR SELECT * FROM int8_tbl;
+DECLARE ctt CURSOR FOR SELECT create_temp_tab();
+FETCH ok;
+SAVEPOINT s1;
+FETCH ok; -- should work
+FETCH ctt; -- error occurs here
+ROLLBACK TO s1;
+FETCH ok; -- should work
+FETCH ctt; -- must be rejected
+COMMIT;
+
+DROP FUNCTION create_temp_tab();
+DROP FUNCTION invert(x float8);
+
+
-- tests for the "tid" type
SELECT '(3, 3)'::tid = '(3, 4)'::tid;
SELECT '(3, 3)'::tid = '(3, 3)'::tid;
projects / postgresql / commitdiff