client/server: add delete_id request as a fake sync

Add a delete_id request so that the server can reap
zombies once the client acks the deletion.  As a
result, the wl_map zombie_list would only be used
for clients that do not have this change.

To keep compatibility, the delete_id request is
dressed as a sync request, but carries a server
object ID instead.

Update tests to compensate for additional handshake
that sets up delete_id requests.

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

src/wayland-client.c

@@ -104,6 +104,7 @@ struct wl_display {
 	int reader_count;
 	uint32_t read_serial;
 	pthread_cond_t reader_cond;
+	bool use_sync_for_delete_id_requests;
 };
 
 /** \endcond */
@@ -495,11 +496,74 @@ wl_proxy_create_for_id(struct wl_proxy *factory,
 	return proxy;
 }
 
+/* In the current protocol, the client has no equivalent to a
+ * wl_display delete_id event.  But it can benefit from one.  Here we
+ * hijack the wl_display sync request such that the client will send a
+ * sync with a server id instead of a client id, which will indicate
+ * it is really being used as a delete_id request.  The client will
+ * only do this if the server had previously sent a delete_id event
+ * for WL_DELETE_ID_HANDSHAKE, indicating it would accept such
+ * hijacked syncs.  */
+static void
+marshal_fake_sync_for_delete_id(struct wl_display *display, uint32_t id)
+{
+	if (id < WL_SERVER_ID_START) {
+		wl_log("error: marshal_fake_sync_for_delete_id on client id %u\n", id);
+		return;
+	}
+
+	/* We have to call wl_closure_marshal directly because we
+	 * already have the display mutex */
+
+	struct wl_proxy *proxy = &display->proxy;
+	const struct wl_message *message =
+		&proxy->object.interface->methods[WL_DISPLAY_SYNC];
+
+	union wl_argument arg;
+	struct wl_closure *closure;
+	/* Even though the sync request takes a new_id arg,
+	 * wl_closure_marshal wants an obj arg for the new_id arg,
+	 * meaning it wants an actual object instead of an id.  So we
+	 * fake up an object.  Fortunately, it only uses the id
+	 * field. */
+	struct wl_object fake_obj;
+	fake_obj.interface = NULL;
+	fake_obj.implementation = NULL;
+	fake_obj.id = id;
+	arg.o = &fake_obj;
+	closure = wl_closure_marshal(&proxy->object, WL_DISPLAY_SYNC, &arg, message);
+
+	if (closure == NULL) {
+		wl_log("Error marshalling request: %s\n", strerror(errno));
+		display_fatal_error(display, errno);
+		return;
+	}
+
+	if (debug_client) {
+		struct wl_event_queue *queue;
+
+		queue = wl_proxy_get_queue(proxy);
+		wl_closure_print(closure, &proxy->object, true, false, NULL,
+				 wl_event_queue_get_name(queue));
+	}
+
+	if (wl_closure_send(closure, display->connection)) {
+		wl_log("Error sending request: %s\n", strerror(errno));
+		display_fatal_error(display, errno);
+	}
+
+	wl_closure_destroy(closure);
+}
+
 static void
 proxy_destroy(struct wl_proxy *proxy)
 {
-	wl_map_zombify(&proxy->display->objects,
+	struct wl_display *display = proxy->display;
-		       proxy->object.id,
+	uint32_t id = proxy->object.id;
+	if (display->use_sync_for_delete_id_requests && id >= WL_SERVER_ID_START)
+		marshal_fake_sync_for_delete_id(display, id);
+
+	wl_map_zombify(&display->objects, id,
 		       proxy->object.interface);
 
 	proxy->flags |= WL_PROXY_FLAG_DESTROYED;
@@ -1031,8 +1095,23 @@ display_handle_delete_id(void *data, struct wl_display *display, uint32_t id)
 {
 	pthread_mutex_lock(&display->mutex);
 
-	if (wl_map_mark_deleted(&display->objects, id) != 0)
+	if (id == WL_DELETE_ID_HANDSHAKE) {
+		/* The server has sent a special "impossible" delete_id event
+		 * to indicate that it can accept delete_id requests.  Note
+		 * that in the display, and reply with a similar delete_id
+		 * request */
+		/* The client doesn't have to return the handshake.
+		 * If it doesn't, the server will learn that the
+		 * client can send delete_id requests the first time
+		 * the client does so, if it does so.  But that might
+		 * be after the server has begun to recycle its IDs.
+		 * Unless the server has a large enough zombie
+		 * ring. */
+		display->use_sync_for_delete_id_requests = true;
+		marshal_fake_sync_for_delete_id(display, id);
+	} else if (wl_map_mark_deleted(&display->objects, id) != 0) {
 		wl_log("error: received delete_id for unknown id (%u)\n", id);
+	}
 
 	pthread_mutex_unlock(&display->mutex);
 }
@@ -1144,6 +1223,7 @@ wl_display_connect_to_fd(int fd)
 	pthread_mutex_init(&display->mutex, NULL);
 	pthread_cond_init(&display->reader_cond, NULL);
 	display->reader_count = 0;
+	display->use_sync_for_delete_id_requests = false;
 
 	if (wl_map_insert_at(&display->objects, 0, 0, NULL) == -1)
 		goto err_connection;
@@ -1472,6 +1552,9 @@ zombify_new_id(void *displayp, uint32_t id, const struct wl_interface *interface
 		}
 		assert(wl_map_lookup_zombie(&display->objects, id) == interface);
 
+		if (display->use_sync_for_delete_id_requests)
+			marshal_fake_sync_for_delete_id(display, id);
+
 		return 0;
 	} else {
 		/* Unexpected: the id is a client ID */

src/wayland-private.h

@@ -47,6 +47,7 @@
 #define WL_SERVER_ID_START 0xff000000
 #define WL_MAP_MAX_OBJECTS 0x00f00000
 #define WL_CLOSURE_MAX_ARGS 20
+#define WL_DELETE_ID_HANDSHAKE 0xffffffff
 
 struct wl_object {
 	const struct wl_interface *interface;
@@ -100,6 +101,9 @@ wl_map_reserve_new(struct wl_map *map, uint32_t i);
 int
 wl_map_zombify(struct wl_map *map, uint32_t i, const struct wl_interface *interface);
 
+void
+wl_map_disable_zombie_list(struct wl_map *map);
+
 int
 wl_map_mark_deleted(struct wl_map *map, uint32_t i);
 

src/wayland-server.c

@@ -359,6 +359,22 @@ zombify_new_id_queue_delete_id_event(void *clientp, uint32_t id,
 	}
 }
 
+/* Only used for demarshalling wl_display sync messages */
+static int
+handle_new_id_for_sync(void *data, uint32_t id, const struct wl_interface *interface)
+{
+	struct wl_map *objects = data;
+
+	if (id >= WL_SERVER_ID_START)
+		/* Assume this is being called do to a sync request
+		 * that is really a delete_id request - which will be
+		 * handled in display_sync.
+		 */
+		return 0;
+
+	return wl_map_reserve_new(objects, id);
+}
+
 static int
 wl_client_connection_data(int fd, uint32_t mask, void *data)
 {
@@ -467,9 +483,24 @@ wl_client_connection_data(int fd, uint32_t mask, void *data)
 			break;
 		}
 
-
+		if (resource == client->display_resource &&
-		closure = wl_connection_demarshal(client->connection, size,
+		    strcmp(message->name,"sync") == 0)
-						  &client->objects, message);
+			/* This is a wl_display::sync message, which
+			 * might be being used as a delete_id request.
+			 * Avoid declaring a server ID in the new_ID
+			 * field as an error by using
+			 * handle_new_id_for_sync as the new_id
+			 * handler.  Would be nice to use opcode ==
+			 * WL_DISPLAY_SYNC, but WL_DISPLAY_SYNC only
+			 * gets defined for clients.
+			 */
+			closure = wl_connection_demarshal_common(client->connection, size,
+								 &client->objects, message,
+								 &handle_new_id_for_sync,
+								 &client->objects);
+		else
+			closure = wl_connection_demarshal(client->connection, size,
+							  &client->objects, message);
 
 		if (closure == NULL && errno == ENOMEM) {
 			wl_resource_post_no_memory(resource);
@@ -575,6 +606,8 @@ WL_EXPORT struct wl_client *
 wl_client_create(struct wl_display *display, int fd)
 {
 	struct wl_client *client;
+	bool do_delete_id_handshake = false;
+	const char *env;
 
 	client = zalloc(sizeof *client);
 	if (client == NULL)
@@ -611,6 +644,28 @@ wl_client_create(struct wl_display *display, int fd)
 
 	wl_priv_signal_emit(&display->create_client_signal, client);
 
+	env = getenv("WAYLAND_SERVER_DELETE_ID_HANDSHAKE");
+	if (env && (strcmp(env,"0")==0 || strcasecmp(env,"no")==0 ||
+		    strcasecmp(env,"false")==0))
+		do_delete_id_handshake = false;
+	else
+		do_delete_id_handshake = true;
+
+	if (do_delete_id_handshake) {
+		/* Send the client a fake delete_id event with id
+		 * WL_DELETE_ID_HANDSHAKE so that a patched client
+		 * will know this is a patched server that can handle
+		 * delete_id requests.  Unpatched clients will log
+		 * this but otherwise ignore it.  Patched clients will
+		 * reply with the same id in a delete_id request(!),
+		 * to indicate that they will take the responsibility
+		 * to send future delete_id requests.
+		 */
+		wl_resource_queue_event(client->display_resource,
+					WL_DISPLAY_DELETE_ID,
+					WL_DELETE_ID_HANDSHAKE);
+	}
+
 	return client;
 
 err_map:
@@ -1079,6 +1134,23 @@ display_sync(struct wl_client *client,
 	struct wl_resource *callback;
 	uint32_t serial;
 
+	if (id >= WL_SERVER_ID_START) {
+		/* This is not a sync request.  It's really a
+		   delete_id request.  A true sync request would have
+		   a client id instead. */
+		if (id == WL_DELETE_ID_HANDSHAKE) {
+			/* the id == WL_DELETE_ID_HANDSHAKE case is only to tell
+			 * us that the client will send other delete_id requests
+			 * later
+			 */
+			wl_map_disable_zombie_list(&client->objects);
+		}
+		else if (wl_map_mark_deleted(&client->objects, id) != 0)
+			wl_log("error: received delete_id for unknown id (%u)\n", id);
+
+		return;
+	}
+
 	callback = wl_resource_create(client, &wl_callback_interface, 1, id);
 	if (callback == NULL) {
 		wl_client_post_no_memory(client);

src/wayland-util.c

@@ -443,6 +443,13 @@ wl_map_zombify(struct wl_map *map, uint32_t i, const struct wl_interface *interf
 	return 0;
 }
 
+
+void
+wl_map_disable_zombie_list(struct wl_map *map)
+{
+	map->zombie_list_count = -1;
+}
+
 int
 wl_map_mark_deleted(struct wl_map *map, uint32_t i)
 {
@@ -475,7 +482,7 @@ wl_map_mark_deleted(struct wl_map *map, uint32_t i)
 	   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;
+	wl_map_disable_zombie_list(map);
 
 	start[i].deleted = 1;
 	if (start[i].zombie) {

tests/connection-test.c

@@ -818,6 +818,11 @@ TEST(request_bogus_size)
 
 	test_set_timeout(1);
 
+	/* Because this test reads the expected error off the wire
+	   directly without anticipating any prior event from the
+	   server, turn off the delete_id handshake event: */
+	assert(setenv("WAYLAND_SERVER_DELETE_ID_HANDSHAKE","0",1) == 0);
+
 	/*
 	 * The manufactured message has real size 12. Test all bogus sizes
 	 * smaller than that, and zero as the last one since wl_closure_init

tests/protocol-logger-test.c

@@ -59,6 +59,13 @@ struct message {
 	const char *message_name;
 	int args_count;
 } messages[] = {
+	{ /* this is the WAYLAND_SERVER_DELETE_ID_HANDSHAKE message */
+		.type = WL_PROTOCOL_LOGGER_EVENT,
+		.class = "wl_display",
+		.opcode = 1,
+		.message_name = "delete_id",
+		.args_count = 1,
+	},
 	{
 		.type = WL_PROTOCOL_LOGGER_REQUEST,
 		.class = "wl_display",