Fix refcounting bug in PLy_modify_tuple().

We must increment the refcount on "plntup" as soon as we have the
reference, not sometime later.  Otherwise, if an error is thrown in
between, the Py_XDECREF(plntup) call in the PG_CATCH block removes a
refcount we didn't add, allowing the object to be freed even though
it's still part of the plpython function's parsetree.

This appears to be the cause of crashes seen on buildfarm member
prairiedog.  It's a bit surprising that we've not seen it fail repeatably
before, considering that the regression tests have been exercising the
faulty code path since 2009.

The real-world impact is probably minimal, since it's unlikely anyone would
be provoking the "TD["new"] is not a dictionary" error in production, and
that's the only case that is actually wrong.  Still, it's a bug affecting
the regression tests, so patch all supported branches.

In passing, remove dead variable "plstr", and demote "platt" to a local
variable inside the PG_TRY block, since we don't need to clean it up
in the PG_CATCH path.

diff --git a/src/pl/plpython/plpy_exec.c b/src/pl/plpython/plpy_exec.c
index b2425edd6b496cc101ffbbc1402f709fadc80d3e..74e2f3dadff0ab7964fe4845babb9dd5b77c3e78 100644 (file)
--- a/src/pl/plpython/plpy_exec.c
+++ b/src/pl/plpython/plpy_exec.c
@@ -635,9 +635,7 @@ PLy_modify_tuple(PLyProcedure *proc, PyObject *pltd, TriggerData *tdata,
 {
        PyObject   *volatile plntup;
        PyObject   *volatile plkeys;
-       PyObject   *volatile platt;
        PyObject   *volatile plval;
-       PyObject   *volatile plstr;
        HeapTuple       rtup;
        int                     natts,
                                i,
@@ -653,7 +651,7 @@ PLy_modify_tuple(PLyProcedure *proc, PyObject *pltd, TriggerData *tdata,
        plerrcontext.previous = error_context_stack;
        error_context_stack = &plerrcontext;
 
-       plntup = plkeys = platt = plval = plstr = NULL;
+       plntup = plkeys = plval = NULL;
        modattrs = NULL;
        modvalues = NULL;
        modnulls = NULL;
@@ -663,10 +661,10 @@ PLy_modify_tuple(PLyProcedure *proc, PyObject *pltd, TriggerData *tdata,
                if ((plntup = PyDict_GetItemString(pltd, "new")) == NULL)
                        ereport(ERROR,
                                        (errmsg("TD[\"new\"] deleted, cannot modify row")));
+               Py_INCREF(plntup);
                if (!PyDict_Check(plntup))
                        ereport(ERROR,
                                        (errmsg("TD[\"new\"] is not a dictionary")));
-               Py_INCREF(plntup);
 
                plkeys = PyDict_Keys(plntup);
                natts = PyList_Size(plkeys);
@@ -679,6 +677,7 @@ PLy_modify_tuple(PLyProcedure *proc, PyObject *pltd, TriggerData *tdata,
 
                for (i = 0; i < natts; i++)
                {
+                       PyObject   *platt;
                        char       *plattstr;
 
                        platt = PyList_GetItem(plkeys, i);
@@ -745,7 +744,6 @@ PLy_modify_tuple(PLyProcedure *proc, PyObject *pltd, TriggerData *tdata,
                Py_XDECREF(plntup);
                Py_XDECREF(plkeys);
                Py_XDECREF(plval);
-               Py_XDECREF(plstr);
 
                if (modnulls)
                        pfree(modnulls);