On Wed, 13 Dec 2017 11:51:19 +0100 emersion <cont...@emersion.fr> wrote:
> Bug [1] reported that wl_display_destroy() doesn't destroy clients, so > client socket file descriptors are being kept open until the compositor > process exits. > > Patch [2] proposed to destroy clients in wl_display_destroy(). The > patch was not accepted because doing so changes the ABI. > > Thus, a new wl_display_destroy_clients() function is added in this > patch. It should be called by compositors right before > wl_display_destroy(). > > [1] https://bugs.freedesktop.org/show_bug.cgi?id=99142 > [2] https://patchwork.freedesktop.org/patch/128832/ > > Signed-off-by: emersion <cont...@emersion.fr> > --- > v2: change commit message, add docs, use a safer approach when iterating > over the client list > > src/wayland-server-core.h | 3 +++ > src/wayland-server.c | 38 ++++++++++++++++++++++++++++++++++++++ > 2 files changed, 41 insertions(+) > > diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h > index fd458c5..2e725d9 100644 > --- a/src/wayland-server-core.h > +++ b/src/wayland-server-core.h > @@ -214,6 +214,9 @@ wl_display_run(struct wl_display *display); > void > wl_display_flush_clients(struct wl_display *display); > > +void > +wl_display_destroy_clients(struct wl_display *display); > + > struct wl_client; > > typedef void (*wl_global_bind_func_t)(struct wl_client *client, void *data, > diff --git a/src/wayland-server.c b/src/wayland-server.c > index 82a3b01..33df413 100644 > --- a/src/wayland-server.c > +++ b/src/wayland-server.c > @@ -1264,6 +1264,44 @@ wl_display_flush_clients(struct wl_display *display) > } > } > > +/** Destroy all clients connected to the display > + * > + * \param display The display object > + * > + * This function should be called right before wl_display_destroy() to ensure > + * all client resources are closed properly. Destroying a client from within > + * wl_display_destroy_clients() is safe, but creating one will leak resources > + * and raise a warning. > + * > + * \memberof wl_display > + */ > +WL_EXPORT void > +wl_display_destroy_clients(struct wl_display *display) > +{ > + struct wl_list tmp_client_list, *pos; > + struct wl_client *client; > + > + /* Move the whole client list to a temporary head because some new > clients > + * might be added to the original head. */ > + wl_list_init(&tmp_client_list); > + wl_list_insert_list(&tmp_client_list, &display->client_list); > + wl_list_init(&display->client_list); > + > + /* wl_list_for_each_safe isn't enough here: it fails if the next client > is > + * destroyed by the destroy handler of the current one. */ > + while (!wl_list_empty(&tmp_client_list)) { > + pos = tmp_client_list.next; > + client = wl_container_of(pos, client, link); > + > + wl_client_destroy(client); > + } > + > + if (!wl_list_empty(&display->client_list)) { > + wl_log("wl_display_destroy_clients: cannot destroy all clients > because " > + "new ones were created by destroy callbacks\n"); > + } > +} > + > static int > socket_data(int fd, uint32_t mask, void *data) > { Hi, very good! Reviewed-by: Pekka Paalanen <pekka.paala...@collabora.co.uk> Do you have your compositor now calling this without issues? It would be awesome to have a test in the libwayland test suite calling this function and checking a client gets destroyed, especially as we don't have Weston using this yet. Thanks, pq
pgpYjx0UbbCU3.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel