[Spice-devel] [PATCH spice-gtk 1/2] session: remove channels on disconnect

[Marc-André Lureau]

Hi

I already have patches fixing this. And yes, it will crash with newer spice-gtk if virt-viewer doesn't have the fix.

(if we don't have this kind of fix, we will keep crashing in virt-manager, and I yet have to check Boxes behaviour)

In any case, the current channel lifecycle model is broken by design, so we should fix it even if it brings temporarily this kind of runtime crashes.

----- Original Message -----
> Hey,
> 
> On Sun, Nov 09, 2014 at 10:00:38PM +0100, Marc-André Lureau wrote:
> > A channel is considered to be part of a session as long as it is
> > alive. However, this model is problematic, since library user may hold
> > channel references, and thus the channel will remain in the
> > session. Calling spice_session_disconnect() several time will end up
> > calling spice_channel_destroy(), releasing references that aren't owned
> > by the session. This usually causes crashes, in particular with language
> > bindings that do not deal well with a library model where object can't
> > be referenced at will.
> 
> With this patch applied, connecting to $vm with virt-viewer, and then
> shutting it down with virsh destroy $vm causes several complaints from
> valgrind, for example:
> ==22451== Invalid write of size 4
> ==22451==    at 0x5D3C43B: spice_session_disconnect (spice-session.c:1664)
> ==22451==    by 0x428A69: virt_viewer_session_spice_main_channel_event
> (virt-viewer-session-spice.c:507)
> ==22451==    by 0x389C61249A: g_cclosure_marshal_VOID__ENUMv (gmarshal.c:706)
> ==22451==    by 0x389C60FF63: _g_closure_invoke_va (gclosure.c:831)
> ==22451==    by 0x389C629B6F: g_signal_emit_valist (gsignal.c:3218)
> ==22451==    by 0x5D47C28: emit_main_context (gio-coroutine.c:200)
> ==22451==    by 0x389BA49AFA: g_main_dispatch (gmain.c:3111)
> ==22451==    by 0x389BA49AFA: g_main_context_dispatch (gmain.c:3710)
> ==22451==    by 0x389BA49E97: g_main_context_iterate.isra.29 (gmain.c:3781)
> ==22451==    by 0x389BA4A1C1: g_main_loop_run (gmain.c:3975)
> ==22451==    by 0x33A7BEBC84: gtk_main (gtkmain.c:1207)
> ==22451==    by 0x430360: main (virt-viewer-main.c:119)
> ==22451==  Address 0xea72e7c is 172 bytes inside a block of size 440 free'd
> ==22451==    at 0x4A07CE9: free (in
> /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==22451==    by 0x389BA4F79E: g_free (gmem.c:190)
> ==22451==    by 0x389BA66BC3: g_slice_free1 (gslice.c:1112)
> ==22451==    by 0x389C631D01: g_type_free_instance (gtype.c:1929)
> ==22451==    by 0x5D3D582: spice_session_channel_destroy
> (spice-session.c:2011)
> ==22451==    by 0x5D3C427: spice_session_disconnect (spice-session.c:1661)
> ==22451==    by 0x428A69: virt_viewer_session_spice_main_channel_event
> (virt-viewer-session-spice.c:507)
> ==22451==    by 0x389C61249A: g_cclosure_marshal_VOID__ENUMv (gmarshal.c:706)
> ==22451==    by 0x389C60FF63: _g_closure_invoke_va (gclosure.c:831)
> ==22451==    by 0x389C629B6F: g_signal_emit_valist (gsignal.c:3218)
> ==22451==    by 0x5D47C28: emit_main_context (gio-coroutine.c:200)
> ==22451==    by 0x389BA49AFA: g_main_dispatch (gmain.c:3111)
> ==22451==    by 0x389BA49AFA: g_main_context_dispatch (gmain.c:3710)
> 
> I don't know if only 2/2 is known to have issue or if it's the whole series.
> 
> Christophe
>