memory leak in PyEval_EvalCodeEx

[1st1]

BPO	46347
Nosy	@gpshead, @vstinner, @larryhastings, @elprans, @markshannon, @1st1, @pablogsal, @miss-islington
PRs	bpo-46347: Fix memory leak in PyEval_EvalCodeEx. #30546 [3.10] bpo-46347: Fix memory leak in PyEval_EvalCodeEx. (GH-30546) #30549 bpo-46347: Fix PyEval_EvalCodeEx to correctly cleanup in error paths #30551 [3.10] bpo-46347: Fix PyEval_EvalCodeEx to correctly cleanup in error paths #30553 bpo-46347: Yet another fix in the erorr path of PyEval_EvalCodeEx #30554

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2022-01-11.23:38:20.113>
created_at = <Date 2022-01-11.21:06:10.203>
labels = ['interpreter-core', '3.10', '3.11']
title = 'memory leak in PyEval_EvalCodeEx'
updated_at = <Date 2022-01-12.02:03:34.304>
user = 'https://github.com/1st1'

bugs.python.org fields:

activity = <Date 2022-01-12.02:03:34.304>
actor = 'pablogsal'
assignee = 'none'
closed = True
closed_date = <Date 2022-01-11.23:38:20.113>
closer = 'yselivanov'
components = ['Interpreter Core']
creation = <Date 2022-01-11.21:06:10.203>
creator = 'yselivanov'
dependencies = []
files = []
hgrepos = []
issue_num = 46347
keywords = ['patch']
message_count = 18.0
messages = ['410329', '410330', '410331', '410332', '410341', '410342', '410343', '410344', '410345', '410349', '410352', '410354', '410358', '410368', '410371', '410375', '410376', '410380']
nosy_count = 8.0
nosy_names = ['gregory.p.smith', 'vstinner', 'larry', 'Elvis.Pranskevichus', 'Mark.Shannon', 'yselivanov', 'pablogsal', 'miss-islington']
pr_nums = ['30546', '30549', '30551', '30553', '30554']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue46347'
versions = ['Python 3.10', 'Python 3.11']

[1st1]

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.

[1st1]

(this is the context of our current investigation btw: MagicStack/asyncpg#874 (comment))

[1st1]

This is the questionable place in the code:

cpython/Python/ceval.c

Lines 6131 to 6166 in dce642f

	PyObject **kwargs = PyMem_Malloc(sizeof(PyObject *)*kwcount);
	if (kwargs == NULL) {
	res = NULL;
	Py_DECREF(kwnames);
	goto fail;
	}
	for (int i = 0; i < kwcount; i++) {
	Py_INCREF(kws[2*i]);
	PyTuple_SET_ITEM(kwnames, i, kws[2*i]);
	kwargs[i] = kws[2*i+1];
	}
	PyFrameConstructor constr = {
	.fc_globals = globals,
	.fc_builtins = builtins,
	.fc_name = ((PyCodeObject *)_co)->co_name,
	.fc_qualname = ((PyCodeObject *)_co)->co_name,
	.fc_code = _co,
	.fc_defaults = defaults,
	.fc_kwdefaults = kwdefs,
	.fc_closure = closure
	};
	PyFunctionObject *func = _PyFunction_FromConstructor(&constr);
	if (func == NULL) {
	return NULL;
	}
	res = _PyEval_Vector(tstate, func, locals,
	allargs, argcount,
	kwnames);
	Py_DECREF(func);
	if (kwcount) {
	Py_DECREF(kwnames);
	PyMem_Free(newargs);
	}
	fail:
	Py_DECREF(defaults);
	return res;

See that kwargs is allocated but never freed or used.

[elprans]

I can confirm that removing the kwargs allocation prevents the leak described in the asyncpg issue from happening.

[1st1]

New changeset 607d8a8 by Yury Selivanov in branch 'main':
bpo-46347: Fix memory leak in PyEval_EvalCodeEx. (bpo-30546)
607d8a8

[pablogsal]

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 :(

[larryhastings]

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.

[larryhastings]

(Sorry--it'll leak "kwnames", "newargs", and "defaults".)

[pablogsal]

Apparently, this is causing adyncpg to leak megabytes in seconds

[miss-islington]

New changeset b1a94f1 by Miss Islington (bot) in branch '3.10':
bpo-46347: Fix memory leak in PyEval_EvalCodeEx. (GH-30546)
b1a94f1

[vstinner]

The memory leak has been fixed in 3.10 and main branches. Can this issue be closed now?