Hi On Thu, Jan 10, 2019 at 5:50 PM Victor Toso <victortoso@xxxxxxxxxx> wrote: > > On Thu, Jan 10, 2019 at 05:25:21PM +0400, Marc-André Lureau wrote: > > Hi > > > > On Thu, Jan 10, 2019 at 4:47 PM Victor Toso <victortoso@xxxxxxxxxx> wrote: > > > > > > 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: > > > > hmm > > > > > 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. > > > > Sorry, you get a lot of clipboard-grab from the remote, but where is > > the problem? > > Problem is, why fetch data that no other application is > requesting. > > Situations like the one below happens due state changing fast + > idle callbacks for clear/request clipboard data... > > https://gitlab.freedesktop.org/spice/win32/vd_agent/issues/6#note_85246 Sorry, I don't understand the problem, yes I try :) > > > > v2 -> v3: > > > * Split the fix in two patches while adding some info in a 3rd patch; > > > * Kept the "clipboard_clear" as it was (Jakub) > > > * Added a "clipboard_grab" log in clipboard_grab() function; > > > > > > 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 | 15 +++++++++++++++ > > > 1 file changed, 15 insertions(+) > > > > > > diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c > > > index 85d5880..f5959f7 100644 > > > --- a/src/spice-gtk-session.c > > > +++ b/src/spice-gtk-session.c > > > @@ -763,6 +763,19 @@ static void clipboard_get(GtkClipboard *clipboard, > > > > > > SPICE_DEBUG("clipboard get"); > > > > > > +#ifdef GDK_WINDOWING_X11 > > > + /* Do not request clipboard data while we are still interacting with the > > > + * Guest; Clipboard data could change shortly and this request would just > > > + * be wasteful. */ > > > > I don't think there is anything preventing another app the > > clipboard data while spice-gtk has the focus. In fact, that's > > how clipboard manager work afaik. > > That means that another application in Client OS might get remote > clipboard data while the user is interacting with remote VM. > That's pretty much a security concern as well. This does not > happen on Wayland and I would rather that we avoid it with X11 > too, even if it breaks ClipboardManagers that don't request our > clipboard after we focus-out I think it could happen on wayland as well, for example with https://extensions.gnome.org/extension/779/clipboard-indicator/. > > Thanks for reviewing, > Victor > > > > > > + if (GDK_IS_X11_DISPLAY(gdk_display_get_default()) && > > > + spice_gtk_session_get_keyboard_has_focus(self)) { > > > + SPICE_DEBUG("clipboard get: not requesting data while holding focus"); > > > + return; > > > + } > > > +#endif > > > + SPICE_DEBUG("clipboard get: has-foucus=%d", > > > + spice_gtk_session_get_keyboard_has_focus(self)); > > > + > > > selection = get_selection_from_clipboard(s, clipboard); > > > g_return_if_fail(selection != -1); > > > g_return_if_fail(info < SPICE_N_ELEMENTS(atom2agent)); > > > @@ -818,6 +831,8 @@ static gboolean clipboard_grab(SpiceMainChannel *main, guint selection, > > > guint32* types, guint32 ntypes, > > > gpointer user_data) > > > { > > > + SPICE_DEBUG("clipboard_grab"); > > > + > > > g_return_val_if_fail(SPICE_IS_GTK_SESSION(user_data), FALSE); > > > > > > SpiceGtkSession *self = user_data; > > > -- > > > 2.20.1 > > > > > > _______________________________________________ > > > Spice-devel mailing list > > > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > > > https://lists.freedesktop.org/mailman/listinfo/spice-devel > > > > > > > > -- > > Marc-André Lureau -- Marc-André Lureau _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel