Re: [Libguestfs] [libnbd PATCH 2/2] python: Fix memory leak on callback error
On 11/30/21 04:15, Eric Blake wrote:
Commit 89af010d ("python: Fix more memory leaks", v1.5.2)
tried to
patch some python memory leaks on callback error paths. But it missed
an obvious one: we are mistakenly calling Py_INCREF on the result of
Py_BuildValue, which results in an object that is over-referenced and
thus never gets freed. Worse but less likely, if Py_BuildValue fails
due to no memory, Py_INCREF(NULL) causes a crash.
I tested this by watching the memory usage of:
$ nbdkit --no-sr memory 1
$ nbdsh -u nbd://localhost
nbd> def f(a, b, c, d, e):
... pass
...
nbd> def leak():
... for i in range(0, 100000):
... try:
... h.block_status(1, 0, f)
... except:
... pass
...
then repeatedly using leak().
Fixes: 936488d4 ("python: Implement Callback properly.", v0.1)
---
Hmm, I'm still seeing a leak even with this applied, so I'm missing
yet another problem.
I feel I know too little to review this patch, plus if there is another
known-to-exist leak in the python binding, shouldn't those patches both
become their own series?
(Of course if Rich ACKs this, I have nothing against it.)
Thanks,
Laszlo
generator/Python.ml | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/generator/Python.ml b/generator/Python.ml
index 49281bf..cb6c6b8 100644
--- a/generator/Python.ml
+++ b/generator/Python.ml
@@ -1,6 +1,6 @@
(* hey emacs, this is OCaml code: -*- tuareg -*- *)
(* nbd client library in userspace: Python bindings
- * Copyright (C) 2013-2020 Red Hat Inc.
+ * Copyright (C) 2013-2021 Red Hat Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -216,7 +216,7 @@ let print_python_closure_wrapper { cbname; cbargs } =
| CBArrayAndLen _ | CBMutable _ -> assert false
) cbargs;
pr ");\n";
- pr " Py_INCREF (py_args);\n";
+ pr " if (!py_args) { PyErr_PrintEx (0); return -1; }\n";
pr "\n";
pr " py_save = PyGILState_Ensure ();\n";
pr " py_ret = PyObject_CallObject (data->fn, py_args);\n";