Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 2d5e0f0

Browse files
committed
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.
1 parent c2a6724 commit 2d5e0f0

File tree

1 file changed

+3
-5
lines changed

1 file changed

+3
-5
lines changed

src/pl/plpython/plpy_exec.c

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -635,9 +635,7 @@ PLy_modify_tuple(PLyProcedure *proc, PyObject *pltd, TriggerData *tdata,
635635
{
636636
PyObject *volatile plntup;
637637
PyObject *volatile plkeys;
638-
PyObject *volatile platt;
639638
PyObject *volatile plval;
640-
PyObject *volatile plstr;
641639
HeapTuple rtup;
642640
int natts,
643641
i,
@@ -653,7 +651,7 @@ PLy_modify_tuple(PLyProcedure *proc, PyObject *pltd, TriggerData *tdata,
653651
plerrcontext.previous = error_context_stack;
654652
error_context_stack = &plerrcontext;
655653

656-
plntup = plkeys = platt = plval = plstr = NULL;
654+
plntup = plkeys = plval = NULL;
657655
modattrs = NULL;
658656
modvalues = NULL;
659657
modnulls = NULL;
@@ -663,10 +661,10 @@ PLy_modify_tuple(PLyProcedure *proc, PyObject *pltd, TriggerData *tdata,
663661
if ((plntup = PyDict_GetItemString(pltd, "new")) == NULL)
664662
ereport(ERROR,
665663
(errmsg("TD[\"new\"] deleted, cannot modify row")));
664+
Py_INCREF(plntup);
666665
if (!PyDict_Check(plntup))
667666
ereport(ERROR,
668667
(errmsg("TD[\"new\"] is not a dictionary")));
669-
Py_INCREF(plntup);
670668

671669
plkeys = PyDict_Keys(plntup);
672670
natts = PyList_Size(plkeys);
@@ -679,6 +677,7 @@ PLy_modify_tuple(PLyProcedure *proc, PyObject *pltd, TriggerData *tdata,
679677

680678
for (i = 0; i < natts; i++)
681679
{
680+
PyObject *platt;
682681
char *plattstr;
683682

684683
platt = PyList_GetItem(plkeys, i);
@@ -745,7 +744,6 @@ PLy_modify_tuple(PLyProcedure *proc, PyObject *pltd, TriggerData *tdata,
745744
Py_XDECREF(plntup);
746745
Py_XDECREF(plkeys);
747746
Py_XDECREF(plval);
748-
Py_XDECREF(plstr);
749747

750748
if (modnulls)
751749
pfree(modnulls);

0 commit comments

Comments
 (0)