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

[james harvey]

On Mon, Jan 7, 2019 at 7:41 AM Victor Toso <victortoso at redhat.com> wrote:
>
> 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

(I'm the reporter of this linux issue.)  I appreciate your ongoing work on this.

v1 of your patch works extremely well for me.  I've been running it
since Dec 14.

I just tried v2, on top of git-gtk dbdf692.  I immediately had many
problems.  Attached are explanations and links to "remote-viewer
--spice-debug" logs.

I certainly don't know the internals of spice, but my general feeling
is there's a couple of things going on:
(1) "clipboard get" runs 2-4 times per CTRL+C or CTRL+V which seems odd to me
(2) Sometimes on a paste, "clipboard_clear" is ran.
(3) A spice client open but without focus (i.e. guest booted to Win 7,
and completely thrown off to the side and ignored) is still running
"clipboard_get_targets" when in the other client the clipboard is
changed by CTRL+C or CTRL+V, but without leaving its focus.
(4) Something (maybe v2 clipboard problems) completely crashed one of
my spice clients.  I haven't seen this happen before.


If I have a single spice client open, it works the majority of the
time, but not as well as v1.

When it usually works: https://termbin.com/0m7d
* From a single CTRL+V, 2 sets of "clipboard get" and "Do not
request..."  Not sure why there are 2 rather than 1.

When it sometimes fails: https://termbin.com/dcnc
* From a single CTRL+V, for some reason "clipboard_clear",
"clipboard_get_targets", which sets up the next paste as a ticking
timebomb of lagging the system for about 10 seconds.


If I have more than one spice client open, it acts about as badly as
without any patch.  Both problems I've detailed come back: copying a
cell in Excel, and repeatedly pasting it into different cells; and
copying a cell, pasting it into a different one, re-copying the new
cell, pasting it into a different one, and repeating.  By additional
clients being "open", I mean as soon as it starts, even before the
guest OS boots up and starts spice tools, and it stops the moment the
additional clients fully close.  For these logs, I was running two
instances of "remote-viewer --spice-debug", and I've made a comment
saying what I did, then show the log from that interaction from me,
from the viewer I was using followed by the one I wasn't.  (6659 is
always the one I was interacting with, and 12652/16746 is the one I
wasn't.)

When it rarely works: https://termbin.com/awsw
* From a single CTRL+C:
** On the client being used, 4 sets of "clipboard get", showing "Do
not request..."
** On the client not being used, "inputs", "clipboard_get_targets",
and "Failed to retrieve clipboard text"
* From a single CTRL+V
** On the client being used, 4 sets of "clipboard get", showing "Do
not request..."
** On the client not being used, "inputs", "clipboard_get_targets",
"Retrieving the clipboard data has failed", "Failed to retrieve
clipboard text"

When it usually fails - Failure type "A": https://termbin.com/te1g
* From a single CTRL+C:
** On the client being used, 4 sets of "clipboard get", showing "Do
not request..."
** On the client not being used, "inputs", "clipboard_get_targets",
and "Failed to retrieve clipboard text"
* From a single CTRL+V:
** On the client being used, "clipboard_clear",
"clipboard_get_targets", which makes the next paste to be a ticking
timebomb.
** On the client not being used, "inputs", "clipboard_get_targets",
"Retrieving the clipboard data has failed", "clipboard_get_targets",
"Clipboard is already grabbed, ignoring 8 atoms", "Failed to retrieve
clipboard text"

When it usually fails - Failure type "B": https://termbin.com/hseu
* Sometimes it fails with the client not being used showing "Failed to
retrieve clipboard text" where it above showed "Clipboard is already
grabbed, ignoring 8 atoms"
* In this one I include an extra section for hitting the down arrow
again and pasting, knowing with 100% certainty that will lag the
system and fail.  This could have been shown on the other failure
logs, just this is the only time I did it

As an extra bonus, while I was typing this email (on my host system,
so with both spice clients open but neither actively being used) the
client I've been referring to as "not being used" (even though here
neither were being used) completely crashed.  I don't remember if I
was copying or pasting anything in this email when it happened.  So, I
don't know if it might be related to the v2 patch, or if it might be
due to upgrading spice-git from 94f186e to dbdf692.  In these 39
commits I brought in to compile the v2 patch on top of, I do see some
having to do with "channel-display" and "display".

This crash log: https://termbin.com/9z37

The crash looks like it starts at 22:12:43.460 (line 209), with a
"channel-main.c:1362 main-1:0: Ignoring clipboard grab", then a
"spice-channel.c:801 main-1:0: Send error Error sending data: Broken
pipe" which spirals into a ton of other exits, resets, and errors.  In
this log are periodically 3 libusb lines which I always see happen
every 60 seconds while running "remote-viewer --spice-debug", anytime
with any release, git, or git version with patches.  But, after the
crash has spiraled through a ton of error lines, there's a lot more
libusb debugging information there.

Please let me know if it looks like this crash is related to the
clipboard problems, or if I should create a bug report on it.  My best
guess is it's related to the clipboard problems.  I'm going back to
the v1 patch, but will do so on top of current git (dbdf692) to try to
see if I get any more crashing.