[PATCH weston v4] libweston-desktop: fix stale ping when a wl_shell_surface is destroyed

[Quentin Glidic]

On 08/12/2016 16:20, Giulio Camuffo wrote:
> When sending a ping event to a surface using the wl_shell interface,
> if that surface is destroyed before we receive the pong we will never
> receive it, even if the client is actually responsive, since the
> interface does not exist anymore. So when the surface if destroyed
> pretend it's a pong and reset the ping state.
>
> Signed-off-by: Giulio Camuffo <giuliocamuffo at gmail.com>

I made a few renames to match the current code.
I was going to push it, but I found a last issue, see below.


> ---
>
> v3: store the ping serial in the surface instead of the client wrapper
> v4: removed leftover change
>
>  libweston-desktop/wl-shell.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/libweston-desktop/wl-shell.c b/libweston-desktop/wl-shell.c
> index 399139c..f75b022 100644
> --- a/libweston-desktop/wl-shell.c
> +++ b/libweston-desktop/wl-shell.c
> @@ -57,6 +57,7 @@ struct weston_desktop_wl_shell_surface {
>  	struct weston_desktop_seat *popup_seat;
>  	enum weston_desktop_wl_shell_surface_state state;
>  	struct wl_listener wl_surface_resource_destroy_listener;
> +	uint32_t ping_serial;
>  };
>
>  static void
> @@ -112,6 +113,7 @@ weston_desktop_wl_shell_surface_ping(struct weston_desktop_surface *dsurface,
>  {
>  	struct weston_desktop_wl_shell_surface *surface = user_data;
>
> +	surface->ping_serial = serial;
>  	wl_shell_surface_send_ping(surface->resource, serial);
>  }
>
> @@ -181,11 +183,27 @@ weston_desktop_wl_shell_change_state(struct weston_desktop_wl_shell_surface *sur
>  }
>
>  static void
> +pong_client(struct weston_desktop_wl_shell_surface *surface)

weston_desktop_wl_shell_surface_pong_client

> +{
> +	struct weston_desktop_client *client =
> +		weston_desktop_surface_get_client(surface->surface);
> +
> +	weston_desktop_client_pong(client, surface->ping_serial);
> +	surface->ping_serial = 0;
> +}
> +
> +static void
>  weston_desktop_wl_shell_surface_destroy(struct weston_desktop_surface *dsurface,
>  					void *user_data)
>  {
>  	struct weston_desktop_wl_shell_surface *surface = user_data;
>
> +	/* If the surface being destroyed was the one that was pinged before
> +	 * we need to fake a pong here, because it cannot answer the ping anymore,
> +	 * even if the client is responsive. */
> +	if (surface->ping_serial != 0)
> +		pong_client(surface);
> +
>  	wl_list_remove(&surface->wl_surface_resource_destroy_listener.link);
>
>  	weston_desktop_wl_shell_surface_maybe_ungrab(surface);
> @@ -203,8 +221,10 @@ weston_desktop_wl_shell_surface_protocol_pong(struct wl_client *wl_client,
>  					      uint32_t serial)
>  {
>  	struct weston_desktop_surface *surface = wl_resource_get_user_data(resource);

dsurface

> +	struct weston_desktop_wl_shell_surface *wls =
> +		weston_desktop_surface_get_implementation_data(surface);

surface

> -	weston_desktop_client_pong(weston_desktop_surface_get_client(surface), serial);
> +	pong_client(wls);

We should check that surface->ping_serial == serial somehow.
Maybe it is safe to ignore serial for now, as it fixes something, then 
add a check in a follow-up commit.

Thoughts? Pekka (aka assertman)?

Cheers,


>  }
>
>  static void
>


-- 

Quentin “Sardem FF7” Glidic