Fix subtransaction cleanup after an outer-subtransaction portal fails.

Formerly, we treated only portals created in the current subtransaction as
having failed during subtransaction abort.  However, if the error occurred
while running a portal created in an outer subtransaction (ie, a cursor
declared before the last savepoint), that has to be considered broken too.

To allow reliable detection of which ones those are, add a bookkeeping
field to struct Portal that tracks the innermost subtransaction in which
each portal has actually been executed.  (Without this, we'd end up
failing portals containing functions that had called the subtransaction,
thereby breaking plpgsql exception blocks completely.)

In addition, when we fail an outer-subtransaction Portal, transfer its
resources into the subtransaction's resource owner, so that they're
released early in cleanup of the subxact.  This fixes a problem reported by
Jim Nasby in which a function executed in an outer-subtransaction cursor
could cause an Assert failure or crash by referencing a relation created
within the inner subtransaction.

The proximate cause of the Assert failure is that AtEOSubXact_RelationCache
assumed it could blow away a relcache entry without first checking that the
entry had zero refcount.  That was a bad idea on its own terms, so add such
a check there, and to the similar coding in AtEOXact_RelationCache.  This
provides an independent safety measure in case there are still ways to
provoke the situation despite the Portal-level changes.

This has been broken since subtransactions were invented, so back-patch
to all supported branches.

Tom Lane and Michael Paquier

1 files changed, 32 insertions, 3 deletions

diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 44e95098acb..420ef3da4c7 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -2048,7 +2048,9 @@ RelationClearRelation(Relation relation, bool rebuild)
 {
 	/*
 	 * 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) :
@@ -2654,11 +2656,25 @@ AtEOXact_cleanup(Relation relation, bool isCommit)
 	{
 		if (isCommit)
 			relation->rd_createSubid = InvalidSubTransactionId;
-		else
+		else if (RelationHasReferenceCountZero(relation))
 		{
 			RelationClearRelation(relation, false);
 			return;
 		}
+		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));
+		}
 	}
 
 	/*
@@ -2747,11 +2763,24 @@ AtEOSubXact_cleanup(Relation relation, bool isCommit,
 	{
 		if (isCommit)
 			relation->rd_createSubid = parentSubid;
-		else
+		else if (RelationHasReferenceCountZero(relation))
 		{
 			RelationClearRelation(relation, false);
 			return;
 		}
+		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));
+		}
 	}
 
 	/*