[virt-tools-list] [virt-viewer] Don't unref 'display' in VirtViewerDisplaySpice::finalize

[Fabiano Fidêncio]

On Mon, 2014-09-22 at 16:56 +0200, Christophe Fergeau wrote:
> Right after being created, the SpiceDisplay 'display' private member is
> added to the VirtViewerDisplaySpice GTK+ container with
> gtk_container_add. This call will take ownership of the floating
> reference that SpiceDisplay got upon creation.
> This means VirtViewerDisplaySpice::display is a pointer to SpiceDisplay,
> but it must not be unref'ed when the object is destroyed as we don't own
> that reference.
> As the container which owns the reference is the
> VirtViewerDisplaySpice instance itself, we don't need to take an
> additional reference here.
> 
> This fixes a crash when exiting remote-viewer after connecting to a
> SPICE VM:
>   #0  0x00007ffff3f33a81 in g_type_check_instance_is_fundamentally_a (type_instance=0x874500, fundamental_type=80) at gtype.c:3981
>   #1  0x00007ffff3f19f96 in g_object_unref (_object=0x874500) at gobject.c:3067
>   #2  0x000000000042a1ea in virt_viewer_display_spice_finalize (obj=0x6ebc30) at virt-viewer-display-spice.c:65
>   #3  0x00007ffff3f1a257 in g_object_unref (_object=0x6ebc30) at gobject.c:3170
>   #4  0x0000000000428de7 in destroy_display (data=0x6ebc30) at virt-viewer-session-spice.c:649
>   #5  0x00007ffff3bbb51b in g_ptr_array_foreach (array=0x7e12a0, func=0x428d71 <destroy_display>, user_data=0x0) at garray.c:1502
>   #6  0x00007ffff3bbaadf in ptr_array_free (array=0x7e12a0, flags=FREE_SEGMENT) at garray.c:1088
>   #7  0x00007ffff3bbaa10 in g_ptr_array_unref (array=0x7e12a0) at garray.c:1036
>   #8  0x00007ffff3bcf9bd in g_data_set_internal (datalist=0xa0adb0, key_id=1622, new_data=0x0, new_destroy_func=0x0, dataset=0x0) at gdataset.c:407
>   #9  0x00007ffff3bcfe74 in g_datalist_id_set_data_full (datalist=0xa0adb0, key_id=1622, data=0x0, destroy_func=0x0) at gdataset.c:670
>   #10 0x00007ffff3f1a771 in g_object_set_data (object=0xa0ada0, key=0x437252 "virt-viewer-displays", data=0x0) at gobject.c:3461
>   #11 0x0000000000429b56 in virt_viewer_session_spice_channel_destroy (s=0x6eb910, channel=0xa0ada0, session=0x8cb3a0) at virt-viewer-session-spice.c:854
>   #12 0x00007ffff3f12d81 in g_cclosure_marshal_VOID__OBJECT (closure=0x8e8fd0, return_value=0x0, n_param_values=2, param_values=0x7fffffffcd80, invocation_hint=0x7fffffffccc0, marshal_data=0x0) at gmarshal.c:1272
>   #13 0x00007ffff3f0e143 in g_closure_invoke (closure=0x8e8fd0, return_value=0x0, n_param_values=2, param_values=0x7fffffffcd80, invocation_hint=0x7fffffffccc0) at gclosure.c:768
>   #14 0x00007ffff3f2aef0 in signal_emit_unlocked_R (node=0x7c1f20, detail=0, instance=0x6eb910, emission_return=0x0, instance_and_params=0x7fffffffcd80) at gsignal.c:3553
>   #15 0x00007ffff3f2a1f3 in g_signal_emit_valist (instance=0x6eb910, signal_id=219, detail=0, var_args=0x7fffffffd058) at gsignal.c:3309
>   #16 0x00007ffff3f2a746 in g_signal_emit (instance=0x6eb910, signal_id=219, detail=0) at gsignal.c:3365
>   #17 0x00007ffff529d784 in spice_session_channel_destroy (session=0x6eb910, channel=0xa0ada0) at spice-session.c:1990
>   #18 0x00007ffff529ed25 in spice_channel_dispose (gobject=0xa0ada0) at spice-channel.c:153
>   #19 0x00007ffff52acd26 in spice_display_channel_dispose (object=0xa0ada0) at channel-display.c:136
>   #20 0x00007ffff3f1a132 in g_object_unref (_object=0xa0ada0) at gobject.c:3133
>   #21 0x00007ffff52a4afb in spice_channel_delayed_unref (data=0xa0ada0) at spice-channel.c:2156
>   #22 0x00007ffff3bf21d1 in g_idle_dispatch (source=0xa35a00, callback=0x7ffff52a49f3 <spice_channel_delayed_unref>, user_data=0xa0ada0) at gmain.c:5320
>   #23 0x00007ffff3bef8eb in g_main_dispatch (context=0x68a920) at gmain.c:3064
>   #24 0x00007ffff3bf0661 in g_main_context_dispatch (context=0x68a920) at gmain.c:3663
>   #25 0x00007ffff3bf0853 in g_main_context_iterate (context=0x68a920, block=1, dispatch=1, self=0x6c8c60) at gmain.c:3734
>   #26 0x00007ffff3bf0c7c in g_main_loop_run (loop=0x889b20) at gmain.c:3928
>   #27 0x00007ffff69be44f in gtk_main () at gtkmain.c:1207
>   #28 0x0000000000431896 in main (argc=1, argv=0x7fffffffd648) at remote-viewer-main.c:183

Didn't get the crash here, but got a warning, solved by this patch.
ACK!

> ---
>  src/virt-viewer-display-spice.c | 13 -------------
>  1 file changed, 13 deletions(-)
> 
> diff --git a/src/virt-viewer-display-spice.c b/src/virt-viewer-display-spice.c
> index e568cfb..6cd8394 100644
> --- a/src/virt-viewer-display-spice.c
> +++ b/src/virt-viewer-display-spice.c
> @@ -58,22 +58,9 @@ static void virt_viewer_display_spice_close(VirtViewerDisplay *display G_GNUC_UN
>  static gboolean virt_viewer_display_spice_selectable(VirtViewerDisplay *display);
>  
>  static void
> -virt_viewer_display_spice_finalize(GObject *obj)
> -{
> -    VirtViewerDisplaySpice *spice = VIRT_VIEWER_DISPLAY_SPICE(obj);
> -
> -    g_object_unref(spice->priv->display);
> -
> -    G_OBJECT_CLASS(virt_viewer_display_spice_parent_class)->finalize(obj);
> -}
> -
> -static void
>  virt_viewer_display_spice_class_init(VirtViewerDisplaySpiceClass *klass)
>  {
>      VirtViewerDisplayClass *dclass = VIRT_VIEWER_DISPLAY_CLASS(klass);
> -    GObjectClass *oclass = G_OBJECT_CLASS(klass);
> -
> -    oclass->finalize = virt_viewer_display_spice_finalize;
>  
>      dclass->send_keys = virt_viewer_display_spice_send_keys;
>      dclass->get_pixbuf = virt_viewer_display_spice_get_pixbuf;