From: Victor Toso <me@xxxxxxxxxxxxxx>
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@xxxxxxxxxx>
---
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
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel