[Spice-commits] 2 commits - gtk/spice-channel.c gtk/spice-session.c

[Marc-André Lureau]

 gtk/spice-channel.c |   27 ++++++++++++---------------
 gtk/spice-session.c |   10 ++++------
 2 files changed, 16 insertions(+), 21 deletions(-)

New commits:
commit 76cb968a0bd495fe7b5a4c17a3d11fbb3577b9eb
Author: Marc-André Lureau <marcandre.lureau at redhat.com>
Date:   Sun Dec 23 22:18:51 2012 +0100

    Fix switching to TLS regression
    
    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.

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;
commit 5ed7f06e7c7a9fa9c6b0411debd43793335068ec
Author: Marc-André Lureau <marcandre.lureau at redhat.com>
Date:   Sun Dec 23 20:56:09 2012 +0100

    Clean-up idle handler when leaving the open_host_idle()
    
    An explicit yield back to the channel coroutine when the idle function
    is still pending will leave it in the background, referencing objects
    that may no longer exist. Make sure we remove it when
    channel_open_host() is resumed.

diff --git a/gtk/spice-session.c b/gtk/spice-session.c
index b18d67b..f1d6250 100644
--- a/gtk/spice-session.c
+++ b/gtk/spice-session.c
@@ -1713,18 +1713,16 @@ GSocket* spice_session_channel_open_host(SpiceSession *session, SpiceChannel *ch
     open_host.channel = channel;
     open_host.port = atoi(use_tls ? s->tls_port : s->port);
     open_host.client = g_socket_client_new();
-    g_idle_add(open_host_idle_cb, &open_host);
 
+    guint id = g_idle_add(open_host_idle_cb, &open_host);
     /* switch to main loop and wait for connection */
     coroutine_yield(NULL);
-    if (open_host.error != NULL) {
-        g_return_val_if_fail(open_host.socket == NULL, NULL);
+    g_source_remove (id);
 
+    if (open_host.error != NULL) {
         g_warning("%s", open_host.error->message);
         g_clear_error(&open_host.error);
-    } else {
-        g_return_val_if_fail(open_host.socket != NULL, NULL);
-
+    } else if (open_host.socket != NULL) {
         g_socket_set_blocking(open_host.socket, FALSE);
         g_socket_set_keepalive(open_host.socket, TRUE);
     }