[PATCH spice-gtk 2/2] Fix switching to TLS regression

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



The commit fcbbc248a8f885f9a9a6e7c47d7aae0c1ab3cd1b changed the way a
channel coroutine is exiting. In particular, it was going through the
coroutine main cleanup (finishing in main coroutine) while switching
to TLS is recycling the channel. That part of the code is a bit
difficult to grasp, but with this refactoring, I think it makes it
easier to understand the reconnection.
---
 gtk/spice-channel.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
index 369a0a5..14ced89 100644
--- a/gtk/spice-channel.c
+++ b/gtk/spice-channel.c
@@ -1652,7 +1652,7 @@ error:
 #endif /* HAVE_SASL */
 
 /* coroutine context */
-static void spice_channel_recv_link_msg(SpiceChannel *channel)
+static void spice_channel_recv_link_msg(SpiceChannel *channel, gboolean *switch_tls)
 {
     SpiceChannelPrivate *c;
     int rc, num_caps, i;
@@ -1677,10 +1677,8 @@ static void spice_channel_recv_link_msg(SpiceChannel *channel)
         /* nothing */
         break;
     case SPICE_LINK_ERR_NEED_SECURED:
-        c->tls = true;
+        *switch_tls = true;
         CHANNEL_DEBUG(channel, "switching to tls");
-        SPICE_CHANNEL_GET_CLASS(channel)->channel_disconnect(channel);
-        spice_channel_connect(channel);
         return;
     default:
         g_warning("%s: %s: unhandled error %d",
@@ -2175,6 +2173,7 @@ static void *spice_channel_coroutine(void *data)
     SpiceChannelPrivate *c = channel->priv;
     guint verify;
     int rc, delay_val = 1;
+    gboolean switch_tls = FALSE;
 
     CHANNEL_DEBUG(channel, "Started background coroutine %p", &c->coroutine);
 
@@ -2297,28 +2296,26 @@ connected:
 
     spice_channel_send_link(channel);
     spice_channel_recv_link_hdr(channel);
-    spice_channel_recv_link_msg(channel);
+    spice_channel_recv_link_msg(channel, &switch_tls);
+    if (switch_tls)
+        goto cleanup;
     spice_channel_recv_auth(channel);
 
     while (spice_channel_iterate(channel))
         ;
 
-    /* TODO: improve it, this is a bit hairy, c->coroutine will be
-       overwritten on (re)connect, so we skip the normal cleanup
-       path. Ideally, we shouldn't use the same channel structure? */
-    if (c->state == SPICE_CHANNEL_STATE_CONNECTING) {
-        g_object_unref(channel);
-        goto end;
-    }
-
 cleanup:
     CHANNEL_DEBUG(channel, "Coroutine exit %s", c->name);
 
     SPICE_CHANNEL_GET_CLASS(channel)->channel_disconnect(channel);
 
-    g_idle_add(spice_channel_delayed_unref, data);
+    if (switch_tls) {
+        c->tls = true;
+        spice_channel_connect(channel);
+        g_object_unref(channel);
+    } else
+        g_idle_add(spice_channel_delayed_unref, data);
 
-end:
     /* Co-routine exits now - the SpiceChannel object may no longer exist,
        so don't do anything else now unless you like SEGVs */
     return NULL;
-- 
1.8.1.rc1.17.g75ed918

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]