From 4a67565b37ef4fae12c341d069b1145cfae10144 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 19 Oct 2002 22:10:58 +0000 Subject: [PATCH] Fix within-function memory leaks in the various PLs' interfaces to SPI_prepare: they all save the prepared plan into topCxt, and so the procCxt copy that's actually returned by SPI_prepare ought to be freed. Diagnosis and plpython fix by Nigel Andrews, followup for other PLs by Tom Lane. --- src/pl/plpgsql/src/pl_exec.c | 7 ++++++- src/pl/plpython/plpython.c | 25 +++++++------------------ src/pl/tcl/pltcl.c | 9 ++++++--- 3 files changed, 19 insertions(+), 22 deletions(-) diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index acddb23161..9adf2d7a2e 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/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.64 2002/09/05 00:43:07 tgl Exp $ + * $Header: /cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.65 2002/10/19 22:10:58 tgl Exp $ * * This software is copyrighted by Jan Wieck - Hamburg. * @@ -2018,6 +2018,7 @@ exec_prepare_plan(PLpgSQL_execstate * estate, expr->plan_simple_expr = NULL; exec_simple_check_plan(expr); + SPI_freeplan(plan); pfree(argtypes); } @@ -2544,10 +2545,14 @@ exec_stmt_open(PLpgSQL_execstate * estate, PLpgSQL_stmt_open * stmt) * ---------- */ curplan = SPI_prepare(querystr, 0, NULL); + if (curplan == NULL) + elog(ERROR, "SPI_prepare() failed for dynamic query \"%s\"", + querystr); portal = SPI_cursor_open(curname, curplan, NULL, NULL); if (portal == NULL) elog(ERROR, "Failed to open cursor"); pfree(querystr); + SPI_freeplan(curplan); /* ---------- * Store the eventually assigned cursor name in the cursor variable diff --git a/src/pl/plpython/plpython.c b/src/pl/plpython/plpython.c index 005d85e9ce..43c3427196 100644 --- a/src/pl/plpython/plpython.c +++ b/src/pl/plpython/plpython.c @@ -29,7 +29,7 @@ * MAINTENANCE, SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS. * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/pl/plpython/plpython.c,v 1.25 2002/10/14 04:20:52 momjian Exp $ + * $Header: /cvsroot/pgsql/src/pl/plpython/plpython.c,v 1.26 2002/10/19 22:10:58 tgl Exp $ * ********************************************************************* */ @@ -1837,21 +1837,7 @@ PLy_plan_dealloc(PyObject * arg) enter(); if (ob->plan) - { - /* - * free the plan... pfree(ob->plan); - * - * FIXME -- leaks saved plan on object destruction. can this be - * avoided? - * I think so. A function prepares and then execp's a statement. - * When we come to deallocate the 'statement' object we obviously - * no long need the plan. Even if we did, without the object - * we're never going to be able to use it again. - * In the against arguments: SPI_saveplan has stuck this under - * the top context so there must be a reason for doing that. - */ - pfree(ob->plan); - } + SPI_freeplan(ob->plan); if (ob->types) PLy_free(ob->types); if (ob->args) @@ -2023,6 +2009,7 @@ PLy_spi_prepare(PyObject * self, PyObject * args) PyObject *list = NULL; PyObject *volatile optr = NULL; char *query; + void *tmpplan; enter(); @@ -2062,7 +2049,6 @@ PLy_spi_prepare(PyObject * self, PyObject * args) int nargs, i; - nargs = PySequence_Length(list); if (nargs > 0) { @@ -2125,7 +2111,10 @@ PLy_spi_prepare(PyObject * self, PyObject * args) RAISE_EXC(1); } - plan->plan = SPI_saveplan(plan->plan); + /* transfer plan from procCxt to topCxt */ + tmpplan = plan->plan; + plan->plan = SPI_saveplan(tmpplan); + SPI_freeplan(tmpplan); if (plan->plan == NULL) { PLy_exception_set(PLy_exc_spi_error, diff --git a/src/pl/tcl/pltcl.c b/src/pl/tcl/pltcl.c index f2bf14ab92..0210dfd817 100644 --- a/src/pl/tcl/pltcl.c +++ b/src/pl/tcl/pltcl.c @@ -31,7 +31,7 @@ * ENHANCEMENTS, OR MODIFICATIONS. * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/pl/tcl/pltcl.c,v 1.65 2002/10/14 04:20:52 momjian Exp $ + * $Header: /cvsroot/pgsql/src/pl/tcl/pltcl.c,v 1.66 2002/10/19 22:10:58 tgl Exp $ * **********************************************************************/ @@ -1285,7 +1285,7 @@ pltcl_elog(ClientData cdata, Tcl_Interp *interp, else if (strcmp(argv[1], "NOTICE") == 0) level = NOTICE; else if (strcmp(argv[1], "WARNING") == 0) - level = ERROR; + level = WARNING; else if (strcmp(argv[1], "ERROR") == 0) level = ERROR; else if (strcmp(argv[1], "FATAL") == 0) @@ -1837,7 +1837,8 @@ pltcl_SPI_prepare(ClientData cdata, Tcl_Interp *interp, } /************************************************************ - * Save the plan + * Save the plan into permanent memory (right now it's in the + * SPI procCxt, which will go away at function end). ************************************************************/ qdesc->plan = SPI_saveplan(plan); if (qdesc->plan == NULL) @@ -1866,6 +1867,8 @@ pltcl_SPI_prepare(ClientData cdata, Tcl_Interp *interp, elog(ERROR, "pltcl: SPI_saveplan() failed - %s", reason); } + /* Release the procCxt copy to avoid within-function memory leak */ + SPI_freeplan(plan); /************************************************************ * Insert a hashtable entry for the plan and return -- GitLab