[PATCH v2 wayland 07/11] client: Replace the singleton zombie with bespoke zombies
Derek Foreman
derekf at osg.samsung.com
Mon Apr 17 15:39:37 UTC 2017
On 13/04/17 11:51 AM, Derek Foreman wrote:
> Using the singleton zombie object doesn't allow us to posthumously retain
> object interface information, which makes it difficult to properly inter
> future events destined for the recently deceased proxy.
>
> Notably, this makes it impossible for zombie proxy destined file
> descriptors to be properly consumed.
>
> Instead of having a singleton zombie object, we add a new wl_map flag
> (valid only on the client side where zombies roam - the existing
> "legacy" flag was only ever used on the server side) to indicate that a
> map entry is now a zombie.
>
> We still have to refcount and potentially free the proxy immediately or
> we're left with situations where it can be leaked forever. If we clean
> up on delete_id, for example, a forced disconnect will result in a leaked
> proxy (breaking much of the test suite).
>
> So, since we must free the zombied proxy immediately, we store just the
> interface for it in its previous map location, so signature information
> can be retained for zombie-destined events.
>
> Signed-off-by: Derek Foreman <derekf at osg.samsung.com>
> ---
> src/connection.c | 20 ++++++++-
> src/wayland-client.c | 114 ++++++++++++++++++++++++++++++++++++++++++++------
> src/wayland-private.h | 9 ++--
> src/wayland-util.c | 2 -
> 4 files changed, 126 insertions(+), 19 deletions(-)
>
> diff --git a/src/connection.c b/src/connection.c
> index f2ebe06..84f5d79 100644
> --- a/src/connection.c
> +++ b/src/connection.c
> @@ -809,6 +809,24 @@ wl_connection_demarshal(struct wl_connection *connection,
> return NULL;
> }
>
> +bool
> +wl_object_is_zombie(struct wl_map *map, uint32_t id)
> +{
> + uint32_t flags;
> +
> + if (map->side == WL_MAP_SERVER_SIDE)
> + return false;
> +
> + if (id >= WL_SERVER_ID_START)
> + return false;
> +
> + flags = wl_map_lookup_flags(map, id);
> + if (flags & WL_MAP_ENTRY_ZOMBIE)
> + return true;
> +
> + return false;
> +}
> +
> int
> wl_closure_lookup_objects(struct wl_closure *closure, struct wl_map *objects)
> {
> @@ -830,7 +848,7 @@ wl_closure_lookup_objects(struct wl_closure *closure, struct wl_map *objects)
> closure->args[i].o = NULL;
>
> object = wl_map_lookup(objects, id);
> - if (object == WL_ZOMBIE_OBJECT) {
> + if (wl_object_is_zombie(objects, id)) {
> /* references object we've already
> * destroyed client side */
> object = NULL;
> diff --git a/src/wayland-client.c b/src/wayland-client.c
> index 7243d3d..50fdfad 100644
> --- a/src/wayland-client.c
> +++ b/src/wayland-client.c
> @@ -55,6 +55,11 @@ enum wl_proxy_flag {
> WL_PROXY_FLAG_WRAPPER = (1 << 2),
> };
>
> +struct wl_zombie {
> + int event_count;
> + int *fd_count;
> +};
> +
> struct wl_proxy {
> struct wl_object object;
> struct wl_display *display;
> @@ -64,6 +69,7 @@ struct wl_proxy {
> void *user_data;
> wl_dispatcher_func_t dispatcher;
> uint32_t version;
> + struct wl_zombie *zombie;
> };
>
> struct wl_global {
> @@ -324,6 +330,66 @@ wl_display_create_queue(struct wl_display *display)
> return queue;
> }
>
> +static int
> +message_count_fds(const char *signature)
> +{
> + unsigned int count, i, fds = 0;
> + struct argument_details arg;
> +
> + count = arg_count_for_signature(signature);
> + for (i = 0; i < count; i++) {
> + signature = get_next_argument(signature, &arg);
> + if (arg.type == 'h')
> + fds++;
> + }
> +
> + return fds;
> +}
> +
> +static bool
> +prepare_zombie(struct wl_proxy *proxy)
> +{
> + const struct wl_message *message;
> + int i, count;
> + struct wl_zombie *zombie = NULL;
> +
> + for (i = 0; i < proxy->object.interface->event_count; i++) {
> + message = &proxy->object.interface->events[i];
> + count = message_count_fds(message->signature);
> + if (count && !zombie) {
> + zombie = zalloc(sizeof(struct wl_zombie));
> + if (!zombie)
> + return false;
> + zombie->event_count = proxy->object.interface->event_count;
> + zombie->fd_count = zalloc(zombie->event_count * sizeof(int));
> + if (!zombie->fd_count) {
> + free(zombie);
> + return false;
> + }
> + }
> + if (count)
> + zombie->fd_count[i] = count;
> + }
> + proxy->zombie = zombie;
> + return true;
> +}
> +
> +static enum wl_iterator_result
> +inter_zombie(void *element, void *data, uint32_t flags)
> +{
> + struct wl_zombie *zombie;
> + if (!(flags & WL_MAP_ENTRY_ZOMBIE))
missing whitespace between variables and code fixed locally, will be in v3.
> + return WL_ITERATOR_CONTINUE;
> +
> + if (element) {
> + zombie = element;
> + free(zombie->fd_count);
> + free(zombie);
> + }
> +
> + return WL_ITERATOR_CONTINUE;
> +}
> +
> static struct wl_proxy *
> proxy_create(struct wl_proxy *factory, const struct wl_interface *interface,
> uint32_t version)
> @@ -340,6 +406,10 @@ proxy_create(struct wl_proxy *factory, const struct wl_interface *interface,
> proxy->queue = factory->queue;
> proxy->refcount = 1;
> proxy->version = version;
> + if (!prepare_zombie(proxy)) {
> + free(proxy);
> + return NULL;
> + }
>
> proxy->object.id = wl_map_insert_new(&display->objects, 0, proxy);
>
> @@ -397,6 +467,11 @@ wl_proxy_create_for_id(struct wl_proxy *factory,
> proxy->refcount = 1;
> proxy->version = factory->version;
>
> + if (!prepare_zombie(proxy)) {
> + free(proxy);
> + return NULL;
> + }
> +
> wl_map_insert_at(&display->objects, 0, id, proxy);
>
> return proxy;
> @@ -405,18 +480,26 @@ wl_proxy_create_for_id(struct wl_proxy *factory,
> static void
> proxy_destroy(struct wl_proxy *proxy)
> {
> + bool need_zombie;
bool need_zombie = false;
Will be in v3.
(I'll wait a little while before pushing out a 3rd series.)
Thanks,
Derek
> +
> if (proxy->flags & WL_PROXY_FLAG_ID_DELETED)
> wl_map_remove(&proxy->display->objects, proxy->object.id);
> - else if (proxy->object.id < WL_SERVER_ID_START)
> - wl_map_insert_at(&proxy->display->objects, 0,
> - proxy->object.id, WL_ZOMBIE_OBJECT);
> - else
> + else if (proxy->object.id < WL_SERVER_ID_START) {
> + wl_map_insert_at(&proxy->display->objects,
> + WL_MAP_ENTRY_ZOMBIE,
> + proxy->object.id,
> + (void *)proxy->zombie);
> + need_zombie = true;
> + } else
> wl_map_insert_at(&proxy->display->objects, 0,
> proxy->object.id, NULL);
>
>
> proxy->flags |= WL_PROXY_FLAG_DESTROYED;
>
> + if (!need_zombie)
> + inter_zombie(proxy->zombie, NULL, WL_MAP_ENTRY_ZOMBIE);
> +
> proxy->refcount--;
> if (!proxy->refcount)
> free(proxy);
> @@ -828,20 +911,26 @@ display_handle_error(void *data,
> static void
> display_handle_delete_id(void *data, struct wl_display *display, uint32_t id)
> {
> - struct wl_proxy *proxy;
> + void *map_entry;
> + struct wl_proxy *proxy = NULL;
> + bool is_zombie;
>
> pthread_mutex_lock(&display->mutex);
>
> - proxy = wl_map_lookup(&display->objects, id);
> -
> - if (!proxy)
> + map_entry = wl_map_lookup(&display->objects, id);
> + is_zombie = wl_object_is_zombie(&display->objects, id);
> + if (!map_entry && !is_zombie)
> wl_log("error: received delete_id for unknown id (%u)\n", id);
>
> - if (proxy && proxy != WL_ZOMBIE_OBJECT)
> + if (!is_zombie)
> + proxy = map_entry;
> +
> + if (proxy)
> proxy->flags |= WL_PROXY_FLAG_ID_DELETED;
> - else
> + else {
> + inter_zombie(map_entry, NULL, WL_MAP_ENTRY_ZOMBIE);
> wl_map_remove(&display->objects, id);
> -
> + }
> pthread_mutex_unlock(&display->mutex);
> }
>
> @@ -1036,6 +1125,7 @@ wl_display_connect(const char *name)
> WL_EXPORT void
> wl_display_disconnect(struct wl_display *display)
> {
> + wl_map_for_each(&display->objects, inter_zombie, NULL);
> wl_connection_destroy(display->connection);
> wl_map_release(&display->objects);
> wl_event_queue_release(&display->default_queue);
> @@ -1228,7 +1318,7 @@ queue_event(struct wl_display *display, int len)
> return 0;
>
> proxy = wl_map_lookup(&display->objects, id);
> - if (!proxy || proxy == WL_ZOMBIE_OBJECT) {
> + if (!proxy || wl_object_is_zombie(&display->objects, id)) {
> wl_connection_consume(display->connection, size);
> return size;
> }
> diff --git a/src/wayland-private.h b/src/wayland-private.h
> index 58c01b6..a50174b 100644
> --- a/src/wayland-private.h
> +++ b/src/wayland-private.h
> @@ -57,9 +57,6 @@ struct wl_object {
> uint32_t id;
> };
>
> -extern struct wl_object global_zombie_object;
> -#define WL_ZOMBIE_OBJECT ((void*)&global_zombie_object)
> -
> int
> wl_interface_equal(const struct wl_interface *iface1,
> const struct wl_interface *iface2);
> @@ -69,7 +66,8 @@ wl_interface_equal(const struct wl_interface *iface1,
> * flags. If more flags are ever added, the implementation of wl_map will have
> * to change to allow for new flags */
> enum wl_map_entry_flags {
> - WL_MAP_ENTRY_LEGACY = (1 << 0)
> + WL_MAP_ENTRY_LEGACY = (1 << 0), /* Server side only */
> + WL_MAP_ENTRY_ZOMBIE = (1 << 0) /* Client side only */
> };
>
> struct wl_map {
> @@ -190,6 +188,9 @@ wl_connection_demarshal(struct wl_connection *connection,
> struct wl_map *objects,
> const struct wl_message *message);
>
> +bool
> +wl_object_is_zombie(struct wl_map *map, uint32_t id);
> +
> int
> wl_closure_lookup_objects(struct wl_closure *closure, struct wl_map *objects);
>
> diff --git a/src/wayland-util.c b/src/wayland-util.c
> index ce387f4..3a471a8 100644
> --- a/src/wayland-util.c
> +++ b/src/wayland-util.c
> @@ -153,8 +153,6 @@ wl_array_copy(struct wl_array *array, struct wl_array *source)
>
> /** \cond */
>
> -struct wl_object global_zombie_object;
> -
> int
> wl_interface_equal(const struct wl_interface *a, const struct wl_interface *b)
> {
>
More information about the wayland-devel
mailing list