[virt-tools-list] [PATCH virt-viewer] ovirt-foreign-menu: Plug memory leak

[Victor Toso]

On Thu, Aug 29, 2019 at 02:56:23PM -0300, Eduardo Lima (Etrunko) wrote:
> On 8/29/19 12:50 PM, Victor Toso wrote:
> > On Wed, Aug 28, 2019 at 06:18:02PM -0300, Eduardo Lima (Etrunko) wrote:
> >> Error caught by valgrind, the OvirtCollection object created in function
> >> ovirt_foreign_menu_fetch_vm_async() was never freed.
> >>
> >> 433 (40 direct, 393 indirect) bytes in 1 blocks are definitely lost in loss record 16,708 of 17,677
> >>    at 0x5868FDF: g_type_create_instance (in /usr/lib64/libgobject-2.0.so.0.6000.6)
> >>    by 0x584B42C: ??? (in /usr/lib64/libgobject-2.0.so.0.6000.6)
> >>    by 0x584D347: g_object_new_valist (in /usr/lib64/libgobject-2.0.so.0.6000.6)
> >>    by 0x584D69C: g_object_new (in /usr/lib64/libgobject-2.0.so.0.6000.6)
> >>    by 0x558E823: ovirt_collection_new (ovirt-collection.c:304)
> >>    by 0x558E98C: ovirt_sub_collection_new_from_resource_search (ovirt-collection.c:375)
> > 
> > I guess > ovirt_api_search_vms()
> > 
> > Does this happen too on code not compiled with
> > HAVE_OVIRT_API_SEARCH_VMS, otherwise you might need to ref after
> > ovirt_api_get_vms()
> 
> The ovirt_api_get_vms() and ovirt_api_search_vms() functions are
> compatible, returning an empty OvirtCollection object, which is only
> populated by the ovirt_collection_fetch_async() call.
> 
> So, indeed the same leak happens with both calls, because the
> OvirtCollection that is created in one function should be freed on the
> callback, as there is no where else holding the reference to it.

Thanks,
Acked-by: Victor Toso <victortoso at redhat.com>

> 
> Regards, Eduardo.
> 
> > 
> >>    by 0x42D510: ovirt_foreign_menu_fetch_vm_async (ovirt-foreign-menu.c:994)
> >>    by 0x42D510: ovirt_foreign_menu_next_async_step (ovirt-foreign-menu.c:316)
> >>    by 0x42D70D: api_fetched_cb (ovirt-foreign-menu.c:1025)
> >>    by 0x570BC19: ??? (in /usr/lib64/libgio-2.0.so.0.6000.6)
> >>    by 0x570C7EC: ??? (in /usr/lib64/libgio-2.0.so.0.6000.6)
> >>    by 0x559005D: call_async_cb (ovirt-proxy.c:279)
> >>    by 0x55B5A07: ??? (in /usr/lib64/librest-0.7.so.0.0.0)
> >>
> >> Signed-off-by: Eduardo Lima (Etrunko) <etrunko at redhat.com>
> >> ---
> >>  src/ovirt-foreign-menu.c | 5 ++++-
> >>  1 file changed, 4 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
> >> index 190bb3b..98ab7b9 100644
> >> --- a/src/ovirt-foreign-menu.c
> >> +++ b/src/ovirt-foreign-menu.c
> >> @@ -888,7 +888,7 @@ static void vms_fetched_cb(GObject *source_object,
> >>          g_debug("failed to fetch VM list: %s", error->message);
> >>          g_task_return_error(task, error);
> >>          g_object_unref(task);
> >> -        return;
> >> +        goto end;
> >>      }
> >>  
> >>      g_hash_table_iter_init(&iter, ovirt_collection_get_resources(collection));
> >> @@ -911,6 +911,9 @@ static void vms_fetched_cb(GObject *source_object,
> >>                                  "Could not find a VM with guid \"%s\"", menu->priv->vm_guid);
> >>          g_object_unref(task);
> >>      }
> >> +
> >> +end:
> >> +    g_object_unref(collection);
> >>  }
> >>  
> >>  
> >> -- 
> >> 2.21.0
> >>
> >> _______________________________________________
> >> virt-tools-list mailing list
> >> virt-tools-list at redhat.com
> >> https://www.redhat.com/mailman/listinfo/virt-tools-list
> 
> 
> -- 
> Eduardo de Barros Lima (Etrunko)
> Software Engineer - Red Hat
> etrunko at redhat.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20190830/948522c9/attachment.sig>