Hi, On Fri, Jan 11, 2019 at 01:39:22PM +0400, Marc-André Lureau wrote: > Hi > > On Thu, Jan 10, 2019 at 5:40 PM Victor Toso <victort...@redhat.com> wrote: > > > > Hi, > > > > On Thu, Jan 10, 2019 at 05:18:48PM +0400, Marc-André Lureau wrote: > > > On Thu, Jan 10, 2019 at 4:47 PM Victor Toso <victort...@redhat.com> wrote: > > > > > > > > From: Victor Toso <m...@victortoso.com> > > > > > > > > While interacting within the guest the VD_AGENT_CLIPBOARD_GRAB message > > > > will be sent by the guest agent to notify that agent is holding some > > > > clipboard data. When this clipboard data changes, the agent will send > > > > VD_AGENT_CLIPBOARD_RELEASE to simply notify that previous clipboard > > > > data is not available anymore and a new VD_AGENT_CLIPBOARD_GRAB > > > > follows up with the current clipboard data being hold by agent. > > > > > > > > This patch helps in fixing a state race, in X11, between who is > > > > 'holding' the clipboard grab. > > > > > > > > The bug happens because gtk_clipboard_clear() will set owner to none, > > > > making the rest of the code path consider that clipboard data has > > > > changed in the *client* and ends up requesting the metadata with > > > > gtk_clipboard_request_targets(), handled by clipboard_get_targets(). > > > > > > > > It is possible to have clipboard_get_targets() being called between > > > > one VD_AGENT_CLIPBOARD_RELEASE and other VD_AGENT_CLIPBOARD_GRAB. > > > > > > > > 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 <victort...@redhat.com> > > > > Tested-by: James Harvey @jamespharvey20 > > > > --- > > > > src/spice-gtk-session.c | 16 ++++++++++++++++ > > > > 1 file changed, 16 insertions(+) > > > > > > > > diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c > > > > index adc72a2..85d5880 100644 > > > > --- a/src/spice-gtk-session.c > > > > +++ b/src/spice-gtk-session.c > > > > @@ -27,6 +27,9 @@ > > > > #include <X11/Xlib.h> > > > > #include <gdk/gdkx.h> > > > > #endif > > > > +#ifdef GDK_WINDOWING_WAYLAND > > > > +#include <gdk/gdkwayland.h> > > > > +#endif > > > > #ifdef G_OS_WIN32 > > > > #include <windows.h> > > > > #include <gdk/gdkwin32.h> > > > > @@ -674,6 +677,19 @@ static void clipboard_owner_change(GtkClipboard > > > > *clipboard, > > > > return; > > > > } > > > > > > > > +#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"); > > > > > > Couldn't this race happen without keyboard focus? > > > > In X11 we would receive owner-changed every time that clipboard > > data has changed so, if guest agent sends grab/release without > > user triggering them, it might happen but I don't think that > > would be common. > > > > This patch addresses doing gtk_clipboard_request_targets() while > > spice-gtk is the one changing GtkClipboard. > > > I thought that case was avoided with > gtk_clipboard_get_owner(clipboard) == G_OBJECT(self) > > But you are saying that the gtk_clipboard_clear() call ends up > triggering gtk_clipboard_request_targets() and that is the source of > races. > > Why not simply?
I thought it should work too but it didn't as explained by Jakub.
https://lists.freedesktop.org/archives/spice-devel/2019-January/047142.html
I don't mind ignoring 3/3 patch but some fix here would be good
prior the release.
Earlier on, I had a patch that combined 2/3 and 3/3 which was
acked but I though later that would be better to split the change
which is done on onwer-changed event and avoiding the
clipboard_get() as it could be two different issues IMHO.
https://lists.freedesktop.org/archives/spice-devel/2019-January/047012.html
So, ignoring the clipboard_get() issue for now, do you have any
other idea to address this one?
> - if (gtk_clipboard_get_owner(clipboard) == G_OBJECT(self)) {
> + GObject *owner = gtk_clipboard_get_owner(clipboard);
> + if (owner == NULL || gtk_clipboard_get_owner(clipboard) ==
> G_OBJECT(self)) {
> return;
> }
Cheers,
Victor
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
