postgres.git / summary / log / commit / refs

commit    5b668ac7bef08d67e5cd78ec8854e67f42b49392
Author:   Tom Lane <tgl@sss.pgh.pa.us>
Date:     Wed Mar 26 20:41:44 2014 +0000
First released in: 9.4.0, 8.4.22, 9.0.18, 9.1.14, 9.2.9, 9.3.5

    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.


 src/pl/plpython/plpython.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/src/pl/plpython/plpython.c b/src/pl/plpython/plpython.c
index b748af1d358..2340d47891f 100644
--- a/src/pl/plpython/plpython.c
+++ b/src/pl/plpython/plpython.c
@@ -657,9 +657,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,
@@ -675,7 +673,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;
@@ -685,10 +683,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);
@@ -701,6 +699,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);
@@ -767,7 +766,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);


[parent: 0af5fd82e0ca]