| From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
|---|---|
| To: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Memory leak due to thinko in PL/Python error handling |
| Date: | 2025-10-23 01:49:19 |
| Message-ID: | 1203918.1761184159@sss.pgh.pa.us |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
I have realized that there's an oversight in my recent commit
c6f7f11d8, which so far from preventing edge-case leaks as intended,
actually introduces a memory leak in the normal error-catching path.
The leakage is easy to see if you extract the
catch_python_unique_violation() test case from plpython_error.sql
and run it in a loop, like
pl_regression=# do $$
begin
for i in 1..1000000 loop
perform catch_python_unique_violation();
end loop; end$$;
The backend's VIRT size as reported by top(1) will grow steadily.
The problem is that I did not think through the fact that
PyObject_GetAttrString() acquires a refcount on the returned object,
so that after advancing to the next frame object with
tb = PyObject_GetAttrString(tb, "tb_next");
we have an extra refcount on the new "tb" object, and there's no logic
that will get rid of that. The loop that I introduced with an eye to
cleaning things up:
/* Must release all the objects in the traceback stack */
while (tb != NULL && tb != Py_None)
{
PyObject *tb_prev = tb;
tb = PyObject_GetAttrString(tb, "tb_next");
Py_DECREF(tb_prev);
}
isn't right either, since it likewise doesn't account for the
extra refcount added by PyObject_GetAttrString.
The correct fix, I believe, is as attached. If we avoid collecting
extra refcounts during PLy_traceback(), then PLy_elog_impl() can go
back to simply doing Py_XDECREF(tb) on the first frame. Any
additional frames will go away when the previous frame object is
cleaned up and drops its refcount.
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 a
Coverity complaint about how those strings might get leaked. They
are not leaked, but in testing that I discovered this other leak.
regards, tom lane
| Attachment | Content-Type | Size |
|---|---|---|
| fix-plpython-error-handling-leaks-redux.patch | text/x-diff | 2.2 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tatsuo Ishii | 2025-10-23 02:06:59 | Re: Add RESPECT/IGNORE NULLS and FROM FIRST/LAST options |
| Previous Message | Chao Li | 2025-10-23 01:28:13 | Re: Include extension path on pg_available_extensions |