[virt-tools-list] [PATCH virt-viewer] Ensure windows are destroyed when display closes

[Marc-André Lureau]

Update with a small change:

It could happen that dispose() is called multiple time for cyclic-reference breakage. It's therefore good practice to set the member to NULL after being unref.

----- Mensaje original -----
> From: "Daniel P. Berrange" <berrange at redhat.com>
> 
> When running virt-viewer with the --reconnect argument, when
> the session closes, the VirtViewerWindow instances were being
> freed, but not the GtkWindow itself. So the orphaned window
> stayed around doing nothing. The GtkBuilder instance was also
> leaked.
> 
> Fix these two leaks & also add some debugging to help future
> troubleshooting
> ---
>  src/virt-viewer-app.c           |    2 ++
>  src/virt-viewer-display-spice.c |    2 +-
>  src/virt-viewer-session-spice.c |    4 ++++
>  src/virt-viewer-window.c        |    5 +++++
>  4 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
> index 46af0f8..82149dc 100644
> --- a/src/virt-viewer-app.c
> +++ b/src/virt-viewer-app.c
> @@ -517,6 +517,7 @@ virt_viewer_app_remove_nth_window(VirtViewerApp
> *self, gint nth)
>      win = virt_viewer_app_get_nth_window(self, nth);
>      g_return_val_if_fail(win != NULL, FALSE);
>  
> +    DEBUG_LOG("Remove window %d %p", nth, win);
>      removed = g_hash_table_steal(self->priv->windows, &nth);
>      g_warn_if_fail(removed);
>  
> @@ -536,6 +537,7 @@ virt_viewer_app_set_nth_window(VirtViewerApp
> *self, gint nth, VirtViewerWindow *
>      g_return_if_fail(virt_viewer_app_get_nth_window(self, nth) ==
>      NULL);
>      key = g_malloc(sizeof(gint));
>      *key = nth;
> +    DEBUG_LOG("Insert window %d %p", nth, win);
>      g_hash_table_insert(self->priv->windows, key, win);
>      virt_viewer_app_set_window_subtitle(self, win, nth);
>  
> diff --git a/src/virt-viewer-display-spice.c
> b/src/virt-viewer-display-spice.c
> index 985e116..f7bb26d 100644
> --- a/src/virt-viewer-display-spice.c
> +++ b/src/virt-viewer-display-spice.c
> @@ -110,7 +110,7 @@ display_mark(SpiceChannel *channel G_GNUC_UNUSED,
>               gint mark,
>               VirtViewerDisplay *display)
>  {
> -    DEBUG_LOG("display mark %d", mark);
> +    DEBUG_LOG("Toggle monitor visibility %p %d", channel, mark);
>  
>      virt_viewer_display_set_show_hint(display, mark);
>  }
> diff --git a/src/virt-viewer-session-spice.c
> b/src/virt-viewer-session-spice.c
> index 361e413..d11d7a1 100644
> --- a/src/virt-viewer-session-spice.c
> +++ b/src/virt-viewer-session-spice.c
> @@ -427,6 +427,8 @@
> virt_viewer_session_spice_channel_new(SpiceSession *s,
>  
>      g_object_get(channel, "channel-id", &id, NULL);
>  
> +    DEBUG_LOG("New spice channel %p %s %d", channel,
> g_type_name(G_OBJECT_TYPE(channel)), id);
> +
>      if (SPICE_IS_MAIN_CHANNEL(channel)) {
>          if (self->priv->main_channel != NULL)
>              g_signal_handlers_disconnect_by_func(self->priv->main_channel,
> @@ -522,6 +524,8 @@
> virt_viewer_session_spice_channel_destroy(G_GNUC_UNUSED SpiceSession
> *s,
>      g_return_if_fail(self != NULL);
>  
>      g_object_get(channel, "channel-id", &id, NULL);
> +    DEBUG_LOG("Destroy SPICE channel %s %d",
> g_type_name(G_OBJECT_TYPE(channel)), id);
> +
>      if (SPICE_IS_MAIN_CHANNEL(channel)) {
>          DEBUG_LOG("zap main channel");
>          if (channel == SPICE_CHANNEL(self->priv->main_channel))
> diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c
> index 771a8b9..58aea0f 100644
> --- a/src/virt-viewer-window.c
> +++ b/src/virt-viewer-window.c
> @@ -179,6 +179,11 @@ virt_viewer_window_dispose (GObject *object)
>          priv->display = NULL;
>      }
>  
> +    DEBUG_LOG("Disposing window %p\n", object);
> +
> +    gtk_widget_destroy(priv->window);
> +    g_object_unref(priv->builder);
> +
>      g_free(priv->subtitle);
>      priv->subtitle = NULL;
>  }
> --
> 1.7.10
> 
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list at redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list
>