Skip to content

Commit c6f7f11

Browse files
tglsfdctglsfdc
committed
Fix edge-case resource leaks in PL/Python error reporting.
PLy_elog_impl and its subroutine PLy_traceback intended to avoid leaking any PyObject reference counts, but their coverage of the matter was sadly incomplete. In particular, out-of-memory errors in most of the string-construction subroutines could lead to reference count leaks, because those calls were outside the PG_TRY blocks responsible for dropping reference counts. Fix by (a) adjusting the scopes of the PG_TRY blocks, and (b) moving the responsibility for releasing the reference counts of the traceback-stack objects to PLy_elog_impl. This requires some additional "volatile" markers, but not too many. In passing, fix an ancient thinko: use of the "e_module_o" PyObject was guarded by "if (e_type_s)", where surely "if (e_module_o)" was meant. This would only have visible consequences if the "__name__" attribute were present but the "__module__" attribute wasn't, which apparently never happens; but someday it might. Rearranging the PG_TRY blocks requires indenting a fair amount of code one more tab stop, which I'll do separately for clarity. Author: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/2954090.1748723636@sss.pgh.pa.us Backpatch-through: 13
1 parent e5a3c9d commit c6f7f11

File tree

1 file changed

+55
-50
lines changed

1 file changed

+55
-50
lines changed

‎src/pl/plpython/plpy_elog.c‎

Lines changed: 55 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ PyObject *PLy_exc_spi_error = NULL;
1818

1919

2020
static void PLy_traceback(PyObject *e, PyObject *v, PyObject *tb,
21-
char **xmsg, char **tbmsg, int *tb_depth);
21+
char *volatile *xmsg, char *volatile *tbmsg,
22+
int *tb_depth);
2223
static void PLy_get_spi_error_data(PyObject *exc, int *sqlerrcode, char **detail,
2324
char **hint, char **query, int *position,
2425
char **schema_name, char **table_name, char **column_name,
@@ -43,13 +44,23 @@ void
4344
PLy_elog_impl(int elevel, const char *fmt,...)
4445
{
4546
int save_errno = errno;
46-
char *xmsg;
47-
char *tbmsg;
47+
char *volatile xmsg = NULL;
48+
char *volatile tbmsg = NULL;
4849
int tb_depth;
4950
StringInfoData emsg;
5051
PyObject *exc,
5152
*val,
5253
*tb;
54+
55+
/* If we'll need emsg, must initialize it before entering PG_TRY */
56+
if (fmt)
57+
initStringInfo(&emsg);
58+
59+
PyErr_Fetch(&exc, &val, &tb);
60+
61+
/* Use a PG_TRY block to ensure we release the PyObjects just acquired */
62+
PG_TRY();
63+
{
5364
const char *primary = NULL;
5465
int sqlerrcode = 0;
5566
char *detail = NULL;
@@ -62,8 +73,6 @@ PLy_elog_impl(int elevel, const char *fmt,...)
6273
char *datatype_name = NULL;
6374
char *constraint_name = NULL;
6475

65-
PyErr_Fetch(&exc, &val, &tb);
66-
6776
if (exc != NULL)
6877
{
6978
PyErr_NormalizeException(&exc, &val, &tb);
@@ -81,13 +90,11 @@ PLy_elog_impl(int elevel, const char *fmt,...)
8190
elevel = FATAL;
8291
}
8392

84-
/* this releases our refcount on tb! */
8593
PLy_traceback(exc, val, tb,
8694
&xmsg, &tbmsg, &tb_depth);
8795

8896
if (fmt)
8997
{
90-
initStringInfo(&emsg);
9198
for (;;)
9299
{
93100
va_list ap;
@@ -113,8 +120,6 @@ PLy_elog_impl(int elevel, const char *fmt,...)
113120
primary = xmsg;
114121
}
115122

116-
PG_TRY();
117-
{
118123
ereport(elevel,
119124
(errcode(sqlerrcode ? sqlerrcode : ERRCODE_EXTERNAL_ROUTINE_EXCEPTION),
120125
errmsg_internal("%s", primary ? primary : "no exception data"),
@@ -136,14 +141,23 @@ PLy_elog_impl(int elevel, const char *fmt,...)
136141
}
137142
PG_FINALLY();
138143
{
144+
Py_XDECREF(exc);
145+
Py_XDECREF(val);
146+
/* Must release all the objects in the traceback stack */
147+
while (tb != NULL && tb != Py_None)
148+
{
149+
PyObject *tb_prev = tb;
150+
151+
tb = PyObject_GetAttrString(tb, "tb_next");
152+
Py_DECREF(tb_prev);
153+
}
154+
/* For neatness' sake, also release our string buffers */
139155
if (fmt)
140156
pfree(emsg.data);
141157
if (xmsg)
142158
pfree(xmsg);
143159
if (tbmsg)
144160
pfree(tbmsg);
145-
Py_XDECREF(exc);
146-
Py_XDECREF(val);
147161
}
148162
PG_END_TRY();
149163
}
@@ -154,21 +168,14 @@ PLy_elog_impl(int elevel, const char *fmt,...)
154168
* The exception error message is returned in xmsg, the traceback in
155169
* tbmsg (both as palloc'd strings) and the traceback depth in
156170
* tb_depth.
157-
*
158-
* We release refcounts on all the Python objects in the traceback stack,
159-
* but not on e or v.
160171
*/
161172
static void
162173
PLy_traceback(PyObject *e, PyObject *v, PyObject *tb,
163-
char **xmsg, char **tbmsg, int *tb_depth)
174+
char *volatile *xmsg, char *volatile *tbmsg, int *tb_depth)
164175
{
165-
PyObject *e_type_o;
166-
PyObject *e_module_o;
167-
char *e_type_s = NULL;
168-
char *e_module_s = NULL;
169-
PyObject *vob = NULL;
170-
char *vstr;
171-
StringInfoData xstr;
176+
PyObject *volatile e_type_o = NULL;
177+
PyObject *volatile e_module_o = NULL;
178+
PyObject *volatile vob = NULL;
172179
StringInfoData tbstr;
173180

174181
/*
@@ -186,12 +193,18 @@ PLy_traceback(PyObject *e, PyObject *v, PyObject *tb,
186193
/*
187194
* Format the exception and its value and put it in xmsg.
188195
*/
196+
PG_TRY();
197+
{
198+
char *e_type_s = NULL;
199+
char *e_module_s = NULL;
200+
const char *vstr;
201+
StringInfoData xstr;
189202

190203
e_type_o = PyObject_GetAttrString(e, "__name__");
191204
e_module_o = PyObject_GetAttrString(e, "__module__");
192205
if (e_type_o)
193206
e_type_s = PLyUnicode_AsString(e_type_o);
194-
if (e_type_s)
207+
if (e_module_o)
195208
e_module_s = PLyUnicode_AsString(e_module_o);
196209

197210
if (v && ((vob = PyObject_Str(v)) != NULL))
@@ -215,18 +228,24 @@ PLy_traceback(PyObject *e, PyObject *v, PyObject *tb,
215228
appendStringInfo(&xstr, ": %s", vstr);
216229

217230
*xmsg = xstr.data;
231+
}
232+
PG_FINALLY();
233+
{
234+
Py_XDECREF(e_type_o);
235+
Py_XDECREF(e_module_o);
236+
Py_XDECREF(vob);
237+
}
238+
PG_END_TRY();
218239

219240
/*
220241
* Now format the traceback and put it in tbmsg.
221242
*/
222-
223243
*tb_depth = 0;
224244
initStringInfo(&tbstr);
225245
/* Mimic Python traceback reporting as close as possible. */
226246
appendStringInfoString(&tbstr, "Traceback (most recent call last):");
227247
while (tb != NULL && tb != Py_None)
228248
{
229-
PyObject *volatile tb_prev = NULL;
230249
PyObject *volatile frame = NULL;
231250
PyObject *volatile code = NULL;
232251
PyObject *volatile name = NULL;
@@ -254,17 +273,6 @@ PLy_traceback(PyObject *e, PyObject *v, PyObject *tb,
254273
filename = PyObject_GetAttrString(code, "co_filename");
255274
if (filename == NULL)
256275
elog(ERROR, "could not get file name from Python code object");
257-
}
258-
PG_CATCH();
259-
{
260-
Py_XDECREF(frame);
261-
Py_XDECREF(code);
262-
Py_XDECREF(name);
263-
Py_XDECREF(lineno);
264-
Py_XDECREF(filename);
265-
PG_RE_THROW();
266-
}
267-
PG_END_TRY();
268276

269277
/* The first frame always points at <module>, skip it. */
270278
if (*tb_depth > 0)
@@ -320,29 +328,26 @@ PLy_traceback(PyObject *e, PyObject *v, PyObject *tb,
320328
}
321329
}
322330
}
331+
}
332+
PG_FINALLY();
333+
{
334+
Py_XDECREF(frame);
335+
Py_XDECREF(code);
336+
Py_XDECREF(name);
337+
Py_XDECREF(lineno);
338+
Py_XDECREF(filename);
339+
}
340+
PG_END_TRY();
323341

324-
Py_DECREF(frame);
325-
Py_DECREF(code);
326-
Py_DECREF(name);
327-
Py_DECREF(lineno);
328-
Py_DECREF(filename);
329-
330-
/* Release the current frame and go to the next one. */
331-
tb_prev = tb;
342+
/* Advance to the next frame. */
332343
tb = PyObject_GetAttrString(tb, "tb_next");
333-
Assert(tb_prev != Py_None);
334-
Py_DECREF(tb_prev);
335344
if (tb == NULL)
336345
elog(ERROR, "could not traverse Python traceback");
337346
(*tb_depth)++;
338347
}
339348

340349
/* Return the traceback. */
341350
*tbmsg = tbstr.data;
342-
343-
Py_XDECREF(e_type_o);
344-
Py_XDECREF(e_module_o);
345-
Py_XDECREF(vob);
346351
}
347352

348353
/*

0 commit comments

Comments
 (0)