[Spice-devel] [PATCH 1/2] client: only send one SPICE_MSGC_MAIN_ATTACH_CHANNELS messages

[Alon Levy]

On Thu, Jul 21, 2011 at 05:43:16PM +0200, Christophe Fergeau wrote:
> 492f7a9b fixed unwanted timeouts during initial client startup,
> but it also caused a bad regression when connecting to
> RHEL6+agent guests: the SPICE_MSGS_MAIN_ATTACH_CHANNELS message
> was sent multiple times, once in RedClient::handle_init, then
> once again in RedClient::on_agent_announce_capabilities (which
> can even be triggered multiple times). Sending this message multiple
> times is a big NO and causes the server to close the client connection,
> and the client to die. Add a _msg_attach_message_sent boolean to
> make sure we only send this message once.

ACK.

> ---
>  client/red_client.cpp |   22 ++++++++++++++--------
>  client/red_client.h   |    2 ++
>  2 files changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/client/red_client.cpp b/client/red_client.cpp
> index 67690e9..4a6d3fd 100644
> --- a/client/red_client.cpp
> +++ b/client/red_client.cpp
> @@ -360,6 +360,7 @@ RedClient::RedClient(Application& application)
>      , _auto_display_res (false)
>      , _agent_reply_wait_type (VD_AGENT_END_MESSAGE)
>      , _aborting (false)
> +    , _msg_attach_channels_sent(false)
>      , _agent_connected (false)
>      , _agent_mon_config_sent (false)
>      , _agent_disp_config_sent (false)
> @@ -970,16 +971,12 @@ void RedClient::handle_init(RedPeer::InMessage* message)
>          if (_auto_display_res) {
>             send_agent_monitors_config();
>          }
> -        if (_auto_display_res || !_display_setting.is_empty()) {
> -            _application.activate_interval_timer(*_agent_timer, AGENT_TIMEOUT);
> -        } else {
> -            post_message(new Message(SPICE_MSGC_MAIN_ATTACH_CHANNELS));
> -        }
> +        _application.activate_interval_timer(*_agent_timer, AGENT_TIMEOUT);
>      } else {
>          if (_auto_display_res || !_display_setting.is_empty()) {
>              LOG_WARN("no agent running, display options have been ignored");
>          }
> -        post_message(new Message(SPICE_MSGC_MAIN_ATTACH_CHANNELS));
> +        send_main_attach_channels();
>      }
>  }
>  
> @@ -1026,6 +1023,15 @@ void RedClient::handle_agent_disconnected(RedPeer::InMessage* message)
>      _agent_connected = false;
>  }
>  
> +void RedClient::send_main_attach_channels(void)
> +{
> +    if (_msg_attach_channels_sent)
> +        return;
> +
> +    post_message(new Message(SPICE_MSGC_MAIN_ATTACH_CHANNELS));
> +    _msg_attach_channels_sent = true;
> +}
> +
>  void RedClient::on_agent_announce_capabilities(
>      VDAgentAnnounceCapabilities* caps, uint32_t msg_size)
>  {
> @@ -1056,7 +1062,7 @@ void RedClient::on_agent_announce_capabilities(
>          if (!_display_setting.is_empty()) {
>              LOG_WARN("display options have been requested, but the agent doesn't support these options");
>          }
> -        post_message(new Message(SPICE_MSGC_MAIN_ATTACH_CHANNELS));
> +        send_main_attach_channels();
>          _application.deactivate_interval_timer(*_agent_timer);
>      }
>  }
> @@ -1076,7 +1082,7 @@ void RedClient::on_agent_reply(VDAgentReply* reply)
>      case VD_AGENT_MONITORS_CONFIG:
>      case VD_AGENT_DISPLAY_CONFIG:
>          if (_agent_reply_wait_type == reply->type) {
> -            post_message(new Message(SPICE_MSGC_MAIN_ATTACH_CHANNELS));
> +            send_main_attach_channels();
>              _application.deactivate_interval_timer(*_agent_timer);
>              _agent_reply_wait_type = VD_AGENT_END_MESSAGE;
>          }
> diff --git a/client/red_client.h b/client/red_client.h
> index 7b04d08..7f5b589 100644
> --- a/client/red_client.h
> +++ b/client/red_client.h
> @@ -262,6 +262,7 @@ public:
>  
>      void set_mm_time(uint32_t time);
>      uint32_t get_mm_time();
> +    void send_main_attach_channels(void);
>  
>  protected:
>      virtual void on_connecting();
> @@ -321,6 +322,7 @@ private:
>      uint32_t _agent_reply_wait_type;
>  
>      bool _aborting;
> +    bool _msg_attach_channels_sent;
>  
>      bool _agent_connected;
>      bool _agent_mon_config_sent;
> -- 
> 1.7.6
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel