[Spice-devel] [PATCH] Wait to send monitor config until agent caps are received

[Jonathon Jongsma]

When the first display is disabled and the vdagent is restarted in the guest,
it sometimes becomes enabled. This appears to be caused by a race condition
when an agent becomes connected. When the agent becomes connected, virt-viewer
triggers a display update (spice_main_send_monitor_config()). This display
update happens in a timeout handler, but the timeout interval is set to 0 (so
it behaves basically like an idle handler).

The race happens because spice_main_send_monitor_config() behaves slightly
differently depending on the agent's capabilities. And sometimes the idle
handler runs before the client and server have negotiated capabilities. In this
case, we have to assume that the server does not support sparse monitor
configurations. So instead of sending down an update where display #0 is off
and display #1 is WxH, we send down an update that only a single display:
display #0 is WxH. This results in the first display becoming enabled.

To solve the issue, we trigger a display update when the agent becomes
connected, but give it a long timeout (e.g. 5 seconds). This gives some time
for the agent capabilities negotiation to occur. When we receive the agent
capabilities from the guest, we trigger an immediate display update (cancelling
the previously-scheduled update). If we receive no agent capabilities (perhaps
because we're connected to a very old server that doesn't send capabilities),
the display update will occur after the initial timeout.

Resolves: rhbz#1043782, rhbz#1032923
---

NOTE: the 5-second timeout I've used here is rather arbitrary. I'm open to
modifying this value if somebody has a good argument for a shorter or longer
timeout.

 gtk/channel-main.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/gtk/channel-main.c b/gtk/channel-main.c
index 7a299a4..cb873b7 100644
--- a/gtk/channel-main.c
+++ b/gtk/channel-main.c
@@ -1356,6 +1356,12 @@ static void agent_start(SpiceMainChannel *channel)
         agent_announce_caps(channel);
         agent_send_msg_queue(channel);
     }
+
+    // trigger a display update, but give it a long timeout to allow agent
+    // capabilities to be negotiated. When we receive the capabilities message,
+    // the display update will happen immediately. This long timeout is to
+    // accomodate older servers that may not send capabilities.
+    update_display_timer(SPICE_MAIN_CHANNEL(channel), 5);
 }
 
 /* coroutine context  */
@@ -1508,7 +1514,6 @@ static void main_handle_mouse_mode(SpiceChannel *channel, SpiceMsgIn *in)
 static void main_handle_agent_connected(SpiceChannel *channel, SpiceMsgIn *in)
 {
     agent_start(SPICE_MAIN_CHANNEL(channel));
-    update_display_timer(SPICE_MAIN_CHANNEL(channel), 0);
 }
 
 /* coroutine context */
@@ -1519,7 +1524,6 @@ static void main_handle_agent_connected_tokens(SpiceChannel *channel, SpiceMsgIn
 
     c->agent_tokens = msg->num_tokens;
     agent_start(SPICE_MAIN_CHANNEL(channel));
-    update_display_timer(SPICE_MAIN_CHANNEL(channel), 0);
 }
 
 /* coroutine context */
@@ -1790,6 +1794,7 @@ static void main_agent_handle_msg(SpiceChannel *channel,
         }
         c->agent_caps_received = true;
         g_coroutine_signal_emit(self, signals[SPICE_MAIN_AGENT_UPDATE], 0);
+        update_display_timer(SPICE_MAIN_CHANNEL(channel), 0);
 
         if (caps->request)
             agent_announce_caps(self);
-- 
1.9.3