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.

diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index acddb23161e78fdef517e7ed05c1db19712e2c57..9adf2d7a2e9de13888791943a7207b985baa2949 100644 (file)
--- 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 005d85e9ce10c8b6c0415e8beff3a10696f7daa0..43c3427196196c90a588ce198d9e8f5f92abaf9b 100644 (file)
--- 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 f2bf14ab92bf7482cf9e76bdfc377a995992f223..0210dfd8174d78b781c5f7cca35245e177a575ef 100644 (file)
--- 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