On 06/18/2012 08:22 AM, Peter Krempa wrote:
> On 06/15/12 06:18, Eric Blake wrote:
>> This adds support for the new virDomainListAllSnapshots (a domain
>> function) and virDomainSnapshotListAllChildren (a snapshot function)
>> to the libvirt-python bindings. The implementation is done manually
>> as the generator does not support wrapping lists of C pointers into
>> python objects.
>>
>> + for (i = 0; i < c_retval; i++) {
>> + if ((pyobj_snap = libvirt_virDomainSnapshotPtrWrap(snaps[i]))
>> == NULL ||
>> + PyList_SetItem(py_retval, i, pyobj_snap) < 0) {
>
> Sadly, this code pattern doesn't save us from leaking memory: The
> PyCapsule objects, that are used to wrap libvirt pointers lack a
> destructor, so if this whole function fails, calling Py_DECREF() on the
> resulting output list doesn't save us from leaking all the processed
> references that were stored in the List. Later on, when the list of
> PyCapsules is converted into actual python-libvirt objects, those
> objects contain destructors that dispose the pointers properly.
>
> A workaround would be not to NULL members of the snap array and on
> successful end of the loop just set c_retval to 0 so that the cleanup
> loop is not called. But this still is not perfect.
>
> To fix all possibilities of leaking these pointers, we will need to
> provide destructor callbacks for PyCapsules that wrap libvirt pointers
> and increase the reference count, when the Capsule object is created.
> This does not apply to libvirt_virDomainSnapshotListNames and others as
> strings are wrapped by python wrappers that (I assume) have destructors,
> but does apply on my domain listing bindings.
Sounds like a global cleanup worth doing over multiple affected
functions at once...
>
> ACK, as the code follows a pattern, but memory leaks are possible yet
> very unlikely.
so I pushed it as-is, leaving the PyCapsule fix for later.
--
Eric Blake eblake@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list