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

Quentin Glidic Sat, 01 Oct 2016 02:48:21 -0700

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
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

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

Reply via email to