On Wed, May 03, 2023 at 09:54:13PM -0700, Nathan Bossart wrote: > Here's a new patch that removes the volatile marker from pltdata.
Gah, right after I sent that, I realized we can remove one more volatile marker. Sorry for the noise. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
>From 24a4e91b3cdd66b985fab70f3aed8f6a202b31fe Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nat...@postgresql.org> Date: Wed, 3 May 2023 11:32:43 -0700 Subject: [PATCH v5 1/1] Fix improper returns in PG_TRY blocks. If we exit a PG_TRY block early via "continue", "break", "goto", or "return", we'll skip unwinding its exception stack. This change moves a couple of such "return" statements in PL/Python out of PG_TRY blocks. This was introduced in d0aa965c0a and affects all supported versions. We might also be able to add compile-time checks to avoid recurrence, but that is left as a future exercise. Reported-by: Mikhail Gribkov, Xing Guo Author: Xing Guo Reviewed-by: Michael Paquier, Andres Freund, Tom Lane Discussion: https://postgr.es/m/CAMEv5_v5Y%2B-D%3DCO1%2Bqoe16sAmgC4sbbQjz%2BUtcHmB6zcgS%2B5Ew%40mail.gmail.com Discussion: https://postgr.es/m/CACpMh%2BCMsGMRKFzFMm3bYTzQmMU5nfEEoEDU2apJcc4hid36AQ%40mail.gmail.com Backpatch-through: 11 (all supported versions) --- src/pl/plpython/plpy_exec.c | 54 ++++++++++++++++++++++++------------- 1 file changed, 36 insertions(+), 18 deletions(-) diff --git a/src/pl/plpython/plpy_exec.c b/src/pl/plpython/plpy_exec.c index 923703535a..fcb363891a 100644 --- a/src/pl/plpython/plpy_exec.c +++ b/src/pl/plpython/plpy_exec.c @@ -411,15 +411,20 @@ static PyObject * PLy_function_build_args(FunctionCallInfo fcinfo, PLyProcedure *proc) { PyObject *volatile arg = NULL; - PyObject *volatile args = NULL; + PyObject *args = NULL; int i; + /* + * Make any Py*_New() calls before the PG_TRY block so that we can quickly + * return NULL on failure. We can't return within the PG_TRY block, else + * we'd miss unwinding the exception stack. + */ + args = PyList_New(proc->nargs); + if (!args) + return NULL; + PG_TRY(); { - args = PyList_New(proc->nargs); - if (!args) - return NULL; - for (i = 0; i < proc->nargs; i++) { PLyDatumToOb *arginfo = &proc->args[i]; @@ -683,19 +688,34 @@ PLy_trigger_build_args(FunctionCallInfo fcinfo, PLyProcedure *proc, HeapTuple *r *pltlevel, *pltrelid, *plttablename, - *plttableschema; - PyObject *pltargs, + *plttableschema, *pytnew, - *pytold; - PyObject *volatile pltdata = NULL; + *pytold, + *pltdata; + PyObject *volatile pltargs = NULL; char *stroid; - PG_TRY(); + /* + * Make any Py*_New() calls before the PG_TRY block so that we can quickly + * return NULL on failure. We can't return within the PG_TRY block, else + * we'd miss unwinding the exception stack. + */ + pltdata = PyDict_New(); + if (!pltdata) + return NULL; + + if (tdata->tg_trigger->tgnargs) { - pltdata = PyDict_New(); - if (!pltdata) + pltargs = PyList_New(tdata->tg_trigger->tgnargs); + if (!pltargs) + { + Py_DECREF(pltdata); return NULL; + } + } + PG_TRY(); + { pltname = PLyUnicode_FromString(tdata->tg_trigger->tgname); PyDict_SetItemString(pltdata, "name", pltname); Py_DECREF(pltname); @@ -835,12 +855,9 @@ PLy_trigger_build_args(FunctionCallInfo fcinfo, PLyProcedure *proc, HeapTuple *r int i; PyObject *pltarg; - pltargs = PyList_New(tdata->tg_trigger->tgnargs); - if (!pltargs) - { - Py_DECREF(pltdata); - return NULL; - } + /* pltargs should have been allocated before the PG_TRY block. */ + Assert(pltargs); + for (i = 0; i < tdata->tg_trigger->tgnargs; i++) { pltarg = PLyUnicode_FromString(tdata->tg_trigger->tgargs[i]); @@ -861,6 +878,7 @@ PLy_trigger_build_args(FunctionCallInfo fcinfo, PLyProcedure *proc, HeapTuple *r } PG_CATCH(); { + Py_XDECREF(pltargs); Py_XDECREF(pltdata); PG_RE_THROW(); } -- 2.25.1
