[PATCH v2 wayland 07/11] client: Replace the singleton zombie with bespoke zombies

Derek Foreman derekf at osg.samsung.com
Thu Apr 13 16:51:49 UTC 2017


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))
+		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;
+
 	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)
 {
-- 
2.11.0



More information about the wayland-devel mailing list