[Spice-devel] [spice-gtk v2] gtk-session: do not request guest's clipboard data unnecessarily

Victor Toso victortoso at redhat.com
Mon Jan 7 12:41:10 UTC 2019


From: Victor Toso <me at victortoso.com>

If SpiceGtkSession is holding the keyboard, that's huge indication
that the client should not be requesting guest's clipboard data yet.

This patch adds a check in clipboard_get() callback, to avoid such
requests. In Linux, this only happens with X11 backend.

This patch helps to handle a possible state race between who owns the
grab between client and agent which could lead to agent clipboard
failing or getting stuck, see:

Linux guest:
    https://gitlab.freedesktop.org/spice/linux/vd_agent/issues/9

Windows guest:
    https://gitlab.freedesktop.org/spice/win32/vd_agent/issues/6

The way to reproduce the race might depend on guest system and
applications but it is connected to amount of VDAGENTD_CLIPBOARD_GRAB
sent by the agent which depends on the amount of clipboard changes in
the guest. Simple example is on RHEL 6.10, with Gedit, select a text
char by char; Client receives VDAGENTD_CLIPBOARD_GRAB every time a new
char is selected instead of once when the full message is selected.

v1 -> v2:
* Moved the check to the right place, otherwise the patch would not
  work on Wayland (Christophe, Jakub)
* Improve commit log (Jakub)

Related: https://gitlab.freedesktop.org/spice/win32/vd_agent/issues/6
Related: https://gitlab.freedesktop.org/spice/linux/vd_agent/issues/9
Related: https://bugzilla.redhat.com/show_bug.cgi?id=1594876

Signed-off-by: Victor Toso <victortoso at redhat.com>
---
 src/spice-gtk-session.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
index 1ccae07..a78f619 100644
--- a/src/spice-gtk-session.c
+++ b/src/spice-gtk-session.c
@@ -59,6 +59,7 @@ struct _SpiceGtkSessionPrivate {
     gboolean                clip_hasdata[CLIPBOARD_LAST];
     gboolean                clip_grabbed[CLIPBOARD_LAST];
     gboolean                clipboard_by_guest[CLIPBOARD_LAST];
+    gboolean                clipboard_by_guest_released[CLIPBOARD_LAST];
     /* auto-usbredir related */
     gboolean                auto_usbredir_enable;
     int                     auto_usbredir_reqs;
@@ -634,6 +635,12 @@ static void clipboard_owner_change(GtkClipboard        *clipboard,
     if (s->main == NULL)
         return;
 
+    if (s->clipboard_by_guest_released[selection]) {
+        /* Ignore owner-change event if this is just about agent releasing grab */
+        s->clipboard_by_guest_released[selection] = FALSE;
+        return;
+    }
+
     if (s->clip_grabbed[selection]) {
         s->clip_grabbed[selection] = FALSE;
         if (spice_main_channel_agent_test_capability(s->main, VD_AGENT_CAP_CLIPBOARD_BY_DEMAND))
@@ -728,6 +735,14 @@ static void clipboard_get(GtkClipboard *clipboard,
     g_return_if_fail(info < SPICE_N_ELEMENTS(atom2agent));
     g_return_if_fail(s->main != NULL);
 
+    /* No real need to set clipboard data while no client application will
+     * be requested. This is useful for clients on X11 as in Wayland, this
+     * callback is only called when SpiceGtkSession:keyboard-focus is false */
+    if (spice_gtk_session_get_keyboard_has_focus(self)) {
+        SPICE_DEBUG("Do not request clipboard data while holding the keyboard focus");
+        return;
+    }
+
     ri.selection_data = selection_data;
     ri.info = info;
     ri.loop = g_main_loop_new(NULL, FALSE);
@@ -769,6 +784,15 @@ cleanup:
 
 static void clipboard_clear(GtkClipboard *clipboard, gpointer user_data)
 {
+    SpiceGtkSession *self = user_data;
+    SpiceGtkSessionPrivate *s = self->priv;
+    gint selection = get_selection_from_clipboard(s, clipboard);
+
+    g_return_if_fail(selection != -1);
+
+    if (s->clipboard_by_guest_released[selection])
+        return;
+
     SPICE_DEBUG("clipboard_clear");
     /* We watch for clipboard ownership changes and act on those, so we
        don't need to do anything here */
@@ -1035,6 +1059,8 @@ static void clipboard_release(SpiceMainChannel *main, guint selection,
 
     if (!s->clipboard_by_guest[selection])
         return;
+
+    s->clipboard_by_guest_released[selection] = TRUE;
     gtk_clipboard_clear(clipboard);
     s->clipboard_by_guest[selection] = FALSE;
 }
-- 
2.20.1



More information about the Spice-devel mailing list