8000 The previous fix in CVS HEAD and 8.4 for handling the case where a cu… · sangli00/postgres@97f29c8 · GitHub
[go: up one dir, main page]
Skip to content
8000

Commit 97f29c8

Browse files
hlinnakahlinnaka
committed
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.
1 parent 5e77769 commit 97f29c8

File tree

3 files changed

+58
-7
lines changed

3 files changed

+58
-7
lines changed

‎src/backend/utils/mmgr/portalmem.c

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
* Portions Copyright (c) 1994, Regents of the University of California
1313
*
1414
* IDENTIFICATION
15-
* $Header: /cvsroot/pgsql/src/backend/utils/mmgr/portalmem.c,v 1.62 2003/08/24 21:02:43 petere Exp $
15+
* $Header: /cvsroot/pgsql/src/backend/utils/mmgr/portalmem.c,v 1.62.2.1 2010/07/05 09:27:56 heikki Exp $
1616
*
1717
*-------------------------------------------------------------------------
1818
*/
@@ -288,6 +288,27 @@ PortalCreateHoldStore(Portal portal)
288288
MemoryContextSwitchTo(oldcxt);
289289
}
290290

291+
/*
292+
* PinPortal
293+
* Protect a portal from dropping.
294+
*/
295+
void
296+
PinPortal(Portal portal)
297+
{
298+
if (!portal->portalReady)
299+
elog(ERROR, "cannot pin portal that's not in ready state");
300+
301+
portal->portalPinned = true;
302+
}
303+
304+
void
305+
UnpinPortal(Portal portal)
306+
{
307+
if (!portal->portalPinned)
308+
elog(ERROR, "portal not pinned");
309+
portal->portalPinned = false;
310+
}
311+
291312
/*
292313
* PortalDrop
293314
* Destroy the portal.
@@ -300,9 +321,17 @@ PortalDrop(Portal portal, bool isError)
300321
{
301322
AssertArg(PortalIsValid(portal));
302323

303-
/* Not sure if this case can validly happen or not... */
304-
if (portal->portalActive)
305-
elog(ERROR, "cannot drop active portal");
324+
/*
325+
* Don't allow dropping a pinned portal, it's still needed by whoever
326+
* pinned it. Unless we're doing post-abort cleanup; whoever pinned the
327+
* portal is going to go away at transaction abort anyway.
328+
*
329+
* Not sure if the portalActive case can validly happen or not...
330+
*/
331+
if ((portal->portalPinned && !isError) || portal->portalActive)
332+
ereport(ERROR,
333+
(errcode(ERRCODE_INVALID_CURSOR_STATE),
334+
errmsg("cannot drop active portal \"%s\"", portal->name)));
306335

307336
/*
308337
* Remove portal from hash table. Because we do this first, we will
@@ -400,6 +429,13 @@ AtCommit_Portals(void)
400429
if (portal->portalActive)
401430
continue;
402431

432+
/*
433+
* There should be no pinned portals anymore. Complain if someone
434+
* leaked one.
435+
*/
436+
if (portal->portalPinned)
437+
elog(ERROR, "cannot commit while a portal is pinned");
438+
403439
if (portal->cursorOptions & CURSOR_OPT_HOLD)
404440
{
405441
/*

‎src/include/utils/portal.h

Lines changed: 4 additions & 1 deletion
8000
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
* Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group
4040
* Portions Copyright (c) 1994, Regents of the University of California
4141
*
42-
* $Id: portal.h,v 1.47 2003/08/08 21:42:55 momjian Exp $
42+
* $Id: portal.h,v 1.47.4.1 2010/07/05 09:27:56 heikki Exp $
4343
*
4444
*-------------------------------------------------------------------------
4545
*/
@@ -117,6 +117,7 @@ typedef struct PortalData
117117
bool portalUtilReady; /* PortalRunUtility complete? */
118118
bool portalActive; /* portal is running (can't delete it) */
119119
bool portalDone; /* portal is finished (don't re-run it) */
120+
bool portalPinned; /* portal is pinned (can't delete it) */
120121

121122
/* If not NULL, Executor is active; call ExecutorEnd eventually: */
122123
QueryDesc *queryDesc; /* info needed for executor invocation */
@@ -169,6 +170,8 @@ extern void AtAbort_Portals(void);
169170
extern void AtCleanup_Portals(void);
170171
extern Portal CreatePortal(const char *name, bool allowDup, bool dupSilent);
171172
extern Portal CreateNewPortal(void);
173+
extern void PinPortal(Portal portal);
174+
extern void UnpinPortal(Portal portal);
172175
extern void PortalDrop(Portal portal, bool isError);
173176
extern void DropDependentPortals(MemoryContext queryContext);
174177
extern Portal GetPortalByName(const char< 628C /span> *name);

‎src/pl/plpgsql/src/pl_exec.c

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* procedural language
44
*
55
* IDENTIFICATION
6-
* $Header: /cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.93.2.3 2007/02/08 18:38:19 tgl Exp $
6+
* $Header: /cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.93.2.4 2010/07/05 09:27:57 heikki Exp $
77
*
88
* This software is copyrighted by Jan Wieck - Hamburg.
99
*
@@ -1384,9 +1384,11 @@ exec_stmt_fors(PLpgSQL_execstate * estate, PLpgSQL_stmt_fors * stmt)
13841384

13851385
/*
13861386
* Open the implicit cursor for the statement and fetch the initial 10
1387-
* rows.
1387+
* rows. Pin the portal to make sure it doesn't get closed by the user
1388+
* statements we execute.
13881389
*/
13891390
exec_run_select(estate, stmt->query, 0, &portal);
1391+
PinPortal(portal);
13901392

13911393
SPI_cursor_fetch(portal, true, 10);
13921394
tuptab = SPI_tuptable;
@@ -1425,6 +1427,7 @@ exec_stmt_fors(PLpgSQL_execstate * estate, PLpgSQL_stmt_fors * stmt)
14251427
* (This code should match the code after the loop.)
14261428
*/
14271429
SPI_freetuptable(tuptab);
1430+
UnpinPortal(portal);
14281431
SPI_cursor_close(portal);
14291432
exec_set_found(estate, found);
14301433

@@ -1471,6 +1474,7 @@ exec_stmt_fors(PLpgSQL_execstate * estate, PLpgSQL_stmt_fors * stmt)
14711474
/*
14721475
* Close the implicit cursor
14731476
*/
1477+
UnpinPortal(portal);
14741478
SPI_cursor_close(portal);
14751479

14761480
/*
@@ -2227,6 +2231,12 @@ exec_stmt_dynfors(PLpgSQL_execstate * estate, PLpgSQL_stmt_dynfors * stmt)
22272231
pfree(querystr);
22282232
SPI_freeplan(plan);
22292233

2234+
/*
2235+
* Make sure the portal doesn't get closed by the user statements
2236+
* we execute.
2237+
*/
2238+
PinPortal(portal);
2239+
22302240
/*
22312241
* Fetch the initial 10 tuples
22322242
*/
@@ -2267,6 +2277,7 @@ exec_stmt_dynfors(PLpgSQL_execstate * estate, PLpgSQL_stmt_dynfors * stmt)
22672277
* (This code should match the code after the loop.)
22682278
*/
22692279
SPI_freetuptable(tuptab);
2280+
UnpinPortal(portal);
22702281
SPI_cursor_close(portal);
22712282
exec_set_found(estate, found);
22722283

@@ -2313,6 +2324,7 @@ exec_stmt_dynfors(PLpgSQL_execstate * estate, PLpgSQL_stmt_dynfors * stmt)
23132324
/*
23142325
* Close the implicit cursor
23152326
*/
2327+
UnpinPortal(portal);
23162328
SPI_cursor_close(portal);
23172329

23182330
/*

0 commit comments

Comments
 (0)
0