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

[Jonathon Jongsma]

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);
-- 
2.9.3