From: Nathan Bossart <nathandbossart@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Andres Freund <andres@anarazel.de>
Cc: Xing Guo <higuoxing@gmail.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: PL/Python: Fix return in the middle of PG_TRY() block.
Date: Wed, 3 May 2023 22:01:08 -0700
Message-ID: <20230504050108.GA2203557@nathanxps13> (raw)
In-Reply-To: <20230504045413.GA2202617@nathanxps13>
References: <20230113054900.b7onkvwtkrykeu3z@awork3.anarazel.de>
<20230113180335.GA2160040@nathanxps13>
<CACpMh+D9jz4Lq3qieoU2NkhGASqekhDXnXX2USW4mHa+SWNQag@mail.gmail.com>
<1864041.1673882943@sss.pgh.pa.us>
<20230120010711.jpqbaelpgmuvt2vq@awork3.anarazel.de>
<20230120190201.GC4106863@nathanxps13>
<20230503202116.GA2110623@nathanxps13>
<3012766.1683146012@sss.pgh.pa.us>
<20230503205838.GA2112379@nathanxps13>
<20230504045413.GA2202617@nathanxps13>
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.comAttachments:v5-0001-Fix-improper-returns-in-PG_TRY-blocks.patch (text/x-diff, 3.6K) download
From 24a4e91b3cdd66b985fab70f3aed8f6a202b31fe Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@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.cindex 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: nathandbossart@gmail.com, tgl@sss.pgh.pa.us, andres@anarazel.de, higuoxing@gmail.com
Subject: Re: PL/Python: Fix return in the middle of PG_TRY() block.
In-Reply-To: <20230504050108.GA2203557@nathanxps13>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox