Move return statements out of PG_TRY blocks.
authorNathan Bossart <nathan@postgresql.org>
Wed, 3 May 2023 18:32:43 +0000 (11:32 -0700)
committerNathan Bossart <nathan@postgresql.org>
Thu, 4 May 2023 23:26:00 +0000 (16:26 -0700)
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 prevent
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

index c6f6a6fbccaa61766b517f609fa2854ade7c303a..b38c73aef250312612f3f7eff3f3e23a749da11c 100644 (file)
@@ -415,15 +415,20 @@ static PyObject *
 PLy_function_build_args(FunctionCallInfo fcinfo, PLyProcedure *proc)
 {
    PyObject   *volatile arg = NULL;
-   PyObject   *volatile args = NULL;
+   PyObject   *args;
    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];
@@ -687,19 +692,34 @@ PLy_trigger_build_args(FunctionCallInfo fcinfo, PLyProcedure *proc, HeapTuple *r
               *pltlevel,
               *pltrelid,
               *plttablename,
-              *plttableschema;
-   PyObject   *pltargs,
+              *plttableschema,
+              *pltargs = NULL,
               *pytnew,
-              *pytold;
-   PyObject   *volatile pltdata = NULL;
+              *pytold,
+              *pltdata;
    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 = PyString_FromString(tdata->tg_trigger->tgname);
        PyDict_SetItemString(pltdata, "name", pltname);
        Py_DECREF(pltname);
@@ -839,12 +859,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 = PyString_FromString(tdata->tg_trigger->tgargs[i]);
@@ -865,6 +882,7 @@ PLy_trigger_build_args(FunctionCallInfo fcinfo, PLyProcedure *proc, HeapTuple *r
    }
    PG_CATCH();
    {
+       Py_XDECREF(pltargs);
        Py_XDECREF(pltdata);
        PG_RE_THROW();
    }