<div dir="ltr">ack<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Mar 30, 2015 at 3:08 PM, Christophe Fergeau <span dir="ltr"><<a href="mailto:cfergeau@redhat.com" target="_blank">cfergeau@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">When using virt-viewer --reconnect, virt-viewer currently crashes when<br>
a SPICE VM is destroyed with "virsh destroy"<br>
<br>
What happens is that when the guest is destroyed, virt-viewer receives a<br>
SPICE_CHANNEL_ERROR_IO notification in<br>
virt_viewer_session_spice_main_channel_event().  This triggers the<br>
emission of the "session-disconnected" signal, which will end up calling<br>
spice_session_disconnect() (indirectly through<br>
virt_viewer_app_disconnected/virt_viewer_app_deactivate).<br>
<br>
Since spice-gtk commit ff25f3e, the actual session disconnection is<br>
done from an idle.  When the "session-disconnected" emission stops, the<br>
VirtViewerSession instance is destroyed. However, the associated<br>
VirtViewerDisplaySpice are still alive as the various SpiceChannels<br>
instances hold a reference through the "virt-viewer-displays" GObject<br>
data.<br>
These channels are destroyed when the idle queued by spice_session_disconnect()<br>
run. The associated VirtViewerDisplay are in turn destroyed too, but<br>
this causes attempts to use the VirtViewerSession associated with the<br>
displays, which has already been destroyed. This causes a crash.<br>
<br>
This commit adds a virt_viewer_session_spice_clear_displays() which is<br>
similar to virt_viewer_session_clear_displays(), but makes sure the<br>
"virt-viewer-displays" references are dropped too. This ensures the<br>
VirtViewerDisplay instances don't outlive the VirtViewerSession<br>
they are associated with.<br>
<br>
Backtrace for the crash:<br>
<br>
 #0  0x0000000000413f0f in display_show_hint (display=0x85ab50 [VirtViewerDisplaySpice], pspec=0x939bd0 [GParamFlags], user_data=0x0) at virt-viewer-app.c:949<br>
 #4  0x00000031ff22a29f in <emit signal notify:show-hint on instance 0x85ab50 [VirtViewerDisplaySpice]> (instance=instance@entry=0x85ab50, signal_id=<optimized out>, detail=<optimized out>) at gsignal.c:3361<br>
     #1  0x00000031ff20fc45 in g_closure_invoke (closure=0xa98f50, return_value=return_value@entry=0x0, n_param_values=2, param_values=param_values@entry=0x7fffffffc700, invocation_hint=invocation_hint@entry=0x7fffffffc680) at gclosure.c:768<br>
     #2  0x00000031ff2214c9 in signal_emit_unlocked_R (node=node@entry=0x674f80, detail=detail@entry=1678, instance=instance@entry=0x85ab50, emission_return=emission_return@entry=0x0, instance_and_params=instance_and_params@entry=0x7fffffffc700) at gsignal.c:3549<br>
     #3  0x00000031ff229ed0 in g_signal_emit_valist (instance=<optimized out>, signal_id=<optimized out>, detail=<optimized out>, var_args=var_args@entry=0x7fffffffc8d0) at gsignal.c:3305<br>
 #5  0x00000031ff214175 in g_object_dispatch_properties_changed (object=0x85ab50 [VirtViewerDisplaySpice], n_pspecs=<optimized out>, pspecs=<optimized out>) at gobject.c:1056<br>
 #6  0x00000031ff216661 in g_object_notify (pspec=0x939bd0 [GParamFlags], object=0x85ab50 [VirtViewerDisplaySpice]) at gobject.c:1149<br>
 #7  0x00000031ff216661 in g_object_notify (object=0x85ab50 [VirtViewerDisplaySpice], property_name=<optimized out>) at gobject.c:1197<br>
 #8  0x000000000041e5ab in virt_viewer_display_set_show_hint (self=0x85ab50 [VirtViewerDisplaySpice], mask=1, enable=0) at virt-viewer-display.c:691<br>
 #9  0x000000000042b62d in update_display_ready (self=0x85ab50 [VirtViewerDisplaySpice])<br>
     at virt-viewer-display-spice.c:145<br>
 #13 0x00000031ff22a29f in <emit signal notify:ready on instance 0x898590 [SpiceDisplay]> (instance=instance@entry=0x898590, signal_id=<optimized out>, detail=<optimized out>) at gsignal.c:3361<br>
     #10 0x00000031ff20fc45 in g_closure_invoke (closure=0x99b280, return_value=return_value@entry=0x0, n_param_values=2, param_values=param_values@entry=0x7fffffffcc50, invocation_hint=invocation_hint@entry=0x7fffffffcbd0) at gclosure.c:768<br>
     #11 0x00000031ff2214c9 in signal_emit_unlocked_R (node=node@entry=0x674f80, detail=detail@entry=1696, instance=instance@entry=0x898590, emission_return=emission_return@entry=0x0, instance_and_params=instance_and_params@entry=0x7fffffffcc50) at gsignal.c:3549<br>
     #12 0x00000031ff229ed0 in g_signal_emit_valist (instance=<optimized out>, signal_id=<optimized out>, detail=<optimized out>, var_args=var_args@entry=0x7fffffffce20) at gsignal.c:3305<br>
 #14 0x00000031ff214175 in g_object_dispatch_properties_changed (object=0x898590 [SpiceDisplay], n_pspecs=<optimized out>, pspecs=<optimized out>) at gobject.c:1056<br>
 #15 0x00000031ff216661 in g_object_notify (pspec=0xa83370 [GParamBoolean], object=0x898590 [SpiceDisplay]) at gobject.c:1149<br>
 #16 0x00000031ff216661 in g_object_notify (object=0x898590 [SpiceDisplay], property_name=<optimized out>) at gobject.c:1197<br>
 #17 0x00007ffff7522525 in update_ready (display=0x898590 [SpiceDisplay]) at spice-widget.c:236<br>
 #18 0x00007ffff752257e in set_monitor_ready (self=0x898590 [SpiceDisplay], ready=0)<br>
     at spice-widget.c:244<br>
 #19 0x00007ffff75274e6 in primary_destroy (channel=0x89f5c0 [SpiceDisplayChannel], data=0x898590)<br>
     at spice-widget.c:2169<br>
 #20 0x00007ffff7528918 in channel_destroy (s=0x909fa0 [SpiceSession], channel=0x89f5c0 [SpiceDisplayChannel], data=0x898590) at spice-widget.c:2484<br>
 #24 0x00000031ff22a29f in <emit signal ??? on instance 0x909fa0 [SpiceSession]> (instance=<optimized out>, signal_id=<optimized out>, detail=<optimized out>) at gsignal.c:3361<br>
     #21 0x00000031ff20fc45 in g_closure_invoke (closure=0xa9bda0, return_value=return_value@entry=0x0, n_param_values=2, param_values=param_values@entry=0x7fffffffd280, invocation_hint=invocation_hint@entry=0x7fffffffd200) at gclosure.c:768<br>
     #22 0x00000031ff2214c9 in signal_emit_unlocked_R (node=node@entry=0x9c17d0, detail=detail@entry=0, instance=instance@entry=0x909fa0, emission_return=emission_return@entry=0x0, instance_and_params=instance_and_params@entry=0x7fffffffd280) at gsignal.c:3549<br>
     #23 0x00000031ff229ed0 in g_signal_emit_valist (instance=<optimized out>, signal_id=<optimized out>, detail=<optimized out>, var_args=var_args@entry=0x7fffffffd450) at gsignal.c:3305<br>
 #25 0x00007ffff71c3248 in spice_session_channel_destroy (session=0x909fa0 [SpiceSession], channel=0x89f5c0 [SpiceDisplayChannel]) at spice-session.c:2217<br>
 #26 0x00007ffff71bd8b2 in session_disconnect (self=0x909fa0 [SpiceSession], keep_main=0)<br>
     at spice-session.c:281<br>
 #27 0x00007ffff71c1b27 in session_disconnect_idle (self=0x909fa0 [SpiceSession]) at spice-session.c:1853<br>
 #28 0x00000031fee4a0ba in g_main_context_dispatch (context=0x6a4400) at gmain.c:3122<br>
 #29 0x00000031fee4a0ba in g_main_context_dispatch (context=context@entry=0x6a4400) at gmain.c:3737<br>
 #30 0x00000031fee4a450 in g_main_context_iterate (context=0x6a4400, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at gmain.c:3808<br>
 #31 0x00000031fee4a772 in g_main_loop_run (loop=0x9890f0) at gmain.c:4002<br>
 #32 0x0000003babc05f75 in gtk_main () at gtkmain.c:1219<br>
 #33 0x000000000043143b in main (argc=1, argv=0x7fffffffda48) at virt-viewer-main.c:12<br>
---<br>
 src/virt-viewer-session-spice.c | 20 ++++++++++++++++++--<br>
 1 file changed, 18 insertions(+), 2 deletions(-)<br>
<br>
diff --git a/src/virt-viewer-session-spice.c b/src/virt-viewer-session-spice.c<br>
index 3398f13..4d504dd 100644<br>
--- a/src/virt-viewer-session-spice.c<br>
+++ b/src/virt-viewer-session-spice.c<br>
@@ -90,6 +90,22 @@ static void virt_viewer_session_spice_smartcard_remove(VirtViewerSession *sessio<br>
 static gboolean virt_viewer_session_spice_fullscreen_auto_conf(VirtViewerSessionSpice *self);<br>
 static void virt_viewer_session_spice_apply_monitor_geometry(VirtViewerSession *self, GdkRectangle *monitors, guint nmonitors);<br>
<br>
+static void virt_viewer_session_spice_clear_displays(VirtViewerSessionSpice *self)<br>
+{<br>
+    SpiceSession *session = self->priv->session;<br>
+    GList *l;<br>
+    GList *channels;<br>
+<br>
+    channels = spice_session_get_channels(session);<br>
+    for (l = channels; l != NULL; l = l->next) {<br>
+        SpiceChannel *channel = SPICE_CHANNEL(l->data);<br>
+<br>
+        g_object_set_data(G_OBJECT(channel), "virt-viewer-displays", NULL);<br>
+    }<br>
+    virt_viewer_session_clear_displays(VIRT_VIEWER_SESSION(self));<br>
+}<br>
+<br>
+<br>
 static void<br>
 virt_viewer_session_spice_get_property(GObject *object, guint property_id,<br>
                                        GValue *value, GParamSpec *pspec)<br>
@@ -303,7 +319,7 @@ virt_viewer_session_spice_close(VirtViewerSession *session)<br>
<br>
     g_object_add_weak_pointer(G_OBJECT(self), (gpointer*)&self);<br>
<br>
-    virt_viewer_session_clear_displays(session);<br>
+    virt_viewer_session_spice_clear_displays(self);<br>
<br>
     if (self->priv->session) {<br>
         spice_session_disconnect(self->priv->session);<br>
@@ -518,7 +534,7 @@ virt_viewer_session_spice_main_channel_event(SpiceChannel *channel G_GNUC_UNUSED<br>
     case SPICE_CHANNEL_CLOSED:<br>
         g_debug("main channel: closed");<br>
         /* Ensure the other channels get closed too */<br>
-        virt_viewer_session_clear_displays(session);<br>
+        virt_viewer_session_spice_clear_displays(self);<br>
         if (self->priv->session)<br>
             spice_session_disconnect(self->priv->session);<br>
         break;<br>
<span class="HOEnZb"><font color="#888888">--<br>
2.3.4<br>
<br>
_______________________________________________<br>
virt-tools-list mailing list<br>
<a href="mailto:virt-tools-list@redhat.com">virt-tools-list@redhat.com</a><br>
<a href="https://www.redhat.com/mailman/listinfo/virt-tools-list" target="_blank">https://www.redhat.com/mailman/listinfo/virt-tools-list</a><br>
</font></span></blockquote></div><br><br clear="all"><br>-- <br><div class="gmail_signature">Marc-André Lureau</div>
</div>