The previous fix in CVS HEAD and 8.4 for handling the case where a cursor

being used in a PL/pgSQL FOR loop is closed was inadequate, as Tom Lane
pointed out. The bug affects FOR statement variants too, because you can
close an implicitly created cursor too by guessing the "<unnamed portal X>"
name created for it.

To fix that, "pin" the portal to prevent it from being dropped while it's
being used in a PL/pgSQL FOR loop. Backpatch all the way to 7.4 which is
the oldest supported version.

src/backend/utils/mmgr/portalmem.c

@@ -12,7 +12,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/utils/mmgr/portalmem.c,v 1.62 2003/08/24 21:02:43 petere Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/utils/mmgr/portalmem.c,v 1.62.2.1 2010/07/05 09:27:56 heikki Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -288,6 +288,27 @@ PortalCreateHoldStore(Portal portal)
 	MemoryContextSwitchTo(oldcxt);
 }
 
+/*
+ * PinPortal
+ *		Protect a portal from dropping.
+ */
+void
+PinPortal(Portal portal)
+{
+	if (!portal->portalReady)
+		elog(ERROR, "cannot pin portal that's not in ready state");
+
+	portal->portalPinned = true;
+}
+
+void
+UnpinPortal(Portal portal)
+{
+	if (!portal->portalPinned)
+		elog(ERROR, "portal not pinned");
+	portal->portalPinned = false;
+}
+
 /*
  * PortalDrop
  *		Destroy the portal.
@@ -300,9 +321,17 @@ PortalDrop(Portal portal, bool isError)
 {
 	AssertArg(PortalIsValid(portal));
 
-	/* Not sure if this case can validly happen or not... */
+	/*
-	if (portal->portalActive)
+	 * Don't allow dropping a pinned portal, it's still needed by whoever
-		elog(ERROR, "cannot drop active portal");
+	 * pinned it. Unless we're doing post-abort cleanup; whoever pinned the
+	 * portal is going to go away at transaction abort anyway.
+	 *
+	 * Not sure if the portalActive case can validly happen or not...
+	 */
+	if ((portal->portalPinned && !isError) || portal->portalActive)
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_CURSOR_STATE),
+				 errmsg("cannot drop active portal \"%s\"", portal->name)));
 
 	/*
 	 * Remove portal from hash table.  Because we do this first, we will
@@ -400,6 +429,13 @@ AtCommit_Portals(void)
 		if (portal->portalActive)
 			continue;
 
+		/*
+		 * There should be no pinned portals anymore. Complain if someone
+		 * leaked one.
+		 */
+		if (portal->portalPinned)
+			elog(ERROR, "cannot commit while a portal is pinned");
+
 		if (portal->cursorOptions & CURSOR_OPT_HOLD)
 		{
 			/*

src/include/utils/portal.h

@@ -39,7 +39,7 @@
  * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: portal.h,v 1.47 2003/08/08 21:42:55 momjian Exp $
+ * $Id: portal.h,v 1.47.4.1 2010/07/05 09:27:56 heikki Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -117,6 +117,7 @@ typedef struct PortalData
 	bool		portalUtilReady;	/* PortalRunUtility complete? */
 	bool		portalActive;	/* portal is running (can't delete it) */
 	bool		portalDone;		/* portal is finished (don't re-run it) */
+	bool		portalPinned;	/* portal is pinned (can't delete it) */
 
 	/* If not NULL, Executor is active; call ExecutorEnd eventually: */
 	QueryDesc  *queryDesc;		/* info needed for executor invocation */
@@ -169,6 +170,8 @@ extern void AtAbort_Portals(void);
 extern void AtCleanup_Portals(void);
 extern Portal CreatePortal(const char *name, bool allowDup, bool dupSilent);
 extern Portal CreateNewPortal(void);
+extern void PinPortal(Portal portal);
+extern void UnpinPortal(Portal portal);
 extern void PortalDrop(Portal portal, bool isError);
 extern void DropDependentPortals(MemoryContext queryContext);
 extern Portal GetPortalByName(const char *name);

src/pl/plpgsql/src/pl_exec.c

@@ -3,7 +3,7 @@
  *			  procedural language
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.93.2.3 2007/02/08 18:38:19 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.93.2.4 2010/07/05 09:27:57 heikki Exp $
  *
  *	  This software is copyrighted by Jan Wieck - Hamburg.
  *
@@ -1384,9 +1384,11 @@ exec_stmt_fors(PLpgSQL_execstate * estate, PLpgSQL_stmt_fors * stmt)
 
 	/*
 	 * Open the implicit cursor for the statement and fetch the initial 10
-	 * rows.
+	 * rows. Pin the portal to make sure it doesn't get closed by the user
+	 * statements we execute.
 	 */
 	exec_run_select(estate, stmt->query, 0, &portal);
+	PinPortal(portal);
 
 	SPI_cursor_fetch(portal, true, 10);
 	tuptab = SPI_tuptable;
@@ -1425,6 +1427,7 @@ exec_stmt_fors(PLpgSQL_execstate * estate, PLpgSQL_stmt_fors * stmt)
 				 * (This code should match the code after the loop.)
 				 */
 				SPI_freetuptable(tuptab);
+				UnpinPortal(portal);
 				SPI_cursor_close(portal);
 				exec_set_found(estate, found);
 
@@ -1471,6 +1474,7 @@ exec_stmt_fors(PLpgSQL_execstate * estate, PLpgSQL_stmt_fors * stmt)
 	/*
 	 * Close the implicit cursor
 	 */
+	UnpinPortal(portal);
 	SPI_cursor_close(portal);
 
 	/*
@@ -2227,6 +2231,12 @@ exec_stmt_dynfors(PLpgSQL_execstate * estate, PLpgSQL_stmt_dynfors * stmt)
 	pfree(querystr);
 	SPI_freeplan(plan);
 
+	/*
+	 * Make sure the portal doesn't get closed by the user statements
+	 * we execute.
+	 */
+	PinPortal(portal);
+
 	/*
 	 * Fetch the initial 10 tuples
 	 */
@@ -2267,6 +2277,7 @@ exec_stmt_dynfors(PLpgSQL_execstate * estate, PLpgSQL_stmt_dynfors * stmt)
 				 * (This code should match the code after the loop.)
 				 */
 				SPI_freetuptable(tuptab);
+				UnpinPortal(portal);
 				SPI_cursor_close(portal);
 				exec_set_found(estate, found);
 
@@ -2313,6 +2324,7 @@ exec_stmt_dynfors(PLpgSQL_execstate * estate, PLpgSQL_stmt_dynfors * stmt)
 	/*
 	 * Close the implicit cursor
 	 */
+	UnpinPortal(portal);
 	SPI_cursor_close(portal);
 
 	/*