This new function allows listeners to remove themselves or any other listener when called. This version only works if listeners are properly removed before they are free'd.
wl_signal_emit tries to be safe but it fails: it works fine if a handler removes its own listener, but not if it removes another one. It's not possible to patch wl_signal_emit directly as attempted in [1] because some projects using libwayland directly free destroy listeners without removing them from the list. Using this new strategy fails in this case, causing read-after-free errors. [1]: https://patchwork.freedesktop.org/patch/204641/ Signed-off-by: Simon Ser <cont...@emersion.fr> --- Addressed Markus' comments [1]. [1]: https://lists.freedesktop.org/archives/wayland-devel/2018-July/039042.html src/wayland-server-core.h | 3 ++ src/wayland-server.c | 50 +++++++++++++++++++++++ tests/signal-test.c | 86 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 139 insertions(+) diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h index 2e725d9..4a2948a 100644 --- a/src/wayland-server-core.h +++ b/src/wayland-server-core.h @@ -468,6 +468,9 @@ wl_signal_emit(struct wl_signal *signal, void *data) l->notify(l, data); } +void +wl_signal_emit_safe(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 eae8d2e..3d851f4 100644 --- a/src/wayland-server.c +++ b/src/wayland-server.c @@ -1932,6 +1932,56 @@ 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, safe against removal of any listener. + * + * wl_signal_emit tries to be safe but it fails: it works fine if a handler + * removes its own listener, but not if it removes another one. + * + * \note This function can only be used if listeners are properly removed before + * being free'd. + * + * \param signal The signal object that will emit the signal + * \param data The data that will be emitted with the signal + * + * \sa wl_signal_emit() + * + * \memberof wl_signal + */ +WL_EXPORT void +wl_signal_emit_safe(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. + * 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..dc762a4 100644 --- a/tests/signal-test.c +++ b/tests/signal-test.c @@ -115,3 +115,89 @@ TEST(signal_emit_to_more_listeners) assert(3 * counter == count); } + +struct signal +{ + struct wl_signal signal; + struct wl_listener l1, l2, l3; + int count; + struct wl_listener *current; +}; + +static void notify_remove(struct wl_listener *l, void *data) +{ + struct signal *sig = data; + wl_list_remove(&sig->current->link); + wl_list_init(&sig->current->link); + sig->count++; +} + +#define INIT \ + wl_signal_init(&signal.signal); \ + wl_list_init(&signal.l1.link); \ + wl_list_init(&signal.l2.link); \ + wl_list_init(&signal.l3.link); +#define CHECK_EMIT(expected) \ + signal.count = 0; \ + wl_signal_emit_safe(&signal.signal, &signal); \ + assert(signal.count == expected); + +TEST(signal_remove_listener) +{ + test_set_timeout(4); + + struct signal signal; + + signal.l1.notify = notify_remove; + signal.l2.notify = notify_remove; + signal.l3.notify = notify_remove; + + INIT + wl_signal_add(&signal.signal, &signal.l1); + + signal.current = &signal.l1; + CHECK_EMIT(1) + CHECK_EMIT(0) + + INIT + wl_signal_add(&signal.signal, &signal.l1); + wl_signal_add(&signal.signal, &signal.l2); + + CHECK_EMIT(2) + CHECK_EMIT(1) + + INIT + wl_signal_add(&signal.signal, &signal.l1); + wl_signal_add(&signal.signal, &signal.l2); + + signal.current = &signal.l2; + CHECK_EMIT(1) + CHECK_EMIT(1) + + INIT + wl_signal_add(&signal.signal, &signal.l1); + wl_signal_add(&signal.signal, &signal.l2); + wl_signal_add(&signal.signal, &signal.l3); + + signal.current = &signal.l1; + CHECK_EMIT(3) + CHECK_EMIT(2) + + INIT + wl_signal_add(&signal.signal, &signal.l1); + wl_signal_add(&signal.signal, &signal.l2); + wl_signal_add(&signal.signal, &signal.l3); + + signal.current = &signal.l2; + CHECK_EMIT(2) + CHECK_EMIT(2) + + INIT + wl_signal_add(&signal.signal, &signal.l1); + wl_signal_add(&signal.signal, &signal.l2); + wl_signal_add(&signal.signal, &signal.l3); + + signal.current = &signal.l3; + CHECK_EMIT(2) + CHECK_EMIT(2) +} -- 2.18.0 _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
