server: introduce wl_signal_emit_mutable

wl_signal_emit doesn't handle well situations where a listener removes
another listener. This can happen in practice: wlroots and Weston [1]
both have private helpers to workaround this defect.

wl_signal_emit can't be fixed without breaking the API. Instead,
introduce a new function. Callers need to make sure to always remove
listeners when they are free'd.

[1]: https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/457

Signed-off-by: Simon Ser <contact@emersion.fr>
Signed-off-by: Alexandros Frantzis <alexandros.frantzis@collabora.com>

3 files changed, 107 insertions, 0 deletions

diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h
index cbc70c0..ab5b448 100644
--- a/src/wayland-server-core.h
+++ b/src/wayland-server-core.h
@@ -481,6 +481,9 @@ wl_signal_emit(struct wl_signal *signal, void *data)
 		l->notify(l, data);
 }
 
+void
+wl_signal_emit_mutable(struct wl_signal *signal, void *data);
+
 typedef void (*wl_resource_destroy_func_t)(struct wl_resource *resource);
 
 /*
diff --git a/src/wayland-server.c b/src/wayland-server.c
index a503452..71b0360 100644
--- a/src/wayland-server.c
+++ b/src/wayland-server.c
@@ -2096,6 +2096,69 @@ wl_client_for_each_resource(struct wl_client *client,
 	wl_map_for_each(&client->objects, resource_iterator_helper, &context);
 }
 
+static void
+handle_noop(struct wl_listener *listener, void *data)
+{
+	/* Do nothing */
+}
+
+/** Emits this signal, notifying all registered listeners.
+ *
+ * A safer version of wl_signal_emit() which can gracefully handle additions
+ * and deletions of any signal listener from within listener notification
+ * callbacks.
+ *
+ * Listeners deleted during a signal emission and which have not already been
+ * notified at the time of deletion are not notified by that emission.
+ *
+ * Listeners added (or readded) during signal emission are ignored by that
+ * emission.
+ *
+ * Note that repurposing a listener without explicitly removing it and readding
+ * it is not supported and can lead to unexpected behavior.
+ *
+ * \param signal The signal object that will emit the signal
+ * \param data The data that will be emitted with the signal
+ *
+ * \memberof wl_signal
+ * \since 1.20.90
+ */
+WL_EXPORT void
+wl_signal_emit_mutable(struct wl_signal *signal, void *data)
+{
+	struct wl_listener cursor;
+	struct wl_listener end;
+
+	/* Add two special markers: one cursor and one end marker. This way, we
+	 * know that we've already called listeners on the left of the cursor
+	 * and that we don't want to call listeners on the right of the end
+	 * marker. The 'it' function can remove any element it wants from the
+	 * list without troubles.
+	 *
+	 * There was a previous attempt that used to steal the whole list of
+	 * listeners but then that broke wl_signal_get().
+	 *
+	 * wl_list_for_each_safe tries to be safe but it fails: it works fine
+	 * if the current item is removed, but not if the next one is. */
+	wl_list_insert(&signal->listener_list, &cursor.link);
+	cursor.notify = handle_noop;
+	wl_list_insert(signal->listener_list.prev, &end.link);
+	end.notify = handle_noop;
+
+	while (cursor.link.next != &end.link) {
+		struct wl_list *pos = cursor.link.next;
+		struct wl_listener *l = wl_container_of(pos, l, link);
+
+		wl_list_remove(&cursor.link);
+		wl_list_insert(pos, &cursor.link);
+
+		l->notify(l, data);
+	}
+
+	wl_list_remove(&cursor.link);
+	wl_list_remove(&end.link);
+}
+
 /** \cond INTERNAL */
 
 /** Initialize a wl_priv_signal object
diff --git a/tests/signal-test.c b/tests/signal-test.c
index 7bbaa9f..f7e1bd6 100644
--- a/tests/signal-test.c
+++ b/tests/signal-test.c
@@ -115,3 +115,44 @@ TEST(signal_emit_to_more_listeners)
 
 	assert(3 * counter == count);
 }
+
+struct signal_emit_mutable_data {
+	int count;
+	struct wl_listener *remove_listener;
+};
+
+static void
+signal_notify_mutable(struct wl_listener *listener, void *data)
+{
+	struct signal_emit_mutable_data *test_data = data;
+	test_data->count++;
+}
+
+static void
+signal_notify_and_remove_mutable(struct wl_listener *listener, void *data)
+{
+	struct signal_emit_mutable_data *test_data = data;
+	signal_notify_mutable(listener, test_data);
+	wl_list_remove(&test_data->remove_listener->link);
+}
+
+TEST(signal_emit_mutable)
+{
+	struct signal_emit_mutable_data data = {0};
+
+	/* l2 will remove l3 before l3 is notified */
+	struct wl_signal signal;
+	struct wl_listener l1 = {.notify = signal_notify_mutable};
+	struct wl_listener l2 = {.notify = signal_notify_and_remove_mutable};
+	struct wl_listener l3 = {.notify = signal_notify_mutable};
+
+	wl_signal_init(&signal);
+	wl_signal_add(&signal, &l1);
+	wl_signal_add(&signal, &l2);
+	wl_signal_add(&signal, &l3);
+
+	data.remove_listener = &l3;
+	wl_signal_emit_mutable(&signal, &data);
+
+	assert(data.count == 2);
+}