[virt-tools-list] [PATCH virt-viewer] spice: fix double unref of main channel

[Daniel P. Berrange]

On Wed, Feb 29, 2012 at 12:58:41PM +0100, Marc-André Lureau wrote:
> When doing unref() on a channel, channel-destroy signal may be emitted
> during object dispose time, and it will attempt to unref() the channel
> again likely leading to a crash.
> 
> It may be that spice-gtk should have a different/simpler object
> life-cycle model, but it's also a good assumption to not take strong
> references on the channels, but just keep a weak reference as the
> session is really the channel life-cycle manager.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=797082
> ---
>  src/virt-viewer-session-spice.c |   13 +++----------
>  1 files changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/src/virt-viewer-session-spice.c b/src/virt-viewer-session-spice.c
> index cb7e000..eacb6e6 100644
> --- a/src/virt-viewer-session-spice.c
> +++ b/src/virt-viewer-session-spice.c
> @@ -111,8 +111,6 @@ virt_viewer_session_spice_dispose(GObject *obj)
>      }
>      if (spice->priv->audio)
>          g_object_unref(spice->priv->audio);
> -    if (spice->priv->main_channel)
> -        g_object_unref(spice->priv->main_channel);
>      if (spice->priv->main_window)
>          g_object_unref(spice->priv->main_window);
>  
> @@ -386,16 +384,13 @@ virt_viewer_session_spice_channel_new(SpiceSession *s,
>      g_object_get(channel, "channel-id", &id, NULL);
>  
>      if (SPICE_IS_MAIN_CHANNEL(channel)) {
> -        if (self->priv->main_channel != NULL) {
> -            /* FIXME: use telepathy-glib g_signal_connect_object to automatically disconnect.. */
> +        if (self->priv->main_channel != NULL)
>              g_signal_handlers_disconnect_by_func(self->priv->main_channel,
>                                                   virt_viewer_session_spice_main_channel_event, self);
> -            g_object_unref(self->priv->main_channel);
> -        }
>  
>          g_signal_connect(channel, "channel-event",
>                           G_CALLBACK(virt_viewer_session_spice_main_channel_event), self);
> -        self->priv->main_channel = g_object_ref(channel);
> +        self->priv->main_channel = SPICE_MAIN_CHANNEL(channel);
>      }
>  
>      if (SPICE_IS_DISPLAY_CHANNEL(channel)) {
> @@ -439,10 +434,8 @@ virt_viewer_session_spice_channel_destroy(G_GNUC_UNUSED SpiceSession *s,
>      g_object_get(channel, "channel-id", &id, NULL);
>      if (SPICE_IS_MAIN_CHANNEL(channel)) {
>          DEBUG_LOG("zap main channel");
> -        if (channel == SPICE_CHANNEL(self->priv->main_channel)) {
> -            g_object_unref(self->priv->main_channel);
> +        if (channel == SPICE_CHANNEL(self->priv->main_channel))
>              self->priv->main_channel = NULL;
> -        }
>      }
>  
>      if (SPICE_IS_DISPLAY_CHANNEL(channel)) {

ACK


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|