[Spice-devel] [spice-gtk 2/2] Fix switch to old SPICE protocol

[Christophe Fergeau]

After the previous commit, spice_channel_switch_protocol() is now
called when needed, but it's not doing anything. What happens is
that spice_channel_switch_protocol() triggers a channel disconnection
and then it queues an idle to reconnect (after having changed the
protocol version to be used).

When spice_channel_recv_link_hdr() returns, we need to jump out of
the coroutine to let the idle trigger and the new channel coroutine
be started. But jumping out of the coroutine will call channel_disconnect()
which calls channel_reset() which disables the idle switch_protocol()
just queued. This causes the connection attempt to be apparently
stuck with nothing happening.

Falling back to the older SPICE protocol is not the only situation
when we need to drop the current connection attempt and reconnect,
we also need to do that when the remote server returns
SPICE_LINK_ERR_NEED_SECURED to let the client know it needs to use
a secure port for this channel. This is handled by the 'switch_tls'
variable set in spice_channel_recv_link_msg and handled in
spice_channel_coroutine().

'switch_tls' does the same thing as spice_channel_switch_protocol(),
except that it calls spice_channel_connect() after channel_disconnect()
has been called, which means the idle queued by channel_connect()
won't get cleared.

This all that commit does, remove the spice_channel_switch_protocol()
method and use the same codepath as 'switch_tls' to handle the
reconnection.
---
 gtk/spice-channel.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
index c0e7bba..08418f7 100644
--- a/gtk/spice-channel.c
+++ b/gtk/spice-channel.c
@@ -1169,21 +1169,13 @@ static void spice_channel_send_link(SpiceChannel *channel)
     free(buffer);
 }
 
-static void spice_channel_switch_protocol(SpiceChannel *channel, gint version)
-{
-    SpiceChannelPrivate *c = channel->priv;
-
-    g_object_set(c->session, "protocol", version, NULL);
-    SPICE_CHANNEL_GET_CLASS(channel)->channel_disconnect(channel);
-    spice_channel_connect(channel);
-}
-
 /* coroutine context */
-static gboolean spice_channel_recv_link_hdr(SpiceChannel *channel)
+static gboolean spice_channel_recv_link_hdr(SpiceChannel *channel, gboolean *switch_protocol)
 {
     SpiceChannelPrivate *c = channel->priv;
     int rc;
 
+    *switch_protocol = FALSE;
     rc = spice_channel_read(channel, &c->peer_hdr, sizeof(c->peer_hdr));
     if (rc != sizeof(c->peer_hdr)) {
         g_warning("incomplete link header (%d/%" G_GSIZE_FORMAT ")",
@@ -1216,7 +1208,8 @@ error:
        incompatible. Try with the oldest protocol in this case: */
     if (c->link_hdr.major_version != 1) {
         SPICE_DEBUG("%s: error, switching to protocol 1 (spice 0.4)", c->name);
-        spice_channel_switch_protocol(channel, 1);
+        *switch_protocol = TRUE;
+        g_object_set(c->session, "protocol", 1, NULL);
         return FALSE;
     }
 
@@ -2228,6 +2221,7 @@ static void *spice_channel_coroutine(void *data)
     guint verify;
     int rc, delay_val = 1;
     gboolean switch_tls = FALSE;
+    gboolean switch_protocol = FALSE;
 
     CHANNEL_DEBUG(channel, "Started background coroutine %p", &c->coroutine);
 
@@ -2350,7 +2344,7 @@ connected:
     }
 
     spice_channel_send_link(channel);
-    if (spice_channel_recv_link_hdr(channel) == FALSE)
+    if (spice_channel_recv_link_hdr(channel, &switch_protocol) == FALSE)
         goto cleanup;
     spice_channel_recv_link_msg(channel, &switch_tls);
     if (switch_tls)
@@ -2365,8 +2359,8 @@ cleanup:
 
     SPICE_CHANNEL_GET_CLASS(channel)->channel_disconnect(channel);
 
-    if (switch_tls && !c->tls) {
-        c->tls = true;
+    if (switch_protocol || (switch_tls && !c->tls)) {
+        c->tls = switch_tls;
         spice_channel_connect(channel);
         g_object_unref(channel);
     } else
-- 
1.8.3.1