From 76cb968a0bd495fe7b5a4c17a3d11fbb3577b9eb Mon Sep 17 00:00:00 2001 From: Marc-André Lureau Date: Sun, 23 Dec 2012 22:18:51 +0100 Subject: 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. --- 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; -- cgit v1.2.3