Thread: pl/python: fix ref leak on elog

pl/python: fix ref leak on elog

From
Neil Conway
Date:
Attached is a simple patch that fixes a minor problem in PL/Python: in
PLy_spi_execute_plan(), we invoke the input function for a Postgres data
type. If that function elog's (which is quite possible), PL/Python leaks
references to two temporary Python objects. The fix seems easy, albeit a
bit ugly: the reference count on one of the temporary objects can be
decremented before calling the function. A pointer that points into the
second object is passed to the input function itself, so we have to
PG_CATCH() the elog and decrement the refcount then.

Since the problem might actually occur in practice and the fix seems to
have little chance of inducing a regression, I'm thinking of applying
this to both HEAD and back branches. Barring any objections I'll do that
later today or early tomorrow.

-Neil
Index: src/pl/plpython/plpython.c
===================================================================
RCS file: /Users/neilc/postgres/cvs_root/pgsql/src/pl/plpython/plpython.c,v
retrieving revision 1.67
diff -c -r1.67 plpython.c
*** src/pl/plpython/plpython.c    26 Dec 2005 04:28:48 -0000    1.67
--- src/pl/plpython/plpython.c    29 Dec 2005 16:49:54 -0000
***************
*** 2,8 ****
   * plpython.c - python as a procedural language for PostgreSQL
   *
   * This software is copyright by Andrew Bosma
!  * but is really shameless cribbed from pltcl.c by Jan Weick, and
   * plperl.c by Mark Hollomon.
   *
   * The author hereby grants permission to use, copy, modify,
--- 2,8 ----
   * plpython.c - python as a procedural language for PostgreSQL
   *
   * This software is copyright by Andrew Bosma
!  * but is really shameless cribbed from pltcl.c by Jan Wieck, and
   * plperl.c by Mark Hollomon.
   *
   * The author hereby grants permission to use, copy, modify,
***************
*** 1996,2002 ****
      int            i,
                  rv;
      PLyPlanObject *plan;
-     char       *nulls;
      MemoryContext oldcontext;

      if (list != NULL)
--- 1996,2001 ----
***************
*** 2018,2024 ****
      if (nargs != plan->nargs)
      {
          char       *sv;
-
          PyObject   *so = PyObject_Str(list);

          if (!so)
--- 2017,2022 ----
***************
*** 2036,2048 ****
      oldcontext = CurrentMemoryContext;
      PG_TRY();
      {
!         nulls = palloc(nargs * sizeof(char));

          for (i = 0; i < nargs; i++)
          {
              PyObject   *elem,
                         *so;
-             char       *sv;

              elem = PySequence_GetItem(list, i);
              if (elem != Py_None)
--- 2034,2045 ----
      oldcontext = CurrentMemoryContext;
      PG_TRY();
      {
!         char       *nulls = palloc(nargs * sizeof(char));

          for (i = 0; i < nargs; i++)
          {
              PyObject   *elem,
                         *so;

              elem = PySequence_GetItem(list, i);
              if (elem != Py_None)
***************
*** 2051,2070 ****
                  if (!so)
                      PLy_elog(ERROR, "function \"%s\" could not execute plan",
                               PLy_procedure_name(PLy_curr_procedure));
!                 sv = PyString_AsString(so);

!                 /*
!                  * FIXME -- if this elogs, we have Python reference leak
!                  */
!                 plan->values[i] =
!                     FunctionCall3(&(plan->args[i].out.d.typfunc),
!                                   CStringGetDatum(sv),
!                             ObjectIdGetDatum(plan->args[i].out.d.typioparam),
!                                   Int32GetDatum(-1));

!                 Py_DECREF(so);
!                 Py_DECREF(elem);

                  nulls[i] = ' ';
              }
              else
--- 2048,2073 ----
                  if (!so)
                      PLy_elog(ERROR, "function \"%s\" could not execute plan",
                               PLy_procedure_name(PLy_curr_procedure));
!                 Py_DECREF(elem);

!                 PG_TRY();
!                 {
!                     char *sv = PyString_AsString(so);

!                     plan->values[i] =
!                         FunctionCall3(&(plan->args[i].out.d.typfunc),
!                                       CStringGetDatum(sv),
!                                 ObjectIdGetDatum(plan->args[i].out.d.typioparam),
!                                       Int32GetDatum(-1));
!                 }
!                 PG_CATCH();
!                 {
!                     Py_DECREF(so);
!                     PG_RE_THROW();
!                 }
!                 PG_END_TRY();

+                 Py_DECREF(so);
                  nulls[i] = ' ';
              }
              else

Re: pl/python: fix ref leak on elog

From
Tom Lane
Date:
Neil Conway <neilc@samurai.com> writes:
> Since the problem might actually occur in practice and the fix seems to
> have little chance of inducing a regression, I'm thinking of applying
> this to both HEAD and back branches. Barring any objections I'll do that
> later today or early tomorrow.

Looks alright to me.  I agree with backpatching as far back as we have
PG_TRY.  The reason for the FIXME was that there wasn't any simple way
to fix it before that ...

            regards, tom lane

Re: pl/python: fix ref leak on elog

From
Neil Conway
Date:
Tom Lane wrote:
> Looks alright to me.

Applied to HEAD, REL8_1_STABLE, and REL8_0_STABLE.

-Neil