[Spice-devel] [spice-gtk v4 2/2] gtk-session: clipboard: x11: owner-change improvement

[Victor Toso]

On Mon, Feb 11, 2019 at 11:19:02AM +0100, Jakub Janku wrote:
> On Mon, Feb 11, 2019 at 10:30 AM Victor Toso <victortoso at redhat.com> wrote:
> >
> > Hi,
> >
> > On Mon, Feb 11, 2019 at 10:12:42AM +0100, Jakub Janku wrote:
> > > ping?
> >
> > Should be fixed by:
> 
> You can double-check with James to be sure, but I don't think that's true.
> 
> If you look at the logs ( https://termbin.com/40un ), there's the
> following line:
>     |   (virt-viewer:25767): GSpice-DEBUG: 21:29:06.743:
> spice-gtk-session.c:556 Retrieving the clipboard data has failed
> And the commit you mention below fixes exactly that and that's all,
> the race is still there.

(...)

> > > >
> > > > (1) user copies something in the guest, grab is sent to the
> > > > spice-gtk
> > > > (2) clipboard manager immediately requests the data, data is
> > > > retrieved from the vdagent
> > > > (3) user pastes the copied data in guest, this causes a quick
> > > > re-grab in the guest = a clipboard release message is sent to
> > > > spice-gtk and it is immediately followed by a new grab
> > > > (4) spice-gtk receives clipboard release, so it clears the clipboard
> > > > (5) clipboard manager detects that the clipboard has no owner, so it
> > > > grabs it itself, advertising the data it originally obtained from us
> > > > in step (2) (this normally enables the user to paste the data even
> > > > after the source app has been closed)
> > > > (6) spice-gtk receives "owner-change" signal caused by the grab in
> > > > step (5), requests clipboard targets and sends a grab to vdagent
> > > > (7) spice-gtk finally receives the grab from step (3), so it sets
> > > > the clipboard using gtk_clipboard_set_with_owner()
> > > > (8) vdagent receives grab from step (6), so it too sets the
> > > > clipboard using gtk_clipboard_set_with_owner()
> > > >
> > > > This is clearly a race. We ended up in a state where both spice-gtk
> > > > and vdagent thinks the other component can provide the data, but in
> > > > reality none of them can.
> > > >
> > > > (This does not happen always, as can be seen in the log, the first
> > > > paste succeeds.)
> > > >
> > > > Given current spice protocol, I don't see a way to detect the
> > > > race on either side, but I'd love to be wrong. So I'd update
> > > > the commit log and comment in the code to explain the
> > > > situation and then it's ack from me.

The race is unavoidable with current protocol. What we need is
documentation on what behaviors we expect when such situations
happen... That's what I think but I may be wrong :)

(...)

> > > >> +#ifdef GDK_WINDOWING_X11
> > > >> +    /* In X11, while holding the keyboard-grab we are not interested in this
> > > >> +     * event as it either came by grab or release messages from agent.  */
> > > >> +    if (GDK_IS_X11_DISPLAY(gdk_display_get_default()) &&
> > > >> +        spice_gtk_session_get_keyboard_has_focus(self)) {
> > > >> +        SPICE_DEBUG("clipboard: owner-changed event: not requesting client's target "
> > > >> +                    "while holding focus");
> > > >> +        return;
> > > >> +    }
> > > >> +#endif
> > > >> +    SPICE_DEBUG("clipboard: owner-changed event: has-focus=%d",
> > > >> +                spice_gtk_session_get_keyboard_has_focus(self));
> > > >> +

This patch was nacked by Marc-André because it discards the
situation where Clipboard Managers are getting clipboard data
(from the guest!). This is a behavior that Marc-André would like
to keep for some reason.

My suggestion is to add some configurable property to give users
some choice as there is none nowadays...

   https://lists.freedesktop.org/archives/spice-devel/2019-January/047259.html 


> > > >>      s->clipboard_by_guest[selection] = FALSE;
> > > >>      s->clip_hasdata[selection] = TRUE;
> > > >>      if (s->auto_clipboard_enable && !read_only(self))
> > > >> --
> > > >> 2.20.1
> > > >>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20190211/f8cbf329/attachment.sig>