Issue46347
Created on 2022-01-11 21:06 by yselivanov, last changed 2022-04-11 14:59 by admin. 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) * ![]() |
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) * ![]() |
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) * ![]() |
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) * ![]() |
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) * ![]() |
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) * ![]() |
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) * ![]() |
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) * ![]() |
Date: 2022-01-11 22:57 | |
(Sorry--it'll leak "kwnames", "newargs", and "defaults".) |
|||
| msg410345 - (view) | Author: Pablo Galindo Salgado (pablogsal) * ![]() |
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) * ![]() |
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) * ![]() |
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) * ![]() |
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) * ![]() |
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) * ![]() |
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) * ![]() |
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) * ![]() |
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) * ![]() |
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-04-11 14:59:54 | admin | set | github: 90505 |
| 2022-01-12 02:03:34 | pablogsal | set | messages: + msg410380 |
| 2022-01-12 01:18:50 | Elvis.Pranskevichus | set | messages: + msg410376 |
| 2022-01-12 01:12:52 | gregory.p.smith | set | nosy:
+ gregory.p.smith messages: + msg410375 |
| 2022-01-12 00:35:43 | yselivanov | set | messages: + msg410371 |
| 2022-01-12 00:17:50 | yselivanov | set | messages: + msg410368 |
| 2022-01-12 00:03:02 | yselivanov | set | pull_requests: + pull_request28755 |
| 2022-01-11 23:52:55 | yselivanov | set | pull_requests: + pull_request28754 |
| 2022-01-11 23:38:20 | yselivanov | set | status: open -> closed resolution: fixed stage: patch review -> resolved |
| 2022-01-11 23:37:17 | yselivanov | set | messages: + msg410358 |
| 2022-01-11 23:19:19 | yselivanov | set | messages: + msg410354 |
| 2022-01-11 23:17:53 | vstinner | set | messages:
+ msg410352 components: + Interpreter Core versions: + Python 3.10, Python 3.11 |
| 2022-01-11 23:15:02 | yselivanov | set | pull_requests: + pull_request28752 |
| 2022-01-11 23:09:26 | miss-islington | set | messages: + msg410349 |
| 2022-01-11 22:58:44 | pablogsal | set | messages: + msg410345 |
| 2022-01-11 22:57:30 | larry | set | messages: + msg410344 |
| 2022-01-11 22:56:57 | larry | set | nosy:
+ larry messages: + msg410343 |
| 2022-01-11 22:45:28 | pablogsal | set | messages: + msg410342 |
| 2022-01-11 22:25:39 | yselivanov | set | messages: + msg410341 |
| 2022-01-11 22:25:39 | miss-islington | set | nosy:
+ miss-islington pull_requests: + pull_request28750 |
| 2022-01-11 21:17:11 | yselivanov | set | keywords:
+ patch stage: patch review pull_requests: + pull_request28746 |
| 2022-01-11 21:16:01 | Elvis.Pranskevichus | set | messages: + msg410332 |
| 2022-01-11 21:10:23 | yselivanov | set | messages: + msg410331 |
| 2022-01-11 21:06:35 | yselivanov | set | messages: + msg410330 |
| 2022-01-11 21:06:10 | yselivanov | create | |

