Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit7fb8c92

Browse files
committed
Fix resource leaks in PL/Python error reporting, redux.
Commitc6f7f11 intended to prevent leaking any PyObject referencecounts in edge cases (such as out-of-memory during stringconstruction), but actually it introduced a leak in the normal case.Repeating an error-trapping operation often enough would lead tosession-lifespan memory bloat. The problem is that I failed tothink about the fact that PyObject_GetAttrString() increments therefcount of the returned PyObject, so that simply walking down thelist of error frame objects causes all but the first one to havetheir refcount incremented.I experimented with several more-or-less-complex ways around that,and eventually concluded that the right fix is simply to drop thenewly-obtained refcount as soon as we walk to the next frameobject in PLy_traceback. This sounds unsafe, but it's perfectlyokay because the caller holds a refcount on the first frame objectand each frame object holds a refcount on the next one; so thecurrent frame object can't disappear underneath us.By the same token, we can simplify the caller's cleanup back tosimply dropping its refcount on the first object. Cleanup ofeach frame object will lead in turn to the refcount of the nextone going to zero.I also added a couple of comments explaining why PLy_elog_impl()doesn't try to free the strings acquired from PLy_get_spi_error_data()or PLy_get_error_data(). That's because I got here by looking at aCoverity complaint about how those strings might get leaked. Theyare not leaked, but in testing that I discovered this other leak.Back-patch, asc6f7f11 was. It's a bit nervous-making to beputting such a fix into v13, which is only a couple weeks from itsfinal release; but I can't see that leaving a recently-introducedleak in place is a better idea.Author: Tom Lane <tgl@sss.pgh.pa.us>Discussion:https://postgr.es/m/1203918.1761184159@sss.pgh.pa.usBackpatch-through: 13
1 parent10799d0 commit7fb8c92

File tree

1 file changed

+21
-8
lines changed

1 file changed

+21
-8
lines changed

‎src/pl/plpython/plpy_elog.c‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -143,14 +143,7 @@ PLy_elog_impl(int elevel, const char *fmt,...)
143143
{
144144
Py_XDECREF(exc);
145145
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-
}
146+
Py_XDECREF(tb);
154147
/* For neatness' sake, also release our string buffers */
155148
if (fmt)
156149
pfree(emsg.data);
@@ -347,6 +340,17 @@ PLy_traceback(PyObject *e, PyObject *v, PyObject *tb,
347340
tb=PyObject_GetAttrString(tb,"tb_next");
348341
if (tb==NULL)
349342
elog(ERROR,"could not traverse Python traceback");
343+
344+
/*
345+
* Release the refcount that PyObject_GetAttrString acquired on the
346+
* next frame object. We don't need it, because our caller has a
347+
* refcount on the first frame object and the frame objects each have
348+
* a refcount on the next one. If we tried to hold this refcount
349+
* longer, it would greatly complicate cleanup in the event of a
350+
* failure in the above PG_TRY block.
351+
*/
352+
Py_DECREF(tb);
353+
350354
(*tb_depth)++;
351355
}
352356

@@ -380,6 +384,10 @@ PLy_get_sqlerrcode(PyObject *exc, int *sqlerrcode)
380384

381385
/*
382386
* Extract the error data from a SPIError
387+
*
388+
* Note: the returned string values are pointers into the given PyObject.
389+
* They must not be free()'d, and are not guaranteed to be valid once
390+
* we stop holding a reference on the PyObject.
383391
*/
384392
staticvoid
385393
PLy_get_spi_error_data(PyObject*exc,int*sqlerrcode,char**detail,
@@ -416,6 +424,11 @@ PLy_get_spi_error_data(PyObject *exc, int *sqlerrcode, char **detail,
416424
*
417425
* Note: position and query attributes are never set for Error so, unlike
418426
* PLy_get_spi_error_data, this function doesn't return them.
427+
*
428+
* Note: the returned string values are palloc'd in the current context.
429+
* While our caller could pfree them later, there's no real need to do so,
430+
* and it would be complicated to handle both this convention and that of
431+
* PLy_get_spi_error_data.
419432
*/
420433
staticvoid
421434
PLy_get_error_data(PyObject*exc,int*sqlerrcode,char**detail,char**hint,

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp