[Spice-devel] [vdagent-linux] x11: invalidate requests for targets on grab from client

[Victor Toso]

Hi,

On Thu, Feb 21, 2019 at 09:25:57PM +0100, Jakub Janků wrote:
> If XSetSelectionOwner() is invoked during the time
> we are waiting for the requested clipboard targets,
> the targets we eventually receive are no longer valid.
> 
> To solve this, ignore the same count of target notifications
> as we expected at the time we received grab from the client.

Thanks, seems fine :)

> Otherwise we end up in a situation when vdagent holds
> the clipboard grab in the guest but cannot provide data to the
> apps that request it - this can be observed in the log:
> 
>     clipboard: received selection request event for target *, while not owning client clipboard
> 
> Signed-off-by: Jakub Janků <jjanku at redhat.com>

I'll be testing a little bit more this with previous patch and if
all is good, I'll proposed a release later Today.

> ---
> 
> This addresses the same issue as [0], just in the X11 clipboard implementation.
> 
> [0] https://lists.freedesktop.org/archives/spice-devel/2019-February/048075.html
> 
> ---
>  src/vdagent/x11-priv.h |  1 +
>  src/vdagent/x11.c      | 11 +++++++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/src/vdagent/x11-priv.h b/src/vdagent/x11-priv.h
> index e487aa2..99676d2 100644
> --- a/src/vdagent/x11-priv.h
> +++ b/src/vdagent/x11-priv.h
> @@ -92,6 +92,7 @@ struct vdagent_x11 {
>      int xfixes_event_base;
>      int max_prop_size;
>      int expected_targets_notifies[256];
> +    int ignore_targets_notifies[256];
>      int clipboard_owner[256];
>      int clipboard_type_count[256];
>      uint32_t clipboard_agent_types[256][256];
> diff --git a/src/vdagent/x11.c b/src/vdagent/x11.c
> index 484be5e..c2515a8 100644
> --- a/src/vdagent/x11.c
> +++ b/src/vdagent/x11.c
> @@ -945,6 +945,12 @@ static void vdagent_x11_handle_targets_notify(struct vdagent_x11 *x11,
>  
>      x11->expected_targets_notifies[selection]--;
>  
> +    if (x11->ignore_targets_notifies[selection] > 0) {
> +        x11->ignore_targets_notifies[selection]--;
> +        VSELPRINTF("ignoring selection notify TARGETS");
> +        return;
> +    }
> +
>      /* If we have more targets_notifies pending, ignore this one, we
>         are only interested in the targets list of the current owner
>         (which is the last one we've requested a targets list from) */
> @@ -1246,6 +1252,11 @@ void vdagent_x11_clipboard_grab(struct vdagent_x11 *x11, uint8_t selection,
>                         x11->selection_window, CurrentTime);
>      vdagent_x11_set_clipboard_owner(x11, selection, owner_client);
>  
> +    /* If there're pending requests for targets, ignore the returned
> +     * targets as the XSetSelectionOwner() call above made them invalid */
> +    x11->ignore_targets_notifies[selection] =
> +        x11->expected_targets_notifies[selection];
> +

    Acked-by: Victor Toso <victortoso at redhat.com>

Victor

>      /* Flush output buffers and consume any pending events */
>      vdagent_x11_do_read(x11);
>  }
> -- 
> 2.20.1
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- 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/20190222/c03dfa6c/attachment.sig>