On Mon, Feb 11, 2019 at 10:30 AM Victor Toso <victortoso@xxxxxxxxxx> 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. Cheers, Jakub > > commit 214f8fd969127c41a7d53def196118ee0549a411 > Author: Jakub Janků <jjankuatredhat.com> > Date: Sun Jan 27 18:14:20 2019 +0100 > > clipboard: don't request targets without owner on X11 > > On X11, if the owner in GdkEventOwnerChange is set to NULL, > it means there's no data in the clipboard, so it's pointless to > request targets as the request will fail anyway. > > On Wayland, owner is always NULL, so don't do anything there. > > Signed-off-by: Jakub Janků <jjanku at redhat.com> > Acked-by: Victor Toso <victortoso@xxxxxxxxxx> > > The fact that the patch in this mail thread is related to keyboard-grab > was the reason to be nacked. If you want to discuss that, we might move > along to that thread on clibpoard-managers, sent as RFC > > https://lists.freedesktop.org/archives/spice-devel/2019-January/047259.html > > > On Mon, Jan 28, 2019 at 10:29 AM Jakub Janku <jjanku@xxxxxxxxxx> wrote: > > > > > > Hi, > > > > > > I tried to fix this bug in a less radical way, but my patch unfortunately did not cover all the cases. > > > > > > I obtained some logs from James Harvey which make the situation clearer - it can be found here: > > > https://termbin.com/40un > > > So I'll try to explain what's happening: > > > > > > James uses KDE which has a clipboard manager integrated in (klipper). > > > > > > (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. > > I think we can still improve documentation to clarify the race and > behaviors. There are a few things that I thought were obvious and in > fact Marc-André pointed out I was wrong. > > Either way, let me know if I'm missing something. > > Cheers! > > > > Cheers, > > > Jakub > > > > > > On Mon, Jan 14, 2019, 1:34 PM Victor Toso <victortoso@xxxxxxxxxx wrote: > > >> > > >> From: Victor Toso <me@xxxxxxxxxxxxxx> > > >> > > >> On X11, the release-grab message might end up clearing the > > >> GtkClipboard which triggers the owner-changed callback having the > > >> event owner as NULL. > > >> > > >> We should not be calling gtk_clipboard_request_targets() in this > > >> situation as is prone to errors as the intention here is request > > >> clipboard information from changes made by client OS. > > >> > > >> The fix is to avoid any such request while spice client is holding the > > >> keyboard 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 > > >> > > >> Changed in v4: > > >> - Updated commit log > > >> > > >> Signed-off-by: Victor Toso <victortoso@xxxxxxxxxx> > > >> Tested-by: James Harvey @jamespharvey20 > > >> --- > > >> src/spice-gtk-session.c | 13 +++++++++++++ > > >> 1 file changed, 13 insertions(+) > > >> > > >> diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c > > >> index abce43f..20df70d 100644 > > >> --- a/src/spice-gtk-session.c > > >> +++ b/src/spice-gtk-session.c > > >> @@ -674,6 +674,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"); > > >> + return; > > >> + } > > >> +#endif > > >> + SPICE_DEBUG("clipboard: owner-changed event: has-focus=%d", > > >> + spice_gtk_session_get_keyboard_has_focus(self)); > > >> + > > >> s->clipboard_by_guest[selection] = FALSE; > > >> s->clip_hasdata[selection] = TRUE; > > >> if (s->auto_clipboard_enable && !read_only(self)) > > >> -- > > >> 2.20.1 > > >> _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel