Re: [Libguestfs] python: Avoid leaking py_array and py_args in event callbacks
On 2/17/23 18:39, Richard W.M. Jones wrote:
Version 1 was here:
https://listman.redhat.com/archives/libguestfs/2023-February/030732.html
Following Eric's suggestion here:
https://listman.redhat.com/archives/libguestfs/2023-February/030746.html
let's decrement the reference of py_array right after adding it to the
args. (This works even if args fails to be built).
I agree; (not having seen your new patch) I'd come to the same
conclusion after reading Eric's comment. We need to drop the initial
reference on py_array unconditionally -- if we succeed constructing the
tuple, it takes ownership, so we should drop our original (temporary)
reference; and if the tuple construction fails, there's nobody to foist
py_array upon, so we need to drop it just the same.
However the other part of Eric's suggestion is wrong as it ends up
calling Py_DECREF (args) when args == NULL along the error path. This
lead me to look more closely at this patch:
https://listman.redhat.com/archives/libguestfs/2019-January/020346.html
which I believe is wrong (at least, the part that fiddles with the
reference to args). I cannot reproduce the original problem, nor can
I find any justification by looking at the documentation of
PyObject_CallObject.
So we start by reverting that commit.
Yup, when I first looked at your v1 patch, git-blame had led me to
commit 85235aec8377 ("python: fix call of Python handlers of events",
2019-01-22), and I found the Py_INCREF() call dubious.
Laszlo