util: unified and enhanced zombie handing in wl_map

servers and clients now both use zombies.  Zombies are
interface pointers, so that they retain info for
demarshalling fds AND new_id args, which must be turned
into other zombies.

Demarshalling messages to zombies is handled by the
same wl_connection_demarshal code used for messages
to live objects.

Server zombies are eventually reaped after moving
through FIFO zombie ring, because there is no delete_id
request in the protocol.

Signed-off-by: Jonathan Leivent <jleivent@comcast.net>

src/connection.c

@@ -689,10 +689,14 @@ wl_closure_vmarshal(struct wl_object *sender, uint32_t opcode, va_list ap,
 }
 
 struct wl_closure *
-wl_connection_demarshal(struct wl_connection *connection,
+wl_connection_demarshal_common(struct wl_connection *connection,
 			uint32_t size,
 			struct wl_map *objects,
-			const struct wl_message *message)
+			const struct wl_message *message,
+			int (*handle_new_id)(void *data,
+						uint32_t id,
+						const struct wl_interface *interface),
+			void *data)
 {
 	uint32_t *p, *next, *end, length, length_in_u32, id;
 	int fd;
@@ -813,7 +817,7 @@ wl_connection_demarshal(struct wl_connection *connection,
 				goto err;
 			}
 
-			if (wl_map_reserve_new(objects, id) < 0) {
+			if (handle_new_id(data, id, message->types[i]) < 0) {
 				if (errno == EINVAL) {
 					wl_log("not a valid new object id (%u), "
 					       "message %s(%s)\n", id,
@@ -876,21 +880,51 @@ wl_connection_demarshal(struct wl_connection *connection,
 	return NULL;
 }
 
+static int
+reserve_new_id(void *data, uint32_t id, const struct wl_interface *interface)
+{
+	struct wl_map *objects = data;
+	return wl_map_reserve_new(objects, id);
+}
+
+struct wl_closure *
+wl_connection_demarshal(struct wl_connection *connection,
+			uint32_t size,
+			struct wl_map *objects,
+			const struct wl_message *message)
+{
+	return wl_connection_demarshal_common(connection,
+					      size,
+					      objects,
+					      message,
+					      &reserve_new_id,
+					      objects);
+}
+
+/* demarshal messages to zombies.  handle_new_id should create a new zombie. */
+void
+wl_connection_demarshal_zombie(struct wl_connection *connection,
+			       uint32_t size,
+			       struct wl_map *objects,
+			       const struct wl_message *message,
+			       int (*handle_new_id)(void* data,
+						      uint32_t id,
+						      const struct wl_interface *interface),
+			       void *data)
+{
+	struct wl_closure *closure = wl_connection_demarshal_common(connection,
+								    size,
+								    objects,
+								    message,
+								    handle_new_id,
+								    data);
+	wl_closure_destroy(closure);
+}
+
 bool
 wl_object_is_zombie(struct wl_map *map, uint32_t id)
 {
-	uint32_t flags;
+	return wl_map_lookup_zombie(map, id) != NULL;
-
-	/* Zombie objects only exist on the client side. */
-	if (map->side == WL_MAP_SERVER_SIDE)
-		return false;
-
-	/* Zombie objects can only have been created by the client. */
-	if (id >= WL_SERVER_ID_START)
-		return false;
-
-	flags = wl_map_lookup_flags(map, id);
-	return !!(flags & WL_MAP_ENTRY_ZOMBIE);
 }
 
 int
@@ -916,7 +950,7 @@ wl_closure_lookup_objects(struct wl_closure *closure, struct wl_map *objects)
 			object = wl_map_lookup(objects, id);
 			if (wl_object_is_zombie(objects, id)) {
 				/* references object we've already
-				 * destroyed client side */
+				 * destroyed */
 				object = NULL;
 			} else if (object == NULL && id != 0) {
 				wl_log("unknown object (%u), message %s(%s)\n",

src/wayland-client.c

@@ -50,16 +50,10 @@
 /** \cond */
 
 enum wl_proxy_flag {
-	WL_PROXY_FLAG_ID_DELETED = (1 << 0),
 	WL_PROXY_FLAG_DESTROYED = (1 << 1),
 	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;
@@ -412,66 +406,6 @@ wl_display_create_queue_with_name(struct wl_display *display, const char *name)
 	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 struct wl_zombie *
-prepare_zombie(struct wl_proxy *proxy)
-{
-	const struct wl_interface *interface = proxy->object.interface;
-	const struct wl_message *message;
-	int i, count;
-	struct wl_zombie *zombie = NULL;
-
-	/* If we hit an event with an FD, ensure we have a zombie object and
-	 * fill the fd_count slot for that event with the number of FDs for
-	 * that event. Interfaces with no events containing FDs will not have
-	 * zombie objects created. */
-	for (i = 0; i < interface->event_count; i++) {
-		message = &interface->events[i];
-		count = message_count_fds(message->signature);
-
-		if (!count)
-			continue;
-
-		if (!zombie) {
-			zombie = zalloc(sizeof(*zombie) +
-				        (interface->event_count * sizeof(int)));
-			if (!zombie)
-				return NULL;
-
-			zombie->event_count = interface->event_count;
-			zombie->fd_count = (int *) &zombie[1];
-		}
-
-		zombie->fd_count[i] = count;
-	}
-
-	return zombie;
-}
-
-static enum wl_iterator_result
-free_zombies(void *element, void *data, uint32_t flags)
-{
-	if (flags & WL_MAP_ENTRY_ZOMBIE)
-		free(element);
-
-	return WL_ITERATOR_CONTINUE;
-}
-
 static struct wl_proxy *
 proxy_create(struct wl_proxy *factory, const struct wl_interface *interface,
 	     uint32_t version)
@@ -564,21 +498,9 @@ wl_proxy_create_for_id(struct wl_proxy *factory,
 static void
 proxy_destroy(struct wl_proxy *proxy)
 {
-	if (proxy->flags & WL_PROXY_FLAG_ID_DELETED) {
+	wl_map_zombify(&proxy->display->objects,
-		wl_map_remove(&proxy->display->objects, proxy->object.id);
+		       proxy->object.id,
-	} else if (proxy->object.id < WL_SERVER_ID_START) {
+		       proxy->object.interface);
-		struct wl_zombie *zombie = prepare_zombie(proxy);
-
-		/* The map now contains the zombie entry, until the delete_id
-		 * event arrives. */
-		wl_map_insert_at(&proxy->display->objects,
-				 WL_MAP_ENTRY_ZOMBIE,
-				 proxy->object.id,
-				 zombie);
-	} else {
-		wl_map_insert_at(&proxy->display->objects, 0,
-				 proxy->object.id, NULL);
-	}
 
 	proxy->flags |= WL_PROXY_FLAG_DESTROYED;
 
@@ -1107,22 +1029,10 @@ display_handle_error(void *data,
 static void
 display_handle_delete_id(void *data, struct wl_display *display, uint32_t id)
 {
-	struct wl_proxy *proxy;
-
 	pthread_mutex_lock(&display->mutex);
 
-	proxy = wl_map_lookup(&display->objects, id);
+	if (wl_map_mark_deleted(&display->objects, id) != 0)
-
-	if (wl_object_is_zombie(&display->objects, id)) {
-		/* For zombie objects, the 'proxy' is actually the zombie
-		 * event-information structure, which we can free. */
-		free(proxy);
-		wl_map_remove(&display->objects, id);
-	} else if (proxy) {
-		proxy->flags |= WL_PROXY_FLAG_ID_DELETED;
-	} else {
 		wl_log("error: received delete_id for unknown id (%u)\n", id);
-	}
 
 	pthread_mutex_unlock(&display->mutex);
 }
@@ -1359,7 +1269,6 @@ WL_EXPORT void
 wl_display_disconnect(struct wl_display *display)
 {
 	wl_connection_destroy(display->connection);
-	wl_map_for_each(&display->objects, free_zombies, NULL);
 	wl_map_release(&display->objects);
 	wl_event_queue_release(&display->default_queue);
 	free(display->default_queue.name);
@@ -1537,6 +1446,40 @@ increase_closure_args_refcount(struct wl_closure *closure)
 	closure->proxy->refcount++;
 }
 
+/* zombify_new_id is only called by wl_connection_demarshal_zombie
+ * when the event it is demarhsalling has new_id argments, meaning
+ * they're server IDs.  Those server IDs need to have zombies created
+ * for them in case the server subsequently sends an event to them,
+ * which might contain fds or more new_ids. */
+static int
+zombify_new_id(void *displayp, uint32_t id, const struct wl_interface *interface)
+{
+	struct wl_display *display = displayp;
+
+	if (id >= WL_SERVER_ID_START) {
+		/* Make the server ID a zombie in case it gets sent an
+		 * event.  This handles the "domino effect" case where
+		 * new_id args in events sent to zombies are
+		 * subsequently sent events, with new_id args, and so
+		 * on. */
+		if (wl_map_insert_at(&display->objects, 0, id, (void*)interface) != 0) {
+			errno = EINVAL;
+			return -1;
+		}
+		if (wl_map_zombify(&display->objects, id, interface) != 0) {
+			errno = EINVAL;
+			return -1;
+		}
+		assert(wl_map_lookup_zombie(&display->objects, id) == interface);
+
+		return 0;
+	} else {
+		/* Unexpected: the id is a client ID */
+		errno = EINVAL;
+		return -1;
+	}
+}
+
 static int
 queue_event(struct wl_display *display, int len)
 {
@@ -1548,7 +1491,7 @@ queue_event(struct wl_display *display, int len)
 	struct wl_event_queue *queue;
 	struct timespec tp;
 	unsigned int time;
-	int num_zombie_fds;
+	const struct wl_interface *interface;
 
 	wl_connection_copy(display->connection, p, sizeof p);
 	id = p[0];
@@ -1557,30 +1500,66 @@ queue_event(struct wl_display *display, int len)
 	if (len < size)
 		return 0;
 
-	/* If our proxy is gone or a zombie, just eat the event (and any FDs,
-	 * if applicable). */
 	proxy = wl_map_lookup(&display->objects, id);
-	if (!proxy || wl_object_is_zombie(&display->objects, id)) {
+	if (!proxy) {
-		struct wl_zombie *zombie = wl_map_lookup(&display->objects, id);
+		interface = wl_map_lookup_zombie(&display->objects, id);
-		num_zombie_fds = (zombie && opcode < zombie->event_count) ?
-			zombie->fd_count[opcode] : 0;
 
 		if (debug_client) {
 			clock_gettime(CLOCK_REALTIME, &tp);
 			time = (tp.tv_sec * 1000000L) + (tp.tv_nsec / 1000);
 
-			fprintf(stderr, "[%7u.%03u] discarded [%s]#%d.[event %d]"
+			fprintf(stderr, "[%7u.%03u] discarded [%s]#%u.[event %d]"
-				"(%d fd, %d byte)\n",
+				"(%s, %d byte)\n",
 				time / 1000, time % 1000,
-				zombie ? "zombie" : "unknown",
+				interface ? "zombie" : "unknown",
 				id, opcode,
-				num_zombie_fds, size);
+				interface ? interface->name : "unknown interface",
+				size);
 		}
-		if (num_zombie_fds > 0)
-			wl_connection_close_fds_in(display->connection,
-						   num_zombie_fds);
 
-		wl_connection_consume(display->connection, size);
+		if (interface) {
+			if (opcode >= interface->event_count) {
+				wl_log("interface '%s' has no event %u\n",
+				       interface->name, opcode);
+				return -1;
+			}
+			message = &interface->events[opcode];
+			if (message == NULL) {
+				wl_log("interface '%s' opcode %u has no event\n",
+				       interface->name, opcode);
+				return -1;
+			}
+			if (message->name == NULL) {
+				wl_log("interface '%s' opcode %u has noname event,\n",
+				       interface->name, opcode);
+				return -1;
+			}
+			wl_connection_demarshal_zombie(display->connection, size,
+						       &display->objects,
+						       message,
+						       &zombify_new_id, display);
+		} else {
+			/* what to do if there is no zombie interface? The
+			 * original zombie code just consumed the size and
+			 * continued on.  The assumption was if there was no
+			 * zombie, then the reason was that there's no fds to
+			 * consume, because zombies were only created for proxies
+			 * that have fd-bearing events.  But, there may have been
+			 * new_id args, so that could still have caused problems.
+			 * Under the new zombie regime, which always creates
+			 * zombies for proxies (because even an unpatched server
+			 * sends delete_id events), the absence of a zombie is
+			 * unexpected and means we need to kill the client. */
+			wl_log("Missing zombie for ID %u, event %u\n", id, opcode);
+			return -1;
+			/* Consider the case in the old regime where the proxy had
+			 * no fd-bearing events and no new_id-bearing events.  In
+			 * such cases, when there was no zombie, consuming the
+			 * size and moving on was OK, but there was no way to tell
+			 * it was OK.  Now we know the difference, and will always
+			 * log this error.
+			 */
+		}
 		return size;
 	}
 

src/wayland-private.h

@@ -59,19 +59,23 @@ wl_interface_equal(const struct wl_interface *iface1,
 		   const struct wl_interface *iface2);
 
 /* Flags for wl_map_insert_new and wl_map_insert_at.  Flags can be queried with
- * wl_map_lookup_flags.  The current implementation has room for 1 bit worth of
+ * wl_map_lookup_flags.  The current implementation has room for 32 bits worth of
  * 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), /* Server side only */
-	WL_MAP_ENTRY_ZOMBIE = (1 << 0) /* Client side only */
 };
 
 struct wl_map {
 	struct wl_array client_entries;
 	struct wl_array server_entries;
 	uint32_t side;
+	/* the free_list is LIFO, the zombie list is FIFO */
 	uint32_t free_list;
+	uint32_t zombie_list_head;
+	uint32_t zombie_list_tail;
+	/* if zombie_list_count is < 0, then the zombie_list is disabled */
+	int32_t zombie_list_count;
 };
 
 typedef enum wl_iterator_result (*wl_iterator_func_t)(void *element,
@@ -93,12 +97,18 @@ wl_map_insert_at(struct wl_map *map, uint32_t flags, uint32_t i, void *data);
 int
 wl_map_reserve_new(struct wl_map *map, uint32_t i);
 
-void
+int
-wl_map_remove(struct wl_map *map, uint32_t i);
+wl_map_zombify(struct wl_map *map, uint32_t i, const struct wl_interface *interface);
+
+int
+wl_map_mark_deleted(struct wl_map *map, uint32_t i);
 
 void *
 wl_map_lookup(struct wl_map *map, uint32_t i);
 
+const struct wl_interface *
+wl_map_lookup_zombie(struct wl_map *map, uint32_t i);
+
 uint32_t
 wl_map_lookup_flags(struct wl_map *map, uint32_t i);
 
@@ -179,6 +189,26 @@ wl_closure_vmarshal(struct wl_object *sender,
 		    uint32_t opcode, va_list ap,
 		    const struct wl_message *message);
 
+struct wl_closure *
+wl_connection_demarshal_common(struct wl_connection *connection,
+			       uint32_t size,
+			       struct wl_map *objects,
+			       const struct wl_message *message,
+			       int (*handle_new_id)(void *data,
+						    uint32_t id,
+						    const struct wl_interface *interface),
+			       void *data);
+
+void
+wl_connection_demarshal_zombie(struct wl_connection *connection,
+			uint32_t size,
+			struct wl_map *objects,
+			const struct wl_message *message,
+			int (*handle_new_id)(void* data,
+					       uint32_t id,
+					       const struct wl_interface *interface),
+			void *data);
+
 struct wl_closure *
 wl_connection_demarshal(struct wl_connection *connection,
 			uint32_t size,

src/wayland-server.c

@@ -327,6 +327,38 @@ destroy_client_with_error(struct wl_client *client, const char *reason)
 	wl_client_destroy(client);
 }
 
+/* zombify_new_id_queue_delete_id_event is only called by
+ * wl_connection_demarshal_zombie when the request it is demarhsalling
+ * has new_id argments, meaning they're client IDs.  Those client IDs
+ * need to have zombies created for them in case the client
+ * subsequently sends a request to them, which might contain fds or
+ * more new_ids. */
+static int
+zombify_new_id_queue_delete_id_event(void *clientp, uint32_t id,
+				     const struct wl_interface *interface)
+{
+	struct wl_client *client = clientp;
+
+	if (id < WL_SERVER_ID_START) {
+		if (wl_map_insert_at(&client->objects, 0, id, (void*)interface) != 0) {
+			errno = EINVAL;
+			return -1;
+		}
+		if (wl_map_zombify(&client->objects, id, interface) != 0) {
+			errno = EINVAL;
+			return -1;
+		}
+		if (client->display_resource)
+			wl_resource_queue_event(client->display_resource,
+						WL_DISPLAY_DELETE_ID, id);
+
+		return 0;
+	} else {
+		/* Unexpected: the id is a server ID. */
+		return -1;
+	}
+}
+
 static int
 wl_client_connection_data(int fd, uint32_t mask, void *data)
 {
@@ -340,6 +372,7 @@ wl_client_connection_data(int fd, uint32_t mask, void *data)
 	uint32_t resource_flags;
 	int opcode, size, since;
 	int len;
+	const struct wl_interface *interface;
 
 	if (mask & WL_EVENT_HANGUP) {
 		wl_client_destroy(client);
@@ -383,9 +416,29 @@ wl_client_connection_data(int fd, uint32_t mask, void *data)
 		resource = wl_map_lookup(&client->objects, p[0]);
 		resource_flags = wl_map_lookup_flags(&client->objects, p[0]);
 		if (resource == NULL) {
-			wl_resource_post_error(client->display_resource,
+			interface = wl_map_lookup_zombie(&client->objects, p[0]);
-					       WL_DISPLAY_ERROR_INVALID_OBJECT,
+			if (interface == NULL) {
-					       "invalid object %u", p[0]);
+				wl_resource_post_error(client->display_resource,
+						       WL_DISPLAY_ERROR_INVALID_OBJECT,
+						       "invalid object %u", p[0]);
+				break;
+			}
+
+			if (opcode >= interface->method_count) {
+				wl_resource_post_error(client->display_resource,
+						       WL_DISPLAY_ERROR_INVALID_METHOD,
+						       "invalid method %d, object %s#%u",
+						       opcode,
+						       interface->name,
+						       p[0]);
+				break;
+			}
+
+			message = &interface->methods[opcode];
+			wl_connection_demarshal_zombie(client->connection, size,
+						       &client->objects, message,
+						       &zombify_new_id_queue_delete_id_event,
+						       client);
 			break;
 		}
 
@@ -735,7 +788,8 @@ remove_and_destroy_resource(void *element, void *data, uint32_t flags)
 {
 	struct wl_resource *resource = element;
 	struct wl_client *client = resource->client;
-	uint32_t id = resource->object.id;;
+	uint32_t id = resource->object.id;
+	const struct wl_interface *interface = resource->object.interface;
 
 	wl_signal_emit(&resource->deprecated_destroy_signal, resource);
 	/* Don't emit the new signal for deprecated resources, as that would
@@ -746,16 +800,12 @@ remove_and_destroy_resource(void *element, void *data, uint32_t flags)
 	if (resource->destroy)
 		resource->destroy(resource);
 
+	if (id < WL_SERVER_ID_START && client->display_resource)
+		wl_resource_queue_event(client->display_resource,
+					WL_DISPLAY_DELETE_ID, id);
+	
 	/* The resource should be cleared from the map before memory is freed. */
-	if (id < WL_SERVER_ID_START) {
+	wl_map_zombify(&client->objects, id, interface);
-		if (client->display_resource) {
-			wl_resource_queue_event(client->display_resource,
-						WL_DISPLAY_DELETE_ID, id);
-		}
-		wl_map_insert_at(&client->objects, 0, id, NULL);
-	} else {
-		wl_map_remove(&client->objects, id);
-	}
 
 	if (!(flags & WL_MAP_ENTRY_LEGACY))
 		free(resource);

src/wayland-util.c

@@ -30,6 +30,7 @@
 #include <stdio.h>
 #include <string.h>
 #include <stdarg.h>
+#include <assert.h>
 
 #include "wayland-util.h"
 #include "wayland-private.h"
@@ -169,20 +170,48 @@ wl_interface_equal(const struct wl_interface *a, const struct wl_interface *b)
 	return a == b || strcmp(a->name, b->name) == 0;
 }
 
-union map_entry {
+struct map_entry {
-	uintptr_t next;
+	uint32_t next;
+	uint32_t flags : 8*sizeof(uint32_t) - 3;
+	uint32_t zombie : 1;
+	uint32_t freelisted : 1;
+	uint32_t deleted : 1;
 	void *data;
 };
 
-#define map_entry_is_free(entry) ((entry).next & 0x1)
+int32_t max_zombie_list_count = 64;
-#define map_entry_get_data(entry) ((void *)((entry).next & ~(uintptr_t)0x3))
+
-#define map_entry_get_flags(entry) (((entry).next >> 1) & 0x1)
+/* We cannot use NULL (0) to denote null links in the free list or
+   zombie list because index 0 is allowed on the server side: id ==
+   WL_SERVER_ID_START --> index == 0.  Instead, use: */
+static const uint32_t map_null_link = ~0;
+
+static inline int
+map_entry_is_free(struct map_entry *entry)
+{
+	return entry->zombie || entry->freelisted;
+}
+
+static inline void
+map_entry_clear(struct map_entry *entry)
+{
+	entry->next = map_null_link;
+	entry->flags = 0;
+	entry->zombie = 0;
+	entry->freelisted = 0;
+	entry->deleted = 0;
+	entry->data = NULL;
+}
 
 void
 wl_map_init(struct wl_map *map, uint32_t side)
 {
 	memset(map, 0, sizeof *map);
 	map->side = side;
+	map->free_list = map_null_link;
+	map->zombie_list_head = map_null_link;
+	map->zombie_list_tail = map_null_link;
+	map->zombie_list_count = 0;
 }
 
 void
@@ -195,7 +224,7 @@ wl_map_release(struct wl_map *map)
 uint32_t
 wl_map_insert_new(struct wl_map *map, uint32_t flags, void *data)
 {
-	union map_entry *start, *entry;
+	struct map_entry *start, *entry;
 	struct wl_array *entries;
 	uint32_t base;
 	uint32_t count;
@@ -208,9 +237,10 @@ wl_map_insert_new(struct wl_map *map, uint32_t flags, void *data)
 		base = WL_SERVER_ID_START;
 	}
 
-	if (map->free_list) {
+	if (map->free_list != map_null_link) {
 		start = entries->data;
-		entry = &start[map->free_list >> 1];
+		entry = &start[map->free_list];
+		assert(entry->freelisted);
 		map->free_list = entry->next;
 	} else {
 		entry = wl_array_add(entries, sizeof *entry);
@@ -235,20 +265,21 @@ wl_map_insert_new(struct wl_map *map, uint32_t flags, void *data)
 		errno = ENOSPC;
 		return 0;
 	}
+	map_entry_clear(entry);
 	entry->data = data;
-	entry->next |= (flags & 0x1) << 1;
+	entry->flags = flags;
-
 	return count + base;
 }
 
 int
 wl_map_insert_at(struct wl_map *map, uint32_t flags, uint32_t i, void *data)
 {
-	union map_entry *start;
+	struct map_entry *start;
 	uint32_t count;
 	struct wl_array *entries;
 
 	if (i < WL_SERVER_ID_START) {
+		assert(i == 0 || map->side == WL_MAP_SERVER_SIDE);
 		entries = &map->client_entries;
 	} else {
 		entries = &map->server_entries;
@@ -272,8 +303,10 @@ wl_map_insert_at(struct wl_map *map, uint32_t flags, uint32_t i, void *data)
 	}
 
 	start = entries->data;
+
+	map_entry_clear(&start[i]);
 	start[i].data = data;
-	start[i].next |= (flags & 0x1) << 1;
+	start[i].flags = flags;
 
 	return 0;
 }
@@ -281,7 +314,7 @@ wl_map_insert_at(struct wl_map *map, uint32_t flags, uint32_t i, void *data)
 int
 wl_map_reserve_new(struct wl_map *map, uint32_t i)
 {
-	union map_entry *start;
+	struct map_entry *start;
 	uint32_t count;
 	struct wl_array *entries;
 
@@ -318,10 +351,16 @@ wl_map_reserve_new(struct wl_map *map, uint32_t i)
 			return -1;
 
 		start = entries->data;
-		start[i].data = NULL;
+		map_entry_clear(&start[i]);
 	} else {
 		start = entries->data;
-		if (start[i].data != NULL) {
+
+		assert(!start[i].freelisted);
+
+		/* In the new zombie regime, there may be zombies in
+		 * any map, even opposite side ones.  So a test here
+		 * for start[i].data != NULL is no longer right.*/
+		if (!map_entry_is_free(&start[i])) {
 			errno = EINVAL;
 			return -1;
 		}
@@ -330,34 +369,128 @@ wl_map_reserve_new(struct wl_map *map, uint32_t i)
 	return 0;
 }
 
-void
+int
-wl_map_remove(struct wl_map *map, uint32_t i)
+wl_map_zombify(struct wl_map *map, uint32_t i, const struct wl_interface *interface)
 {
-	union map_entry *start;
+	struct map_entry *start, *entry;
 	struct wl_array *entries;
+	bool use_zombie_list;
+	uint32_t count;
+	static bool checked_env = false;
+	const char *max_var;
+
+	assert(i != 0);
+
+	if (i < WL_SERVER_ID_START) {
+		use_zombie_list = false;
+		entries = &map->client_entries;
+	} else {
+		use_zombie_list = (map->side == WL_MAP_SERVER_SIDE) &&
+			(map->zombie_list_count >= 0);
+		entries = &map->server_entries;
+		i -= WL_SERVER_ID_START;
+	}
+
+	start = entries->data;
+	count = entries->size / sizeof *start;
+
+	if (i >= count)
+		return -1;
+
+	if (start[i].deleted) {
+		/* No need to make this entry into a zombie if it has
+		   already been in a delete_id message - put it on the
+		   free list instead */
+		start[i].next = map->free_list;
+		start[i].freelisted = 1;
+		map->free_list = i;
+		return 0;
+	}
+
+	start[i].data = (void*)interface;
+	start[i].zombie = 1;
+	start[i].next = map_null_link;
+
+	if (use_zombie_list) {
+		if (!checked_env) {
+			checked_env = true;
+			max_var = getenv("WAYLAND_MAX_ZOMBIE_LIST_COUNT");
+			if (max_var && *max_var)
+				max_zombie_list_count = atoi(max_var);
+		}
+
+		if (map->zombie_list_tail != map_null_link)
+			start[map->zombie_list_tail].next = i;
+		else
+			map->zombie_list_head = i;
+		map->zombie_list_tail = i;
+		map->zombie_list_count++;
+
+		if (map->zombie_list_count > max_zombie_list_count) {
+			i = map->zombie_list_head;
+			entry = &start[i];
+			map->zombie_list_head = entry->next;
+			if (map->zombie_list_head == map_null_link)
+				map->zombie_list_tail = map_null_link;
+			map->zombie_list_count--;
+
+			entry->next = map->free_list;
+			entry->freelisted = 1;
+			entry->zombie = 0;
+			map->free_list = i;
+		}
+	}
+	return 0;
+}
+
+int
+wl_map_mark_deleted(struct wl_map *map, uint32_t i)
+{
+	struct map_entry *start;
+	struct wl_array *entries;
+	uint32_t count;
+
+	assert(i != 0);
 
 	if (i < WL_SERVER_ID_START) {
 		if (map->side == WL_MAP_SERVER_SIDE)
-			return;
+			return 0;
 
 		entries = &map->client_entries;
 	} else {
 		if (map->side == WL_MAP_CLIENT_SIDE)
-			return;
+			return 0;
 
 		entries = &map->server_entries;
 		i -= WL_SERVER_ID_START;
 	}
 
 	start = entries->data;
-	start[i].next = map->free_list;
+	count = entries->size / sizeof *start;
-	map->free_list = (i << 1) | 1;
+
+	if (i >= count)
+		return -1;
+
+	/* turn off the zombie list - because the zombie_list is not
+	   needed if we are receiving delete_id messages, and is
+	   incompatible with randomly moving zombies directly to the
+	   free list */
+	map->zombie_list_count = -1;
+
+	start[i].deleted = 1;
+	if (start[i].zombie) {
+		start[i].next = map->free_list;
+		start[i].freelisted = 1;
+		start[i].zombie = 0;
+		map->free_list = i;
+	}
+	return 0;
 }
 
 void *
 wl_map_lookup(struct wl_map *map, uint32_t i)
 {
-	union map_entry *start;
+	struct map_entry *start;
 	uint32_t count;
 	struct wl_array *entries;
 
@@ -371,8 +504,31 @@ wl_map_lookup(struct wl_map *map, uint32_t i)
 	start = entries->data;
 	count = entries->size / sizeof *start;
 
-	if (i < count && !map_entry_is_free(start[i]))
+	if (i < count && !map_entry_is_free(&start[i]))
-		return map_entry_get_data(start[i]);
+		return start[i].data;
+
+	return NULL;
+}
+
+const struct wl_interface *
+wl_map_lookup_zombie(struct wl_map *map, uint32_t i)
+{
+	struct map_entry *start;
+	uint32_t count;
+	struct wl_array *entries;
+
+	if (i < WL_SERVER_ID_START) {
+		entries = &map->client_entries;
+	} else {
+		entries = &map->server_entries;
+		i -= WL_SERVER_ID_START;
+	}
+
+	start = entries->data;
+	count = entries->size / sizeof *start;
+
+	if (i < count && start[i].zombie)
+		return start[i].data;
 
 	return NULL;
 }
@@ -380,7 +536,7 @@ wl_map_lookup(struct wl_map *map, uint32_t i)
 uint32_t
 wl_map_lookup_flags(struct wl_map *map, uint32_t i)
 {
-	union map_entry *start;
+	struct map_entry *start;
 	uint32_t count;
 	struct wl_array *entries;
 
@@ -394,8 +550,8 @@ wl_map_lookup_flags(struct wl_map *map, uint32_t i)
 	start = entries->data;
 	count = entries->size / sizeof *start;
 
-	if (i < count && !map_entry_is_free(start[i]))
+	if (i < count && !map_entry_is_free(&start[i]))
-		return map_entry_get_flags(start[i]);
+		return start[i].flags;
 
 	return 0;
 }
@@ -404,16 +560,16 @@ static enum wl_iterator_result
 for_each_helper(struct wl_array *entries, wl_iterator_func_t func, void *data)
 {
 	enum wl_iterator_result ret = WL_ITERATOR_CONTINUE;
-	union map_entry entry, *start;
+	struct map_entry entry, *start;
 	size_t count;
 
-	start = (union map_entry *) entries->data;
+	start = (struct map_entry *) entries->data;
-	count = entries->size / sizeof(union map_entry);
+	count = entries->size / sizeof(struct map_entry);
 
 	for (size_t idx = 0; idx < count; idx++) {
 		entry = start[idx];
-		if (entry.data && !map_entry_is_free(entry)) {
+		if (entry.data && !map_entry_is_free(&entry)) {
-			ret = func(map_entry_get_data(entry), data, map_entry_get_flags(entry));
+			ret = func(entry.data, data, entry.flags);
 			if (ret != WL_ITERATOR_CONTINUE)
 				break;
 		}

tests/connection-test.c

@@ -393,6 +393,7 @@ demarshal(struct marshal_data *data, const char *format,
 	assert(closure);
 	wl_closure_invoke(closure, WL_CLOSURE_INVOKE_SERVER, &object, 0, data);
 	wl_closure_destroy(closure);
+	wl_map_release(&objects);
 }
 
 TEST(connection_demarshal)
@@ -476,6 +477,7 @@ marshal_demarshal(struct marshal_data *data,
 	assert(closure);
 	wl_closure_invoke(closure, WL_CLOSURE_INVOKE_SERVER, &object, 0, data);
 	wl_closure_destroy(closure);
+	wl_map_release(&objects);
 }
 
 TEST(connection_marshal_demarshal)
@@ -540,6 +542,7 @@ expected_fail_demarshal(struct marshal_data *data, const char *format,
 
 	assert(closure == NULL);
 	assert(errno == expected_error);
+	wl_map_release(&objects);
 }
 
 TEST(connection_demarshal_null_strings)

tests/map-test.c

@@ -89,7 +89,8 @@ TEST(map_remove)
 	assert(wl_map_lookup(&map, j) == &b);
 	assert(wl_map_lookup(&map, k) == &c);
 
-	wl_map_remove(&map, j);
+	wl_map_mark_deleted(&map, j);
+	wl_map_zombify(&map, j, NULL);
 	assert(wl_map_lookup(&map, j) == NULL);
 
 	/* Verify that we insert d at the hole left by removing b */
@@ -107,15 +108,18 @@ TEST(map_flags)
 
 	wl_map_init(&map, WL_MAP_SERVER_SIDE);
 	i = wl_map_insert_new(&map, 0, &a);
-	j = wl_map_insert_new(&map, 1, &b);
+	uint32_t flag_value = 0xabcdef10;
+	/* 3 bits of flags are used internally, so we lose the high 3 here: */
+	uint32_t high_truncated_flag_value = (flag_value << 3) >> 3;
+	j = wl_map_insert_new(&map, high_truncated_flag_value, &b);
 	assert(i == WL_SERVER_ID_START);
 	assert(j == WL_SERVER_ID_START + 1);
 
 	assert(wl_map_lookup(&map, i) == &a);
 	assert(wl_map_lookup(&map, j) == &b);
 
-    assert(wl_map_lookup_flags(&map, i) == 0);
+	assert(wl_map_lookup_flags(&map, i) == 0);
-    assert(wl_map_lookup_flags(&map, j) == 1);
+	assert(wl_map_lookup_flags(&map, j) == high_truncated_flag_value);
 
 	wl_map_release(&map);
 }