[Spice-devel] [PATCH spice-gtk 1/9] channel: delay event report to after coroutine exit

[Marc-André Lureau]

----- Original Message -----
> Hey,
> 
> I think the shortlog is a bit misleading, it's only some specific events
> which are reported this way. In particular, SPICE_CHANNEL_ERROR_LINK is
> still reported as before.
> Bit of context as to _why_ this change is needed would be nice.
> 

It's all related to 3rd patch, "channel: reset connection state on error",
we need the channel to be disconnect/reset when reporting errors,
as in the error callback, the client can attempt a reconnect
reusing the same session & channel. So moving all error reporting to
when the coroutine exits is a prerequisite.

> On Tue, Jan 13, 2015 at 05:38:20PM +0100, Marc-André Lureau wrote:
> > Move to a common place error reporting, after the coroutine exits.
> 
> 
> > ---
> >  gtk/spice-channel-priv.h |  3 ++-
> >  gtk/spice-channel.c      | 25 +++++++++++++------------
> >  2 files changed, 15 insertions(+), 13 deletions(-)
> > 
> > diff --git a/gtk/spice-channel-priv.h b/gtk/spice-channel-priv.h
> > index bd7f490..d70cf86 100644
> > --- a/gtk/spice-channel-priv.h
> > +++ b/gtk/spice-channel-priv.h
> > @@ -66,7 +66,6 @@ struct _SpiceMsgIn {
> >  
> >  enum spice_channel_state {
> >      SPICE_CHANNEL_STATE_UNCONNECTED = 0,
> > -    SPICE_CHANNEL_STATE_FAILED_AUTHENTICATION,
> >      SPICE_CHANNEL_STATE_RECONNECTING,
> >      SPICE_CHANNEL_STATE_CONNECTING,
> >      SPICE_CHANNEL_STATE_READY,
> > @@ -110,6 +109,8 @@ struct _SpiceChannelPrivate {
> >  
> >      char                        name[16];
> >      enum spice_channel_state    state;
> > +    SpiceChannelEvent           event;
> > +
> >      spice_parse_channel_func_t  parser;
> >      SpiceMessageMarshallers     *marshallers;
> >      guint                       channel_watch;
> > diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
> > index 0301204..2f0ae1a 100644
> > --- a/gtk/spice-channel.c
> > +++ b/gtk/spice-channel.c
> > @@ -2489,7 +2489,8 @@ cleanup:
> >  
> >      SPICE_CHANNEL_GET_CLASS(channel)->channel_disconnect(channel);
> >  
> > -    if (c->state == SPICE_CHANNEL_STATE_RECONNECTING) {
> > +    if (c->event == SPICE_CHANNEL_NONE &&
> > +        c->state == SPICE_CHANNEL_STATE_RECONNECTING) {
> >          spice_channel_connect(channel);
> >          g_object_unref(channel);
> >      } else
> 
> Is this hunk strictly needed ? The code flow when the error was emitted
> before exiting was not different, and I expect the old code needed to go
> through there before the coroutine exited ?
> 
> Christophe
>