[Spice-devel] [spice-gtk] Remove channels from SpiceSession::channels on session disconnect

[Christophe Fergeau]

spice_session_disconnect gets rid of the reference it owns on
the SpiceChannels listed in SpiceSession::channels. However, it does
not remove them from that list, which would cause multiple calls to
spice_session_disconnect to remove references it doesn't own, which
will cause memory management issues later on.

This patch removes the channels from SpiceSession::channels after they
are unref'ed to ensure the reference SpiceSession owns will not be
removed multiple times.

It would be cleaner to clear the SpiceChannel::session when it's removed
from the session, but client applications (virt-viewer)/other spice-gtk
code do not deal well with SpiceChannel::session getting set to NULL
when a disconnect event happens.
---
Hey,

I've experimented with that as well, and it seems that this patch would solve
that issue with using spice-gtk from python/... (haven't tested virt-manager at
all), and would cause less issues with existing code, what do you think?

Christophe


 gtk/spice-session.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/gtk/spice-session.c b/gtk/spice-session.c
index c44a3e1..df71eb7 100644
--- a/gtk/spice-session.c
+++ b/gtk/spice-session.c
@@ -1650,8 +1650,12 @@ void spice_session_disconnect(SpiceSession *session)
     for (ring = ring_get_head(&s->channels); ring != NULL; ring = next) {
         next = ring_next(&s->channels, ring);
         item = SPICE_CONTAINEROF(ring, struct channel, link);
-        spice_channel_destroy(item->channel); /* /!\ item and channel are destroy() after this call */
+        ring_remove(&item->link);
+        spice_channel_disconnect(item->channel, SPICE_CHANNEL_NONE);
+        g_object_unref(item->channel);
+        free(item);
     }
+    g_warn_if_fail(ring_is_empty(&s->channels));
 
     s->connection_id = 0;
 
@@ -1940,7 +1944,7 @@ void spice_session_channel_destroy(SpiceSession *session, SpiceChannel *channel)
 {
     SpiceSessionPrivate *s = session->priv;
     struct channel *item = NULL;
-    RingItem *ring;
+    RingItem *ring = NULL;
 
     g_return_if_fail(s != NULL);
     g_return_if_fail(channel != NULL);
@@ -1955,15 +1959,16 @@ void spice_session_channel_destroy(SpiceSession *session, SpiceChannel *channel)
             break;
     }
 
-    g_return_if_fail(ring != NULL);
 
     if (channel == s->cmain) {
         CHANNEL_DEBUG(channel, "the session lost the main channel");
         s->cmain = NULL;
     }
 
-    ring_remove(&item->link);
-    free(item);
+    if (ring != NULL) {
+        ring_remove(&item->link);
+        free(item);
+    }
 
     g_signal_emit(session, signals[SPICE_SESSION_CHANNEL_DESTROY], 0, channel);
 }
-- 
2.1.0