Hey,
First thing, this patch fixes the TLS issues I was seeing with virt-viewer,
so thanks for the quick fix!
On Sun, Dec 23, 2012 at 10:35:34PM +0100, Marc-André Lureau wrote:
> 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;
There are other cases when spice_channel_recv_link_msg can exit early in
error cases. It will have called emit_main_context(channel,
SPICE_CHANNEL_EVENT, SPICE_CHANNEL_ERROR_LINK); before exiting when this
happens. Is it enough to not cause issues when spice_channel_recv_auth is
called after an early return from spice_channel_recv_link_msg?
Apart from this concern, patch seems ok.
Christophe
Attachment:
pgp6zFCsIPU3_.pgp
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel