[PATCH weston] libweston-desktop: fix stale ping when a wl_shell_surface is destroyed
Quentin Glidic
sardemff7+wayland at sardemff7.net
Sat Oct 1 09:48:04 UTC 2016
On 26/09/2016 12:37, 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.
> ---
>
> v2: put the logic in the wl_shell specific file instead of the common code
Sorry for the late reply.
I think the extra zalloc is not a good thing either.
We can simply store the serial in the surface, and send a pong if serial
!= 0 on destroy. Is that good for you?
libweston-desktop already limits pings to one at a time.
Cheers,
>
> libweston-desktop/client.c | 10 +++++++++-
> libweston-desktop/internal.h | 4 +++-
> libweston-desktop/wl-shell.c | 42 ++++++++++++++++++++++++++++++++++++++--
> libweston-desktop/xdg-shell-v5.c | 2 +-
> libweston-desktop/xdg-shell-v6.c | 2 +-
> libweston-desktop/xwayland.c | 3 ++-
> 6 files changed, 56 insertions(+), 7 deletions(-)
>
> diff --git a/libweston-desktop/client.c b/libweston-desktop/client.c
> index 29c3c98..810b6ba 100644
> --- a/libweston-desktop/client.c
> +++ b/libweston-desktop/client.c
> @@ -39,6 +39,7 @@ struct weston_desktop_client {
> uint32_t ping_serial;
> struct wl_event_source *ping_timer;
> struct wl_signal destroy_signal;
> + void *user_data;
> };
>
> void
> @@ -86,7 +87,7 @@ weston_desktop_client_create(struct weston_desktop *desktop,
> wl_dispatcher_func_t dispatcher,
> const struct wl_interface *interface,
> const void *implementation, uint32_t version,
> - uint32_t id)
> + uint32_t id, void *user_data)
> {
> struct weston_desktop_client *client;
> struct wl_display *display;
> @@ -101,6 +102,7 @@ weston_desktop_client_create(struct weston_desktop *desktop,
>
> client->desktop = desktop;
> client->client = wl_client;
> + client->user_data = user_data;
>
> wl_list_init(&client->surface_list);
> wl_signal_init(&client->destroy_signal);
> @@ -210,3 +212,9 @@ 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;
> }
> +
> +void *
> +weston_desktop_client_get_user_data(struct weston_desktop_client *client)
> +{
> + return client->user_data;
> +}
> diff --git a/libweston-desktop/internal.h b/libweston-desktop/internal.h
> index a9c974b..6f8b5aa 100644
> --- a/libweston-desktop/internal.h
> +++ b/libweston-desktop/internal.h
> @@ -128,7 +128,7 @@ weston_desktop_client_create(struct weston_desktop *desktop,
> wl_dispatcher_func_t dispatcher,
> const struct wl_interface *interface,
> const void *implementation, uint32_t version,
> - uint32_t id);
> + uint32_t id, void *user_data);
>
> void
> weston_desktop_client_add_destroy_listener(struct weston_desktop_client *client,
> @@ -143,6 +143,8 @@ weston_desktop_client_get_surface_list(struct weston_desktop_client *client);
> void
> weston_desktop_client_pong(struct weston_desktop_client *client,
> uint32_t serial);
> +void *
> +weston_desktop_client_get_user_data(struct weston_desktop_client *client);
>
> struct weston_desktop_surface *
> weston_desktop_surface_create(struct weston_desktop *desktop,
> diff --git a/libweston-desktop/wl-shell.c b/libweston-desktop/wl-shell.c
> index ded69f7..3a81f42 100644
> --- a/libweston-desktop/wl-shell.c
> +++ b/libweston-desktop/wl-shell.c
> @@ -58,6 +58,11 @@ struct weston_desktop_wl_shell_surface {
> enum weston_desktop_wl_shell_surface_state state;
> };
>
> +struct wl_shell_client {
> + struct weston_desktop_wl_shell_surface *ping_surface;
> + uint32_t ping_serial;
> +};
> +
> static void
> weston_desktop_wl_shell_surface_set_size(struct weston_desktop_surface *dsurface,
> void *user_data,
> @@ -109,7 +114,12 @@ weston_desktop_wl_shell_surface_ping(struct weston_desktop_surface *dsurface,
> uint32_t serial, void *user_data)
> {
> struct weston_desktop_wl_shell_surface *surface = user_data;
> + struct weston_desktop_client *client =
> + weston_desktop_surface_get_client(dsurface);
> + struct wl_shell_client *wsc = weston_desktop_client_get_user_data(client);
>
> + wsc->ping_surface = surface;
> + wsc->ping_serial = serial;
> wl_shell_surface_send_ping(surface->resource, serial);
> }
>
> @@ -179,10 +189,29 @@ weston_desktop_wl_shell_change_state(struct weston_desktop_wl_shell_surface *sur
> }
>
> static void
> +pong_client(struct weston_desktop_client *client, uint32_t serial)
> +{
> + struct wl_shell_client *wsc = weston_desktop_client_get_user_data(client);
> +
> + wsc->ping_surface = NULL;
> + wsc->ping_serial = 0;
> + weston_desktop_client_pong(client, serial);
> +}
> +
> +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;
> + struct weston_desktop_client *client =
> + weston_desktop_surface_get_client(dsurface);
> + struct wl_shell_client *wsc = weston_desktop_client_get_user_data(client);
> +
> + /* 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 (wsc->ping_surface == surface)
> + pong_client(client, wsc->ping_serial);
>
> weston_desktop_wl_shell_surface_maybe_ungrab(surface);
> weston_desktop_surface_unset_relative_to(surface->surface);
> @@ -199,8 +228,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);
> + struct weston_desktop_client *client =
> + weston_desktop_surface_get_client(surface);
>
> - weston_desktop_client_pong(weston_desktop_surface_get_client(surface), serial);
> + pong_client(client, serial);
> }
>
> static void
> @@ -449,10 +480,17 @@ weston_desktop_wl_shell_bind(struct wl_client *client, void *data,
> uint32_t version, uint32_t id)
> {
> struct weston_desktop *desktop = data;
> + struct wl_shell_client *wsclient;
> +
> + wsclient = zalloc(sizeof(*wsclient));
> + if (!wsclient) {
> + wl_client_post_no_memory(client);
> + return;
> + }
>
> weston_desktop_client_create(desktop, client, NULL, &wl_shell_interface,
> &weston_desktop_wl_shell_implementation,
> - version, id);
> + version, id, wsclient);
> }
>
> struct wl_global *
> diff --git a/libweston-desktop/xdg-shell-v5.c b/libweston-desktop/xdg-shell-v5.c
> index 14216b0..942898a 100644
> --- a/libweston-desktop/xdg-shell-v5.c
> +++ b/libweston-desktop/xdg-shell-v5.c
> @@ -808,7 +808,7 @@ weston_desktop_xdg_shell_bind(struct wl_client *client, void *data,
>
> weston_desktop_client_create(desktop, client,
> xdg_shell_unversioned_dispatch,
> - &xdg_shell_interface, NULL, version, id);
> + &xdg_shell_interface, NULL, version, id, NULL);
> }
>
> struct wl_global *
> diff --git a/libweston-desktop/xdg-shell-v6.c b/libweston-desktop/xdg-shell-v6.c
> index 2afce81..c2c688f 100644
> --- a/libweston-desktop/xdg-shell-v6.c
> +++ b/libweston-desktop/xdg-shell-v6.c
> @@ -1264,7 +1264,7 @@ weston_desktop_xdg_shell_bind(struct wl_client *client, void *data,
> weston_desktop_client_create(desktop, client, NULL,
> &zxdg_shell_v6_interface,
> &weston_desktop_xdg_shell_implementation,
> - version, id);
> + version, id, NULL);
> }
>
> struct wl_global *
> diff --git a/libweston-desktop/xwayland.c b/libweston-desktop/xwayland.c
> index bd68bc6..d965143 100644
> --- a/libweston-desktop/xwayland.c
> +++ b/libweston-desktop/xwayland.c
> @@ -366,7 +366,8 @@ weston_desktop_xwayland_init(struct weston_desktop *desktop)
> return;
>
> xwayland->desktop = desktop;
> - xwayland->client = weston_desktop_client_create(desktop, NULL, NULL, NULL, NULL, 0, 0);
> + xwayland->client = weston_desktop_client_create(desktop, NULL, NULL,
> + NULL, NULL, 0, 0, NULL);
>
> weston_layer_init(&xwayland->layer, &compositor->cursor_layer.link);
>
>
--
Quentin “Sardem FF7” Glidic
More information about the wayland-devel
mailing list