[virt-tools-list] [PATCH virt-viewer 2/2] Remove extra ref on SpiceDisplay

[Jonathon Jongsma]

On Thu, 2014-09-11 at 10:48 +0200, Christophe Fergeau wrote:
> On Wed, Sep 10, 2014 at 10:52:01PM +0200, Fabiano Fidêncio wrote:
> > On Sep 10, 2014 8:42 PM, "Jonathon Jongsma" <jjongsma at redhat.com> wrote:
> > >
> > > There's no need to ref the SpiceDisplay widget when adding it to a
> > > container. The container will take its own ref.
> > 
> > If we remove the SpiceDisplay from the container, it can unref the last
> > reference of the widget. Ack if you are sure that this is not the case why
> > the SpiceDisplay is ref'ed.
> 
> Looking at virt_viewer_display_spice_new and at _finalize, I think
> VirtViewerDisplaySpice already owns a reference to self->priv->display
> even without this explicit 'ref':
> self->priv->display = spice_display_new_with_monitor(s, ...);
> and
> g_object_unref(self->priv->display); in _finalize.
> 
> So I guess this extra ref was just leaked.
> 
> Christophe


Yeah, this extra ref was definitely leaked -- each SpiceDisplay widget
had 2 refs at program exit. In fact, even with this patch the
SpiceDisplay widgets are never actually freed. I think it's because the
VirtViewerDisplaySpice objects are still alive (keeping the SpiceDisplay
alive). Eventually, I need to chase down why things are not freed
properly at exit, but this one is an easy fix.

Jonathon


> 
> > 
> > > ---
> > >  src/virt-viewer-display-spice.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/src/virt-viewer-display-spice.c
> > b/src/virt-viewer-display-spice.c
> > > index 2ce42cf..e568cfb 100644
> > > --- a/src/virt-viewer-display-spice.c
> > > +++ b/src/virt-viewer-display-spice.c
> > > @@ -298,7 +298,7 @@ virt_viewer_display_spice_new(VirtViewerSessionSpice
> > *session,
> > >                                        G_CONNECT_SWAPPED);
> > >      update_display_ready(self);
> > >
> > > -    gtk_container_add(GTK_CONTAINER(self),
> > g_object_ref(self->priv->display));
> > > +    gtk_container_add(GTK_CONTAINER(self),
> > GTK_WIDGET(self->priv->display));
> > >      gtk_widget_show(GTK_WIDGET(self->priv->display));
> > >      g_object_set(self->priv->display,
> > >                   "grab-keyboard", TRUE,
> > > --
> > > 1.9.3
> > >
> > > _______________________________________________
> > > virt-tools-list mailing list
> > > virt-tools-list at redhat.com
> > > https://www.redhat.com/mailman/listinfo/virt-tools-list
> 
> > _______________________________________________
> > virt-tools-list mailing list
> > virt-tools-list at redhat.com
> > https://www.redhat.com/mailman/listinfo/virt-tools-list
>