From 962aefda42f7bbfb6d9b64b7b177e232213d8a59 Mon Sep 17 00:00:00 2001 From: Simon Ser Date: Mon, 31 Jan 2022 23:09:26 +0100 Subject: [PATCH] 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 Signed-off-by: Alexandros Frantzis --- src/wayland-server-core.h | 3 ++ src/wayland-server.c | 63 +++++++++++++++++++++++++++++++++++++++ tests/signal-test.c | 41 +++++++++++++++++++++++++ 3 files changed, 107 insertions(+) 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); +}