[Spice-devel] [PATCH win-vdagent] Don't exit vdagent when a client disconnects

[Pavel Grunt]

Hi,

looks good to me. How does it work on log out - it seems that in that
case it kills the agent.

Pavel

On Thu, 2017-03-16 at 10:08 -0500, Jonathon Jongsma wrote:
> Previously, when a client disconnects, the vdagent executable
> exited.
> The agent was restarted immediately in order to be ready for the
> next
> client to reconnect.
> 
> This is generally fine, but there are certain situations
> where it causes issues. For example, if client A is connected to a
> guest
> and client B connects to the same guest, client A will be forcibly
> disconnected and causes the the vdagent to restart. This scenario is
> racy because the agent can take some time to exit and restart.
> Client B
> may think that the agent is connected at startup and may send agent
> messages to the guest.  At some point the server will recieve
> notification that the agent has exited and send an
> AGENT_DISCONNECTED
> message to client B. After the agent has been fully restarted, an
> AGENT_CONNECTED message will be sent to the client, but any messages
> sent between client connection and the AGENT_DISCONNECTED message
> will
> be lost. This causes problems especially with fullscreen mode in
> virt-viewer.
> 
> The solution is to not exit and restart the agent when a client
> disconnects. This is how the linux vdagent behaves. Instead, we
> simply
> cancel all ongoing file transfers and reset the clipboard when a
> client
> is disconnected.
> 
> Fixes: rhbz#1064495
> ---
>  vdagent/file_xfer.cpp | 20 +++++++++++++++-----
>  vdagent/file_xfer.h   |  3 +++
>  vdagent/vdagent.cpp   | 11 ++++++++---
>  3 files changed, 26 insertions(+), 8 deletions(-)
> 
> diff --git a/vdagent/file_xfer.cpp b/vdagent/file_xfer.cpp
> index 0e90ebe..f763ed3 100644
> --- a/vdagent/file_xfer.cpp
> +++ b/vdagent/file_xfer.cpp
> @@ -31,17 +31,22 @@
>  #include "file_xfer.h"
>  #include "as_user.h"
>  
> -FileXfer::~FileXfer()
> +void FileXfer::reset()
>  {
>      FileXferTasks::iterator iter;
>      FileXferTask* task;
>  
>      for (iter = _tasks.begin(); iter != _tasks.end(); iter++) {
>          task = iter->second;
> -        CloseHandle(task->handle);
> -        DeleteFile(task->name);
> +        task->cancel();
>          delete task;
>      }
> +    _tasks.clear();
> +}
> +
> +FileXfer::~FileXfer()
> +{
> +    reset();
>  }
>  
>  void FileXfer::handle_start(VDAgentFileXferStartMessage* start,
> @@ -152,6 +157,12 @@ fin:
>      return true;
>  }
>  
> +void FileXferTask::cancel()
> +{
> +    CloseHandle(handle);
> +    DeleteFile(name);
> +}
> +
>  void FileXfer::handle_status(VDAgentFileXferStatusMessage* status)
>  {
>      FileXferTasks::iterator iter;
> @@ -168,8 +179,7 @@ void
> FileXfer::handle_status(VDAgentFileXferStatusMessage* status)
>          return;
>      }
>      task = iter->second;
> -    CloseHandle(task->handle);
> -    DeleteFile(task->name);
> +    task->cancel();
>      _tasks.erase(iter);
>      delete task;
>  }
> diff --git a/vdagent/file_xfer.h b/vdagent/file_xfer.h
> index d9c65f3..25cd5c2 100644
> --- a/vdagent/file_xfer.h
> +++ b/vdagent/file_xfer.h
> @@ -34,6 +34,8 @@ typedef struct ALIGN_VC FileXferTask {
>      uint64_t size;
>      uint64_t pos;
>      TCHAR name[MAX_PATH];
> +
> +    void cancel();
>  } ALIGN_GCC FileXferTask;
>  
>  typedef std::map<uint32_t, FileXferTask*> FileXferTasks;
> @@ -42,6 +44,7 @@ class FileXfer {
>  public:
>      ~FileXfer();
>      bool dispatch(VDAgentMessage* msg,
> VDAgentFileXferStatusMessage* status);
> +    void reset();
>  
>  private:
>      void handle_start(VDAgentFileXferStartMessage* start,
> VDAgentFileXferStatusMessage* status);
> diff --git a/vdagent/vdagent.cpp b/vdagent/vdagent.cpp
> index e3ba14b..c25a077 100644
> --- a/vdagent/vdagent.cpp
> +++ b/vdagent/vdagent.cpp
> @@ -112,7 +112,7 @@ private:
>      DWORD get_cximage_format(uint32_t type) const;
>      enum { owner_none, owner_guest, owner_client };
>      void set_clipboard_owner(int new_owner);
> -    enum { CONTROL_STOP, CONTROL_DESKTOP_SWITCH, CONTROL_LOGON,
> CONTROL_CLIPBOARD };
> +    enum { CONTROL_STOP, CONTROL_RESET, CONTROL_DESKTOP_SWITCH,
> CONTROL_LOGON, CONTROL_CLIPBOARD };
>      void set_control_event(int control_command);
>      void handle_control_event();
>      VDIChunk* new_chunk(DWORD bytes = 0);
> @@ -379,6 +379,11 @@ void VDAgent::handle_control_event()
>          _control_queue.pop();
>          vd_printf("Control command %d", control_command);
>          switch (control_command) {
> +        case CONTROL_RESET:
> +            _file_xfer.reset();
> +            set_control_event(CONTROL_CLIPBOARD);
> +            set_clipboard_owner(owner_none);
> +            break;
>          case CONTROL_STOP:
>              _running = false;
>              break;
> @@ -1321,8 +1326,8 @@ void VDAgent::dispatch_message(VDAgentMessage*
> msg, uint32_t port)
>          break;
>      }
>      case VD_AGENT_CLIENT_DISCONNECTED:
> -        vd_printf("Client disconnected, agent to be restarted");
> -        set_control_event(CONTROL_STOP);
> +        vd_printf("Client disconnected, resetting agent state");
> +        set_control_event(CONTROL_RESET);
>          break;
>      case VD_AGENT_MAX_CLIPBOARD:
>          res = handle_max_clipboard((VDAgentMaxClipboard*)msg->data, 
> msg->size);