Re: plpgsql memory leak in 7.3.2? (repost) - Mailing list pgsql-general

"Mark Cave-Ayland" <m.cave-ayland@webbased.co.uk> writes:
> Whilst continuing work on some of our large tables again, we've
> encountered what we think may be a memory leak in plpgsql in 7.3.2.

Yup, you're right.  Looks like I introduced several memory leaks in
plpgsql when I modified spi.c to return a tuple descriptor even with
zero tuples returned: some plpgsql routines assumed they didn't need
to do SPI_freetuptable() after retrieving no tuples.  Patch attached,
or you can grab the updated pl_exec.c from our CVS server.

            regards, tom lane


*** src/pl/plpgsql/src/pl_exec.c~    Tue Jan 21 17:06:36 2003
--- src/pl/plpgsql/src/pl_exec.c    Sun Mar  2 15:45:59 2003
***************
*** 1369,1379 ****
              if (rc != PLPGSQL_RC_OK)
              {
                  /*
!                  * We're aborting the loop, so cleanup and set FOUND
                   */
-                 exec_set_found(estate, found);
                  SPI_freetuptable(tuptab);
                  SPI_cursor_close(portal);

                  if (rc == PLPGSQL_RC_EXIT)
                  {
--- 1369,1380 ----
              if (rc != PLPGSQL_RC_OK)
              {
                  /*
!                  * We're aborting the loop, so cleanup and set FOUND.
!                  * (This code should match the code after the loop.)
                   */
                  SPI_freetuptable(tuptab);
                  SPI_cursor_close(portal);
+                 exec_set_found(estate, found);

                  if (rc == PLPGSQL_RC_EXIT)
                  {
***************
*** 1411,1416 ****
--- 1412,1422 ----
      }

      /*
+      * Release last group of tuples
+      */
+     SPI_freetuptable(tuptab);
+
+     /*
       * Close the implicit cursor
       */
      SPI_cursor_close(portal);
***************
*** 2363,2369 ****
      if (n == 0)
          exec_move_row(estate, rec, row, NULL, tuptab->tupdesc);
      else
!         found = true;

      /*
       * Now do the loop
--- 2369,2375 ----
      if (n == 0)
          exec_move_row(estate, rec, row, NULL, tuptab->tupdesc);
      else
!         found = true;            /* processed at least one tuple */

      /*
       * Now do the loop
***************
*** 2382,2395 ****
               */
              rc = exec_stmts(estate, stmt->body);

-             /*
-              * We're aborting the loop, so cleanup and set FOUND
-              */
              if (rc != PLPGSQL_RC_OK)
              {
!                 exec_set_found(estate, found);
                  SPI_freetuptable(tuptab);
                  SPI_cursor_close(portal);

                  if (rc == PLPGSQL_RC_EXIT)
                  {
--- 2388,2402 ----
               */
              rc = exec_stmts(estate, stmt->body);

              if (rc != PLPGSQL_RC_OK)
              {
!                 /*
!                  * We're aborting the loop, so cleanup and set FOUND.
!                  * (This code should match the code after the loop.)
!                  */
                  SPI_freetuptable(tuptab);
                  SPI_cursor_close(portal);
+                 exec_set_found(estate, found);

                  if (rc == PLPGSQL_RC_EXIT)
                  {
***************
*** 2427,2433 ****
      }

      /*
!      * Close the cursor
       */
      SPI_cursor_close(portal);

--- 2434,2445 ----
      }

      /*
!      * Release last group of tuples
!      */
!     SPI_freetuptable(tuptab);
!
!     /*
!      * Close the implicit cursor
       */
      SPI_cursor_close(portal);

***************
*** 2736,2759 ****
      pfree(curname);

      /* ----------
-      * Initialize the global found variable to false
-      * ----------
-      */
-     exec_set_found(estate, false);
-
-     /* ----------
       * Determine if we fetch into a record or a row
       * ----------
       */
      if (stmt->rec != NULL)
          rec = (PLpgSQL_rec *) (estate->datums[stmt->rec->recno]);
      else
!     {
!         if (stmt->row != NULL)
!             row = (PLpgSQL_row *) (estate->datums[stmt->row->rowno]);
!         else
!             elog(ERROR, "unsupported target in exec_stmt_select()");
!     }

      /* ----------
       * Fetch 1 tuple from the cursor
--- 2748,2762 ----
      pfree(curname);

      /* ----------
       * Determine if we fetch into a record or a row
       * ----------
       */
      if (stmt->rec != NULL)
          rec = (PLpgSQL_rec *) (estate->datums[stmt->rec->recno]);
+     else if (stmt->row != NULL)
+         row = (PLpgSQL_row *) (estate->datums[stmt->row->rowno]);
      else
!         elog(ERROR, "unsupported target in exec_stmt_fetch()");

      /* ----------
       * Fetch 1 tuple from the cursor
***************
*** 2764,2785 ****
      n = SPI_processed;

      /* ----------
!      * If the FETCH didn't return a row, set the target
!      * to NULL and return with FOUND = false.
       * ----------
       */
      if (n == 0)
      {
          exec_move_row(estate, rec, row, NULL, tuptab->tupdesc);
!         return PLPGSQL_RC_OK;
      }
-
-     /* ----------
-      * Put the result into the target and set found to true
-      * ----------
-      */
-     exec_move_row(estate, rec, row, tuptab->vals[0], tuptab->tupdesc);
-     exec_set_found(estate, true);

      SPI_freetuptable(tuptab);

--- 2767,2785 ----
      n = SPI_processed;

      /* ----------
!      * Set the target and the global FOUND variable appropriately.
       * ----------
       */
      if (n == 0)
      {
          exec_move_row(estate, rec, row, NULL, tuptab->tupdesc);
!         exec_set_found(estate, false);
!     }
!     else
!     {
!         exec_move_row(estate, rec, row, tuptab->vals[0], tuptab->tupdesc);
!         exec_set_found(estate, true);
      }

      SPI_freetuptable(tuptab);