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

[Quentin Glidic]

On 23/09/2016 17:30, 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.

Even if it is still internal, I do not like having weston_desktop_client 
outside of client.c.

I would rather pretend the pong for real, as in, directly from 
wl_shell.c in the destroy_surface callback. Do you think it could work?

Cheers,


> ---
>  libweston-desktop/client.c   | 14 ++++----------
>  libweston-desktop/internal.h | 12 +++++++++++-
>  libweston-desktop/surface.c  |  4 ++++
>  3 files changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/libweston-desktop/client.c b/libweston-desktop/client.c
> index 29c3c98..ee8a2f8 100644
> --- a/libweston-desktop/client.c
> +++ b/libweston-desktop/client.c
> @@ -31,16 +31,6 @@
>  #include "libweston-desktop.h"
>  #include "internal.h"
>
> -struct weston_desktop_client {
> -	struct weston_desktop *desktop;
> -	struct wl_client *client;
> -	struct wl_resource *resource;
> -	struct wl_list surface_list;
> -	uint32_t ping_serial;
> -	struct wl_event_source *ping_timer;
> -	struct wl_signal destroy_signal;
> -};
> -
>  void
>  weston_desktop_client_add_destroy_listener(struct weston_desktop_client *client,
>  					   struct wl_listener *listener)
> @@ -75,6 +65,8 @@ static int
>  weston_desktop_client_ping_timeout(void *user_data)
>  {
>  	struct weston_desktop_client *client = user_data;
> +	if (client->ping_serial == 0)
> +		return 1;
>
>  	weston_desktop_api_ping_timeout(client->desktop, client);
>  	return 1;
> @@ -192,6 +184,7 @@ weston_desktop_client_ping(struct weston_desktop_client *client)
>
>  	client->ping_serial =
>  		wl_display_next_serial(wl_client_get_display(client->client));
> +	client->ping_surface = surface;
>  	wl_event_source_timer_update(client->ping_timer, 10000);
>
>  	implementation->ping(surface, client->ping_serial, implementation_data);
> @@ -209,4 +202,5 @@ weston_desktop_client_pong(struct weston_desktop_client *client, uint32_t serial
>
>  	wl_event_source_timer_update(client->ping_timer, 0);
>  	client->ping_serial = 0;
> +	client->ping_surface = NULL;
>  }
> diff --git a/libweston-desktop/internal.h b/libweston-desktop/internal.h
> index a9c974b..7a4e6b2 100644
> --- a/libweston-desktop/internal.h
> +++ b/libweston-desktop/internal.h
> @@ -27,7 +27,17 @@
>  #include "compositor.h"
>
>  struct weston_desktop_seat;
> -struct weston_desktop_client;
> +
> +struct weston_desktop_client {
> +	struct weston_desktop *desktop;
> +	struct wl_client *client;
> +	struct wl_resource *resource;
> +	struct wl_list surface_list;
> +	uint32_t ping_serial;
> +	struct weston_desktop_surface *ping_surface;
> +	struct wl_event_source *ping_timer;
> +	struct wl_signal destroy_signal;
> +};
>
>  struct weston_compositor *
>  weston_desktop_get_compositor(struct weston_desktop *desktop);
> diff --git a/libweston-desktop/surface.c b/libweston-desktop/surface.c
> index 2205107..e448280 100644
> --- a/libweston-desktop/surface.c
> +++ b/libweston-desktop/surface.c
> @@ -127,6 +127,10 @@ weston_desktop_surface_destroy(struct weston_desktop_surface *surface)
>  {
>  	struct weston_desktop_view *view, *next_view;
>  	struct weston_desktop_surface *child, *next_child;
> +	struct weston_desktop_client *client = surface->client;
> +
> +	if (client->ping_surface == surface)
> +		weston_desktop_client_pong(client, client->ping_serial);
>
>  	wl_list_remove(&surface->surface_commit_listener.link);
>  	wl_list_remove(&surface->surface_destroy_listener.link);
>


-- 

Quentin “Sardem FF7” Glidic