classification
Title: memory leak in PyEval_EvalCodeEx
Type: Stage: resolved
Components: Interpreter Core Versions: Python 3.11, Python 3.10
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Elvis.Pranskevichus, Mark.Shannon, gregory.p.smith, larry, miss-islington, pablogsal, vstinner, yselivanov
Priority: normal Keywords: patch

Created on 2022-01-11 21:06 by yselivanov, last changed 2022-01-12 02:03 by pablogsal. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 30546 merged yselivanov, 2022-01-11 21:17
PR 30549 merged miss-islington, 2022-01-11 22:25
PR 30551 merged yselivanov, 2022-01-11 23:15
PR 30553 merged yselivanov, 2022-01-11 23:52
PR 30554 merged yselivanov, 2022-01-12 00:03
Messages (18)
msg410329 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2022-01-11 21:06
I'm investigating a memory leak in 3.10, and while looking at the offending commit I stumbled upon this: in ceval.c:PyEval_EvalCodeEx, there's this allocation:

    PyObject **kwargs = PyMem_Malloc(sizeof(PyObject *)*kwcount);

The problem is that this isn't ever freed. And `kwargs` isn't used anywhere in the function body. It seems to me that this is silently leaking memory.
msg410330 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2022-01-11 21:06
(this is the context of our current investigation btw: https://github.com/MagicStack/asyncpg/issues/874#issuecomment-1009383262)
msg410331 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2022-01-11 21:10
This is the questionable place in the code: https://github.com/python/cpython/blob/dce642f24418c58e67fa31a686575c980c31dd37/Python/ceval.c#L6131-L6166

See that `kwargs` is allocated but never freed or used.
msg410332 - (view) Author: Elvis Pranskevichus (Elvis.Pranskevichus) * (Python triager) Date: 2022-01-11 21:16
I can confirm that removing the kwargs allocation prevents the leak described in the asyncpg issue from happening.
msg410341 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2022-01-11 22:25
New changeset 607d8a838f29ad3c4c4e85b39f338dade5f9cafe by Yury Selivanov in branch 'main':
bpo-46347: Fix memory leak in PyEval_EvalCodeEx. (#30546)
https://github.com/python/cpython/commit/607d8a838f29ad3c4c4e85b39f338dade5f9cafe
msg410342 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2022-01-11 22:45
Turns out this leak affects CYthon quite a lot:

https://github.com/cython/cython/blob/29ad96444b8b1a4f05a6ac2328fde01de4782691/Cython/Utility/ObjectHandling.c#L2139-L2155

This seems to imply that every function call using __Pyx_PyCFunction_FastCall is leaking memory in 3.10, which is quite bad. We may need an urgent release for this :(
msg410343 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2022-01-11 22:56
The function will still leak "kwnames" and "default" if creating the "func" object fails.  Admittedly that would only happen in a low-memory condition which is a) rare and b) probably only happens just before the interpreter completely dies, so it's not worth addressing during today's mild emergency.
msg410344 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2022-01-11 22:57
(Sorry--it'll leak "kwnames", "newargs", and "defaults".)
msg410345 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2022-01-11 22:58
Apparently, this is causing adyncpg to leak megabytes in seconds
msg410349 - (view) Author: miss-islington (miss-islington) Date: 2022-01-11 23:09
New changeset b1a94f1fab7c0aee0705483616a1b2c3f2713c00 by Miss Islington (bot) in branch '3.10':
bpo-46347: Fix memory leak in PyEval_EvalCodeEx. (GH-30546)
https://github.com/python/cpython/commit/b1a94f1fab7c0aee0705483616a1b2c3f2713c00
msg410352 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2022-01-11 23:17
The memory leak has been fixed in 3.10 and main branches. Can this issue be closed now?
msg410354 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2022-01-11 23:19
> The function will still leak "kwnames" and "default" if creating the "func" object fails.

Yeah, here's another PR to address that: https://github.com/python/cpython/pull/30551
msg410358 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2022-01-11 23:37
New changeset 20b5791ce9b47195ce51cfd5acb223b1ca59cdf0 by Yury Selivanov in branch 'main':
bpo-46347: Fix PyEval_EvalCodeEx to correctly cleanup in error paths (#30551)
https://github.com/python/cpython/commit/20b5791ce9b47195ce51cfd5acb223b1ca59cdf0
msg410368 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2022-01-12 00:17
New changeset 6f9ca53a6ac343a5663cc5c52546acf9a63b605a by Yury Selivanov in branch '3.10':
bpo-46347: Fix PyEval_EvalCodeEx to correctly cleanup in error paths (#30553)
https://github.com/python/cpython/commit/6f9ca53a6ac343a5663cc5c52546acf9a63b605a
msg410371 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2022-01-12 00:35
New changeset be578e0c063dad1dbb273f86d5bc77e4e6f14583 by Yury Selivanov in branch 'main':
bpo-46347: Yet another fix in the erorr path of PyEval_EvalCodeEx (#30554)
https://github.com/python/cpython/commit/be578e0c063dad1dbb273f86d5bc77e4e6f14583
msg410375 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2022-01-12 01:12
when was the regression introduced?

I wouldn't necessarily rush an urgent release out unless this was just introduced in the 3.10.1 patch release.  Could it wait for 3.10.2 already scheduled for four weeks from now?
msg410376 - (view) Author: Elvis Pranskevichus (Elvis.Pranskevichus) * (Python triager) Date: 2022-01-12 01:18
The leak was introduced in 3.10a5 (https://github.com/python/cpython/commit/0332e569c12d3dc97171546c6dc10e42c27de34b)
msg410380 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2022-01-12 02:03
> Could it wait for 3.10.2 already scheduled for four weeks from now?

I don't feel comfortable leaving a ton of Cython functions leaking memory constantly on many function calls. According to https://github.com/MagicStack/asyncpg/issues/874 asyncpg can leak quite a lot of memory depending on your usage pattern.

Even if this is the first time we heard about this problem, I think this is important enough to justify a extra release.
History
Date User Action Args
2022-01-12 02:03:34pablogsalsetmessages: + msg410380
2022-01-12 01:18:50Elvis.Pranskevichussetmessages: + msg410376
2022-01-12 01:12:52gregory.p.smithsetnosy: + gregory.p.smith
messages: + msg410375
2022-01-12 00:35:43yselivanovsetmessages: + msg410371
2022-01-12 00:17:50yselivanovsetmessages: + msg410368
2022-01-12 00:03:02yselivanovsetpull_requests: + pull_request28755
2022-01-11 23:52:55yselivanovsetpull_requests: + pull_request28754
2022-01-11 23:38:20yselivanovsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2022-01-11 23:37:17yselivanovsetmessages: + msg410358
2022-01-11 23:19:19yselivanovsetmessages: + msg410354
2022-01-11 23:17:53vstinnersetmessages: + msg410352
components: + Interpreter Core
versions: + Python 3.10, Python 3.11
2022-01-11 23:15:02yselivanovsetpull_requests: + pull_request28752
2022-01-11 23:09:26miss-islingtonsetmessages: + msg410349
2022-01-11 22:58:44pablogsalsetmessages: + msg410345
2022-01-11 22:57:30larrysetmessages: + msg410344
2022-01-11 22:56:57larrysetnosy: + larry
messages: + msg410343
2022-01-11 22:45:28pablogsalsetmessages: + msg410342
2022-01-11 22:25:39yselivanovsetmessages: + msg410341
2022-01-11 22:25:39miss-islingtonsetnosy: + miss-islington
pull_requests: + pull_request28750
2022-01-11 21:17:11yselivanovsetkeywords: + patch
stage: patch review
pull_requests: + pull_request28746
2022-01-11 21:16:01Elvis.Pranskevichussetmessages: + msg410332
2022-01-11 21:10:23yselivanovsetmessages: + msg410331
2022-01-11 21:06:35yselivanovsetmessages: + msg410330
2022-01-11 21:06:10yselivanovcreate