Hi,
On Wed, Jan 16, 2019 at 01:07:27PM +0400, Marc-André Lureau wrote:
> > Not true. Possible client-side bug, reproducible on X11, as I
> > discussed before. As mentioned in the comment, we are sending
> > wrong selection-grab to the guest.
>
> Sorry, I don't follow "we are sending wrong selection-grab" ?
I think I tried too hard already to make you understand that on
X11, that's happening while user is copying in the guest VM.
> > > Not sure sending a reply is going to help much in that case...
> >
> > It helps. Instead of blocking the application, it fails to paste
> > the clipboard and all is good.
>
> But if the agent logic is already wrong, sending a reply isn't going
> to help, it could do anything..
It might have been a valid request at the time it was sent, if
you accept that we sent the selection-grab.
> > > > + * sent a bad selection-grab to the agent or the agent is buggy. */
> > > > + if (s->clipboard_by_guest[selection]) {
> > > > + SPICE_DEBUG("clipboard: agent request: grab on hold by agent, possible race");
> > > > + goto notify_agent;
> > > > + }
> > > > +
> > > > + /* The selection-request by agent should happen only if the clipboard data is set
> > > > + * by client */
> > > > + if (!s->clip_grabbed[selection]) {
> > > > + SPICE_DEBUG("clipboard: agent request: data set by agent, possible race");
> > > > + goto notify_agent;
> > > > + }
> > >
> > > This could be adding more race conditions. clip_grabbed is set
> > > asynchronously after owner changed, and indicate if the grab
> > > message was sent to the agent,
> >
> > We send a selection-release on owner-change. If we receive a
> > selection-request before agent receives the selection-release,
> > this is expected but we should notify the guest, afaics.
> >
> > > as you correctly say. It doesn't mean you can't request client
> > > clipboard content.
> >
> > > I understand the racy case, but the condition seems wrong, it
> > > should attempt to request current client clipboard content, and
> > > fail/succeed after.
> >
> > No. It should request the content from the grab that was sent. If
>
> There is no such thing as clipboard ID/nth in any clipboard protocol I
> know of. User simply request current clipboard content (not the
> nth), whether it changed since last grab is not important: it
> will retrieve something of the requested type or nothing.
Not about the id, but selection metadata itself or the @type
parameter in clipboard_request()
> > clipboard changed, previous grab gets invalidated and a
> > selection-release is sent and another selection-grab is sent with
> > the metadata of the new grab.
> >
> > Proving recent data from old grab seems wrong.
>
> It is not.
>
> > >
> > > > + /* Ready only, still should reply to agent to avoid it waiting for data */
> > >
> > > No, read-only shouldn't reply. We are lacking a way to tell the guest
> > > that the client is read-only though. So this may be acceptable for
> > > now, but we should have a TODO/FIXME..
> >
> > Well, it should not happen anyway as on read-only we should not
> > send selection-grab. I'll fix and resend.
>
> Right
>
> >
> > Thanks for the quick review.
> >
> > > > + if (read_only(self)) {
> > > > + g_warning("clipboard: agent request: read only, deny request");
> > > > + goto notify_agent;
> > > > + }
> > > >
> > > > if (type == VD_AGENT_CLIPBOARD_UTF8_TEXT) {
> > > > gtk_clipboard_request_text(cb, clipboard_received_text_cb,
> > > > @@ -1039,6 +1057,10 @@ static gboolean clipboard_request(SpiceMainChannel *main, guint selection,
> > > > }
> > > >
> > > > return TRUE;
> > > > +
> > > > +notify_agent:
> > > > + spice_main_channel_clipboard_selection_notify(s->main, selection, type, NULL, 0);
> > > > + return FALSE;
> > > > }
> > > >
> > > > static void clipboard_release(SpiceMainChannel *main, guint selection,
> > > > --
> > > > 2.20.1
> > > >
> > > > _______________________________________________
> > > > Spice-devel mailing list
> > > > Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> > > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > >
> > >
> > >
> > > --
> > > Marc-André Lureau
>
>
>
> --
> Marc-André Lureau
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel