[Spice-devel] [PATCH spice-gtk 10/15] session: remove channels on disconnect

[Marc-André Lureau]

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.
---
 gtk/spice-channel.c      |  5 +----
 gtk/spice-session-priv.h |  1 -
 gtk/spice-session.c      | 16 +++++++++++-----
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
index 154dacb..7c18d87 100644
--- a/gtk/spice-channel.c
+++ b/gtk/spice-channel.c
@@ -152,9 +152,6 @@ static void spice_channel_dispose(GObject *gobject)
 
     CHANNEL_DEBUG(channel, "%s %p", __FUNCTION__, gobject);
 
-    if (c->session)
-        spice_session_channel_destroy(c->session, channel);
-
     spice_channel_disconnect(channel, SPICE_CHANNEL_CLOSED);
 
     if (c->session) {
@@ -2041,7 +2038,7 @@ SpiceChannel *spice_channel_new(SpiceSession *s, int type, int id)
  * spice_channel_destroy:
  * @channel:
  *
- * Disconnect and unref the @channel. Called by @spice_session_disconnect()
+ * Disconnect and unref the @channel. Called by @spice_session_channel_destroy()
  *
  **/
 void spice_channel_destroy(SpiceChannel *channel)
diff --git a/gtk/spice-session-priv.h b/gtk/spice-session-priv.h
index 08757a4..282269a 100644
--- a/gtk/spice-session-priv.h
+++ b/gtk/spice-session-priv.h
@@ -46,7 +46,6 @@ gboolean spice_session_get_client_provided_socket(SpiceSession *session);
 GSocketConnection* spice_session_channel_open_host(SpiceSession *session, SpiceChannel *channel,
                                                    gboolean *use_tls, GError **error);
 void spice_session_channel_new(SpiceSession *session, SpiceChannel *channel);
-void spice_session_channel_destroy(SpiceSession *session, SpiceChannel *channel);
 void spice_session_channel_migrate(SpiceSession *session, SpiceChannel *channel);
 
 void spice_session_set_mm_time(SpiceSession *session, guint32 time);
diff --git a/gtk/spice-session.c b/gtk/spice-session.c
index ee80aa5..35b43e6 100644
--- a/gtk/spice-session.c
+++ b/gtk/spice-session.c
@@ -210,6 +210,8 @@ enum {
 
 static guint signals[SPICE_SESSION_LAST_SIGNAL];
 
+static void spice_session_channel_destroy(SpiceSession *session, SpiceChannel *channel);
+
 static void update_proxy(SpiceSession *self, const gchar *str)
 {
     SpiceSessionPrivate *s = self->priv;
@@ -1464,8 +1466,10 @@ void spice_session_switching_disconnect(SpiceSession *self)
     for (ring = ring_get_head(&s->channels); ring != NULL; ring = next) {
         next = ring_next(&s->channels, ring);
         item = SPICE_CONTAINEROF(ring, struct channel, link);
-        if (item->channel != s->cmain)
-            spice_channel_destroy(item->channel); /* /!\ item and channel are destroy() after this call */
+
+        if (item->channel == s->cmain)
+            continue;
+        spice_session_channel_destroy(self, item->channel);
     }
 
     g_warn_if_fail(!ring_is_empty(&s->channels)); /* ring_get_length() == 1 */
@@ -1744,7 +1748,7 @@ 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 */
+        spice_session_channel_destroy(session, item->channel);
     }
 
     s->connection_id = 0;
@@ -2032,8 +2036,7 @@ void spice_session_channel_new(SpiceSession *session, SpiceChannel *channel)
     g_signal_emit(session, signals[SPICE_SESSION_CHANNEL_NEW], 0, channel);
 }
 
-G_GNUC_INTERNAL
-void spice_session_channel_destroy(SpiceSession *session, SpiceChannel *channel)
+static void spice_session_channel_destroy(SpiceSession *session, SpiceChannel *channel)
 {
     SpiceSessionPrivate *s = session->priv;
     struct channel *item = NULL;
@@ -2063,6 +2066,9 @@ void spice_session_channel_destroy(SpiceSession *session, SpiceChannel *channel)
     free(item);
 
     g_signal_emit(session, signals[SPICE_SESSION_CHANNEL_DESTROY], 0, channel);
+
+    g_clear_object(&channel->priv->session);
+    spice_channel_destroy(channel);
 }
 
 G_GNUC_INTERNAL
-- 
2.1.0