[PATCH wayland] server: allow removing any wl_listener from a signal when emitted
Giulio Camuffo
giuliocamuffo at gmail.com
Tue Sep 6 07:06:16 UTC 2016
wl_list_for_each_safe, which was used by wl_signal_emit is not really
safe. If a signal has two listeners, and the first one removes and
re-inits the second one, it would enter an infinite loop.
This commit adds a wl_list_iterate_safe() function which is used by
wl_signal_emit() and adds a test confirming it works.
wl_signal_emit had to be de-inlined because we need an iterator
function, which should stay private.
---
src/wayland-server-core.h | 17 ++---------
src/wayland-server.c | 21 ++++++++++++++
src/wayland-util.c | 29 +++++++++++++++++++
src/wayland-util.h | 29 +++++++++++--------
tests/signal-test.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 142 insertions(+), 26 deletions(-)
diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h
index 2c215e4..a0a3ead 100644
--- a/src/wayland-server-core.h
+++ b/src/wayland-server-core.h
@@ -341,21 +341,8 @@ wl_signal_get(struct wl_signal *signal, wl_notify_func_t notify)
return NULL;
}
-/** Emits this signal, notifying all registered listeners.
- *
- * \param signal The signal object that will emit the signal
- * \param data The data that will be emitted with the signal
- *
- * \memberof wl_signal
- */
-static inline void
-wl_signal_emit(struct wl_signal *signal, void *data)
-{
- struct wl_listener *l, *next;
-
- wl_list_for_each_safe(l, next, &signal->listener_list, link)
- l->notify(l, data);
-}
+void
+wl_signal_emit(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 9ecfd97..93e838a 100644
--- a/src/wayland-server.c
+++ b/src/wayland-server.c
@@ -1742,6 +1742,27 @@ wl_client_for_each_resource(struct wl_client *client,
wl_map_for_each(&client->objects, resource_iterator_helper, &context);
}
+static enum wl_iterator_result
+notify_listener(struct wl_list *link, void *data)
+{
+ struct wl_listener *l = wl_container_of(link, l, link);
+ l->notify(l, data);
+ return WL_ITERATOR_CONTINUE;
+}
+
+/** Emits this signal, notifying all registered listeners.
+ *
+ * \param signal The signal object that will emit the signal
+ * \param data The data that will be emitted with the signal
+ *
+ * \memberof wl_signal
+ */
+WL_EXPORT void
+wl_signal_emit(struct wl_signal *signal, void *data)
+{
+ wl_list_iterate_safe(&signal->listener_list, notify_listener, data);
+}
+
/** \cond */ /* Deprecated functions below. */
uint32_t
diff --git a/src/wayland-util.c b/src/wayland-util.c
index 639ccf8..c6193b7 100644
--- a/src/wayland-util.c
+++ b/src/wayland-util.c
@@ -93,6 +93,35 @@ wl_list_insert_list(struct wl_list *list, struct wl_list *other)
}
WL_EXPORT void
+wl_list_iterate_safe(struct wl_list *list,
+ wl_list_iterator_func_t it, void *data)
+{
+ struct wl_list temp;
+ struct wl_list *pos;
+
+ wl_list_init(&temp);
+
+ /* Take every element out of the list and put them in a temporary list.
+ * This way, the 'it' func can remove any element it wants from the list
+ * without troubles, because we always get the first element, not the
+ * one after the current, which may be invalid.
+ * 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. */
+ while (!wl_list_empty(list)) {
+ pos = list->next;
+
+ wl_list_remove(pos);
+ wl_list_insert(&temp, pos);
+
+ if (it(pos, data) == WL_ITERATOR_STOP)
+ break;
+ }
+
+ /* Now add back all the items in the original list. */
+ wl_list_insert_list(list, &temp);
+}
+
+WL_EXPORT void
wl_array_init(struct wl_array *array)
{
memset(array, 0, sizeof *array);
diff --git a/src/wayland-util.h b/src/wayland-util.h
index cacc122..34697b5 100644
--- a/src/wayland-util.h
+++ b/src/wayland-util.h
@@ -76,6 +76,17 @@ struct wl_interface {
const struct wl_message *events;
};
+/** \enum wl_iterator_result
+ *
+ * This enum represents the return value of an iterator function.
+ */
+enum wl_iterator_result {
+ /** Stop the iteration */
+ WL_ITERATOR_STOP,
+ /** Continue the iteration */
+ WL_ITERATOR_CONTINUE
+};
+
/** \class wl_list
*
* \brief doubly-linked list
@@ -139,6 +150,13 @@ wl_list_empty(const struct wl_list *list);
void
wl_list_insert_list(struct wl_list *list, struct wl_list *other);
+typedef enum wl_iterator_result (*wl_list_iterator_func_t)(struct wl_list *elm,
+ void *data);
+
+void
+wl_list_iterate_safe(struct wl_list *list,
+ wl_list_iterator_func_t iterator, void *data);
+
/**
* Retrieves a pointer to the containing struct of a given member item.
*
@@ -311,17 +329,6 @@ typedef int (*wl_dispatcher_func_t)(const void *, void *, uint32_t,
typedef void (*wl_log_func_t)(const char *, va_list) WL_PRINTF(1, 0);
-/** \enum wl_iterator_result
- *
- * This enum represents the return value of an iterator function.
- */
-enum wl_iterator_result {
- /** Stop the iteration */
- WL_ITERATOR_STOP,
- /** Continue the iteration */
- WL_ITERATOR_CONTINUE
-};
-
#ifdef __cplusplus
}
#endif
diff --git a/tests/signal-test.c b/tests/signal-test.c
index 7bbaa9f..1cc5883 100644
--- a/tests/signal-test.c
+++ b/tests/signal-test.c
@@ -115,3 +115,75 @@ TEST(signal_emit_to_more_listeners)
assert(3 * counter == count);
}
+
+static void notify_remove(struct wl_listener *l, void *data)
+{
+ struct wl_listener *l2 = data;
+ wl_list_remove(&l2->link);
+ wl_list_init(&l2->link);
+}
+
+static void notify(struct wl_listener *l, void *data)
+{}
+
+#define INIT \
+ wl_signal_init(&signal); \
+ wl_list_init(&l1.link); \
+ wl_list_init(&l2.link); \
+ wl_list_init(&l3.link); \
+
+TEST(signal_remove_listener)
+{
+ test_set_timeout(4);
+
+ struct wl_signal signal;
+ struct wl_listener l1, l2, l3;
+
+ l1.notify = notify_remove;
+ l2.notify = notify;
+ l3.notify = notify;
+
+ INIT
+ wl_signal_add(&signal, &l1);
+
+ wl_signal_emit(&signal, &l1);
+ wl_signal_emit(&signal, &l1);
+
+ INIT
+ wl_signal_add(&signal, &l1);
+ wl_signal_add(&signal, &l2);
+
+ wl_signal_emit(&signal, &l1);
+ wl_signal_emit(&signal, &l1);
+
+ INIT
+ wl_signal_add(&signal, &l1);
+ wl_signal_add(&signal, &l2);
+
+ wl_signal_emit(&signal, &l2);
+ wl_signal_emit(&signal, &l2);
+
+ INIT
+ wl_signal_add(&signal, &l1);
+ wl_signal_add(&signal, &l2);
+ wl_signal_add(&signal, &l3);
+
+ wl_signal_emit(&signal, &l1);
+ wl_signal_emit(&signal, &l1);
+
+ INIT
+ wl_signal_add(&signal, &l1);
+ wl_signal_add(&signal, &l2);
+ wl_signal_add(&signal, &l3);
+
+ wl_signal_emit(&signal, &l2);
+ wl_signal_emit(&signal, &l2);
+
+ INIT
+ wl_signal_add(&signal, &l1);
+ wl_signal_add(&signal, &l2);
+ wl_signal_add(&signal, &l3);
+
+ wl_signal_emit(&signal, &l3);
+ wl_signal_emit(&signal, &l3);
+}
--
2.9.3
More information about the wayland-devel
mailing list