[PATCH] core: implement a safe wl_signal_emit

Simon Ser contact at emersion.fr
Wed Feb 14 15:23:39 UTC 2018


The previous implementation tried to be safe but wasn't: a
listener couldn't remove the next one.

This removes the need for wl_priv_signal. newsignal-test is now
merged into signal-test.
---
This is followup of "RFC: server: implement wl_priv_signal_emit without
emit_list" [1] which removes wl_priv_signal.

[1]: https://lists.freedesktop.org/archives/wayland-devel/2018-January/036711.html
 Makefile.am               |   3 -
 src/wayland-private.h     |  17 ---
 src/wayland-server-core.h |  28 +++-
 src/wayland-server.c      | 141 ++++---------------
 tests/newsignal-test.c    | 337 ----------------------------------------------
 tests/signal-test.c       | 222 +++++++++++++++++++++++++++++-
 6 files changed, 270 insertions(+), 478 deletions(-)
 delete mode 100644 tests/newsignal-test.c

diff --git a/Makefile.am b/Makefile.am
index 322d6b8..5db0347 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -171,7 +171,6 @@ built_test_programs =				\
 	socket-test				\
 	queue-test				\
 	signal-test				\
-	newsignal-test				\
 	resources-test				\
 	message-test				\
 	headers-test				\
@@ -252,8 +251,6 @@ queue_test_LDADD = libtest-runner.la
 signal_test_SOURCES = tests/signal-test.c
 signal_test_LDADD = libtest-runner.la
 # wayland-server.c is needed here to access wl_priv_* functions
-newsignal_test_SOURCES = tests/newsignal-test.c src/wayland-server.c
-newsignal_test_LDADD = libtest-runner.la
 resources_test_SOURCES = tests/resources-test.c
 resources_test_LDADD = libtest-runner.la
 message_test_SOURCES = tests/message-test.c
diff --git a/src/wayland-private.h b/src/wayland-private.h
index 12b9032..e2803ee 100644
--- a/src/wayland-private.h
+++ b/src/wayland-private.h
@@ -236,23 +236,6 @@ zalloc(size_t s)
 	return calloc(1, s);
 }
 
-struct wl_priv_signal {
-	struct wl_list listener_list;
-	struct wl_list emit_list;
-};
-
-void
-wl_priv_signal_init(struct wl_priv_signal *signal);
-
-void
-wl_priv_signal_add(struct wl_priv_signal *signal, struct wl_listener *listener);
-
-struct wl_listener *
-wl_priv_signal_get(struct wl_priv_signal *signal, wl_notify_func_t notify);
-
-void
-wl_priv_signal_emit(struct wl_priv_signal *signal, void *data);
-
 void
 wl_connection_close_fds_in(struct wl_connection *connection, int max);
 
diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h
index 2e725d9..965bb39 100644
--- a/src/wayland-server-core.h
+++ b/src/wayland-server-core.h
@@ -462,10 +462,34 @@ wl_signal_get(struct wl_signal *signal, wl_notify_func_t notify)
 static inline void
 wl_signal_emit(struct wl_signal *signal, void *data)
 {
-	struct wl_listener *l, *next;
+	struct wl_listener *l;
+	struct wl_list *pos;
+	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 = NULL;
+	wl_list_insert(signal->listener_list.prev, &end.link);
+	end.notify = NULL;
+
+	while (cursor.link.next != &end.link) {
+		pos = cursor.link.next;
+		l = wl_container_of(pos, l, link);
+
+		wl_list_remove(&cursor.link);
+		wl_list_insert(pos, &cursor.link);
 
-	wl_list_for_each_safe(l, next, &signal->listener_list, link)
 		l->notify(l, data);
+	}
+
+	wl_list_remove(&cursor.link);
+	wl_list_remove(&end.link);
 }
 
 typedef void (*wl_resource_destroy_func_t)(struct wl_resource *resource);
diff --git a/src/wayland-server.c b/src/wayland-server.c
index eb1e500..c41f8e9 100644
--- a/src/wayland-server.c
+++ b/src/wayland-server.c
@@ -76,10 +76,10 @@ struct wl_client {
 	struct wl_resource *display_resource;
 	struct wl_list link;
 	struct wl_map objects;
-	struct wl_priv_signal destroy_signal;
+	struct wl_signal destroy_signal;
 	struct ucred ucred;
 	int error;
-	struct wl_priv_signal resource_created_signal;
+	struct wl_signal resource_created_signal;
 };
 
 struct wl_display {
@@ -95,8 +95,8 @@ struct wl_display {
 	struct wl_list client_list;
 	struct wl_list protocol_loggers;
 
-	struct wl_priv_signal destroy_signal;
-	struct wl_priv_signal create_client_signal;
+	struct wl_signal destroy_signal;
+	struct wl_signal create_client_signal;
 
 	struct wl_array additional_shm_formats;
 
@@ -127,7 +127,7 @@ struct wl_resource {
 	void *data;
 	int version;
 	wl_dispatcher_func_t dispatcher;
-	struct wl_priv_signal destroy_signal;
+	struct wl_signal destroy_signal;
 };
 
 struct wl_protocol_logger {
@@ -508,7 +508,7 @@ wl_client_create(struct wl_display *display, int fd)
 	if (client == NULL)
 		return NULL;
 
-	wl_priv_signal_init(&client->resource_created_signal);
+	wl_signal_init(&client->resource_created_signal);
 	client->display = display;
 	client->source = wl_event_loop_add_fd(display->loop, fd,
 					      WL_EVENT_READABLE,
@@ -531,13 +531,13 @@ wl_client_create(struct wl_display *display, int fd)
 	if (wl_map_insert_at(&client->objects, 0, 0, NULL) < 0)
 		goto err_map;
 
-	wl_priv_signal_init(&client->destroy_signal);
+	wl_signal_init(&client->destroy_signal);
 	if (bind_display(client, display) < 0)
 		goto err_map;
 
 	wl_list_insert(display->client_list.prev, &client->link);
 
-	wl_priv_signal_emit(&display->create_client_signal, client);
+	wl_signal_emit(&display->create_client_signal, client);
 
 	return client;
 
@@ -682,7 +682,7 @@ destroy_resource(void *element, void *data, uint32_t flags)
 	/* Don't emit the new signal for deprecated resources, as that would
 	 * access memory outside the bounds of the deprecated struct */
 	if (!resource_is_deprecated(resource))
-		wl_priv_signal_emit(&resource->destroy_signal, resource);
+		wl_signal_emit(&resource->destroy_signal, resource);
 
 	if (resource->destroy)
 		resource->destroy(resource);
@@ -798,7 +798,7 @@ wl_resource_add_destroy_listener(struct wl_resource *resource,
 	if (resource_is_deprecated(resource))
 		wl_signal_add(&resource->deprecated_destroy_signal, listener);
 	else
-		wl_priv_signal_add(&resource->destroy_signal, listener);
+		wl_signal_add(&resource->destroy_signal, listener);
 }
 
 WL_EXPORT struct wl_listener *
@@ -807,7 +807,7 @@ wl_resource_get_destroy_listener(struct wl_resource *resource,
 {
 	if (resource_is_deprecated(resource))
 		return wl_signal_get(&resource->deprecated_destroy_signal, notify);
-	return wl_priv_signal_get(&resource->destroy_signal, notify);
+	return wl_signal_get(&resource->destroy_signal, notify);
 }
 
 /** Retrieve the interface name (class) of a resource object.
@@ -826,14 +826,14 @@ WL_EXPORT void
 wl_client_add_destroy_listener(struct wl_client *client,
 			       struct wl_listener *listener)
 {
-	wl_priv_signal_add(&client->destroy_signal, listener);
+	wl_signal_add(&client->destroy_signal, listener);
 }
 
 WL_EXPORT struct wl_listener *
 wl_client_get_destroy_listener(struct wl_client *client,
 			       wl_notify_func_t notify)
 {
-	return wl_priv_signal_get(&client->destroy_signal, notify);
+	return wl_signal_get(&client->destroy_signal, notify);
 }
 
 WL_EXPORT void
@@ -841,7 +841,7 @@ wl_client_destroy(struct wl_client *client)
 {
 	uint32_t serial = 0;
 
-	wl_priv_signal_emit(&client->destroy_signal, client);
+	wl_signal_emit(&client->destroy_signal, client);
 
 	wl_client_flush(client);
 	wl_map_for_each(&client->objects, destroy_resource, &serial);
@@ -1023,8 +1023,8 @@ wl_display_create(void)
 	wl_list_init(&display->registry_resource_list);
 	wl_list_init(&display->protocol_loggers);
 
-	wl_priv_signal_init(&display->destroy_signal);
-	wl_priv_signal_init(&display->create_client_signal);
+	wl_signal_init(&display->destroy_signal);
+	wl_signal_init(&display->create_client_signal);
 
 	display->id = 1;
 	display->serial = 0;
@@ -1089,7 +1089,7 @@ wl_display_destroy(struct wl_display *display)
 	struct wl_socket *s, *next;
 	struct wl_global *global, *gnext;
 
-	wl_priv_signal_emit(&display->destroy_signal, display);
+	wl_signal_emit(&display->destroy_signal, display);
 
 	wl_list_for_each_safe(s, next, &display->socket_list, link) {
 		wl_socket_destroy(s);
@@ -1597,7 +1597,7 @@ WL_EXPORT void
 wl_display_add_destroy_listener(struct wl_display *display,
 				struct wl_listener *listener)
 {
-	wl_priv_signal_add(&display->destroy_signal, listener);
+	wl_signal_add(&display->destroy_signal, listener);
 }
 
 /** Registers a listener for the client connection signal.
@@ -1615,14 +1615,14 @@ WL_EXPORT void
 wl_display_add_client_created_listener(struct wl_display *display,
 					struct wl_listener *listener)
 {
-	wl_priv_signal_add(&display->create_client_signal, listener);
+	wl_signal_add(&display->create_client_signal, listener);
 }
 
 WL_EXPORT struct wl_listener *
 wl_display_get_destroy_listener(struct wl_display *display,
 				wl_notify_func_t notify)
 {
-	return wl_priv_signal_get(&display->destroy_signal, notify);
+	return wl_signal_get(&display->destroy_signal, notify);
 }
 
 WL_EXPORT void
@@ -1679,7 +1679,7 @@ wl_resource_create(struct wl_client *client,
 	resource->object.implementation = NULL;
 
 	wl_signal_init(&resource->deprecated_destroy_signal);
-	wl_priv_signal_init(&resource->destroy_signal);
+	wl_signal_init(&resource->destroy_signal);
 
 	resource->destroy = NULL;
 	resource->client = client;
@@ -1695,7 +1695,7 @@ wl_resource_create(struct wl_client *client,
 		return NULL;
 	}
 
-	wl_priv_signal_emit(&client->resource_created_signal, resource);
+	wl_signal_emit(&client->resource_created_signal, resource);
 	return resource;
 }
 
@@ -1883,7 +1883,7 @@ WL_EXPORT void
 wl_client_add_resource_created_listener(struct wl_client *client,
 					struct wl_listener *listener)
 {
-	wl_priv_signal_add(&client->resource_created_signal, listener);
+	wl_signal_add(&client->resource_created_signal, listener);
 }
 
 struct wl_resource_iterator_context {
@@ -1932,101 +1932,6 @@ wl_client_for_each_resource(struct wl_client *client,
 	wl_map_for_each(&client->objects, resource_iterator_helper, &context);
 }
 
-/** \cond INTERNAL */
-
-/** Initialize a wl_priv_signal object
- *
- * wl_priv_signal is a safer implementation of a signal type, with the same API
- * as wl_signal, but kept as a private utility of libwayland-server.
- * It is safer because listeners can be removed from within wl_priv_signal_emit()
- * without corrupting the signal's list.
- *
- * Before passing a wl_priv_signal object to any other function it must be
- * initialized by useing wl_priv_signal_init().
- *
- * \memberof wl_priv_signal
- */
-void
-wl_priv_signal_init(struct wl_priv_signal *signal)
-{
-	wl_list_init(&signal->listener_list);
-	wl_list_init(&signal->emit_list);
-}
-
-/** Add a listener to a signal
- *
- * The new listener will be called when calling wl_signal_emit(). If a listener is
- * added to the signal while wl_signal_emit() is running it will be called from
- * the next time wl_priv_signal_emit() is called.
- * To remove a listener call wl_list_remove() on its link member.
- *
- * \memberof wl_priv_signal
- */
-void
-wl_priv_signal_add(struct wl_priv_signal *signal, struct wl_listener *listener)
-{
-	wl_list_insert(signal->listener_list.prev, &listener->link);
-}
-
-/** Get a listener added to a signal
- *
- * Returns the listener added to the given \a signal and with the given
- * \a notify function, or NULL if there isn't any.
- * Calling this function from withing wl_priv_signal_emit() is safe and will
- * return the correct value.
- *
- * \memberof wl_priv_signal
- */
-struct wl_listener *
-wl_priv_signal_get(struct wl_priv_signal *signal, wl_notify_func_t notify)
-{
-	struct wl_listener *l;
-
-	wl_list_for_each(l, &signal->listener_list, link)
-		if (l->notify == notify)
-			return l;
-	wl_list_for_each(l, &signal->emit_list, link)
-		if (l->notify == notify)
-			return l;
-
-	return NULL;
-}
-
-/** Emit the signal, calling all the installed listeners
- *
- * Iterate over all the listeners added to this \a signal and call
- * their \a notify function pointer, passing on the given \a data.
- * Removing or adding a listener from within wl_priv_signal_emit()
- * is safe.
- */
-void
-wl_priv_signal_emit(struct wl_priv_signal *signal, void *data)
-{
-	struct wl_listener *l;
-	struct wl_list *pos;
-
-	wl_list_insert_list(&signal->emit_list, &signal->listener_list);
-	wl_list_init(&signal->listener_list);
-
-	/* 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(&signal->emit_list)) {
-		pos = signal->emit_list.next;
-		l = wl_container_of(pos, l, link);
-
-		wl_list_remove(pos);
-		wl_list_insert(&signal->listener_list, pos);
-
-		l->notify(l, data);
-	}
-}
-
-/** \endcond INTERNAL */
-
 /** \cond */ /* Deprecated functions below. */
 
 uint32_t
diff --git a/tests/newsignal-test.c b/tests/newsignal-test.c
deleted file mode 100644
index 47c429b..0000000
--- a/tests/newsignal-test.c
+++ /dev/null
@@ -1,337 +0,0 @@
-/*
- * Copyright © 2013 Marek Chalupa
- *
- * Permission is hereby granted, free of charge, to any person obtaining
- * a copy of this software and associated documentation files (the
- * "Software"), to deal in the Software without restriction, including
- * without limitation the rights to use, copy, modify, merge, publish,
- * distribute, sublicense, and/or sell copies of the Software, and to
- * permit persons to whom the Software is furnished to do so, subject to
- * the following conditions:
- *
- * The above copyright notice and this permission notice (including the
- * next paragraph) shall be included in all copies or substantial
- * portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
- * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
- * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
- * NONINFRINGEMENT.  IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
- * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
- * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
- * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
- * SOFTWARE.
- */
-
-#include <assert.h>
-
-#include "test-runner.h"
-#include "wayland-private.h"
-
-static void
-signal_notify(struct wl_listener *listener, void *data)
-{
-	/* only increase counter*/
-	++(*((int *) data));
-}
-
-TEST(signal_init)
-{
-	struct wl_priv_signal signal;
-
-	wl_priv_signal_init(&signal);
-
-	/* Test if listeners' list is initialized */
-	assert(&signal.listener_list == signal.listener_list.next
-		&& "Maybe wl_priv_signal implementation changed?");
-	assert(signal.listener_list.next == signal.listener_list.prev
-		&& "Maybe wl_priv_signal implementation changed?");
-}
-
-TEST(signal_add_get)
-{
-	struct wl_priv_signal signal;
-
-	/* we just need different values of notify */
-	struct wl_listener l1 = {.notify = (wl_notify_func_t) 0x1};
-	struct wl_listener l2 = {.notify = (wl_notify_func_t) 0x2};
-	struct wl_listener l3 = {.notify = (wl_notify_func_t) 0x3};
-	/* one real, why not */
-	struct wl_listener l4 = {.notify = signal_notify};
-
-	wl_priv_signal_init(&signal);
-
-	wl_priv_signal_add(&signal, &l1);
-	wl_priv_signal_add(&signal, &l2);
-	wl_priv_signal_add(&signal, &l3);
-	wl_priv_signal_add(&signal, &l4);
-
-	assert(wl_priv_signal_get(&signal, signal_notify) == &l4);
-	assert(wl_priv_signal_get(&signal, (wl_notify_func_t) 0x3) == &l3);
-	assert(wl_priv_signal_get(&signal, (wl_notify_func_t) 0x2) == &l2);
-	assert(wl_priv_signal_get(&signal, (wl_notify_func_t) 0x1) == &l1);
-
-	/* get should not be destructive */
-	assert(wl_priv_signal_get(&signal, signal_notify) == &l4);
-	assert(wl_priv_signal_get(&signal, (wl_notify_func_t) 0x3) == &l3);
-	assert(wl_priv_signal_get(&signal, (wl_notify_func_t) 0x2) == &l2);
-	assert(wl_priv_signal_get(&signal, (wl_notify_func_t) 0x1) == &l1);
-}
-
-TEST(signal_emit_to_one_listener)
-{
-	int count = 0;
-	int counter;
-
-	struct wl_priv_signal signal;
-	struct wl_listener l1 = {.notify = signal_notify};
-
-	wl_priv_signal_init(&signal);
-	wl_priv_signal_add(&signal, &l1);
-
-	for (counter = 0; counter < 100; counter++)
-		wl_priv_signal_emit(&signal, &count);
-
-	assert(counter == count);
-}
-
-TEST(signal_emit_to_more_listeners)
-{
-	int count = 0;
-	int counter;
-
-	struct wl_priv_signal signal;
-	struct wl_listener l1 = {.notify = signal_notify};
-	struct wl_listener l2 = {.notify = signal_notify};
-	struct wl_listener l3 = {.notify = signal_notify};
-
-	wl_priv_signal_init(&signal);
-	wl_priv_signal_add(&signal, &l1);
-	wl_priv_signal_add(&signal, &l2);
-	wl_priv_signal_add(&signal, &l3);
-
-	for (counter = 0; counter < 100; counter++)
-		wl_priv_signal_emit(&signal, &count);
-
-	assert(3 * counter == count);
-}
-
-struct signal
-{
-	struct wl_priv_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_priv_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_priv_signal_emit(&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_priv_signal_add(&signal.signal, &signal.l1);
-
-	signal.current = &signal.l1;
-	CHECK_EMIT(1)
-	CHECK_EMIT(0)
-
-	INIT
-	wl_priv_signal_add(&signal.signal, &signal.l1);
-	wl_priv_signal_add(&signal.signal, &signal.l2);
-
-	CHECK_EMIT(2)
-	CHECK_EMIT(1)
-
-	INIT
-	wl_priv_signal_add(&signal.signal, &signal.l1);
-	wl_priv_signal_add(&signal.signal, &signal.l2);
-
-	signal.current = &signal.l2;
-	CHECK_EMIT(1)
-	CHECK_EMIT(1)
-
-	INIT
-	wl_priv_signal_add(&signal.signal, &signal.l1);
-	wl_priv_signal_add(&signal.signal, &signal.l2);
-	wl_priv_signal_add(&signal.signal, &signal.l3);
-
-	signal.current = &signal.l1;
-	CHECK_EMIT(3)
-	CHECK_EMIT(2)
-
-	INIT
-	wl_priv_signal_add(&signal.signal, &signal.l1);
-	wl_priv_signal_add(&signal.signal, &signal.l2);
-	wl_priv_signal_add(&signal.signal, &signal.l3);
-
-	signal.current = &signal.l2;
-	CHECK_EMIT(2)
-	CHECK_EMIT(2)
-
-	INIT
-	wl_priv_signal_add(&signal.signal, &signal.l1);
-	wl_priv_signal_add(&signal.signal, &signal.l2);
-	wl_priv_signal_add(&signal.signal, &signal.l3);
-
-	signal.current = &signal.l3;
-	CHECK_EMIT(2)
-	CHECK_EMIT(2)
-}
-
-static void notify_readd(struct wl_listener *l, void *data)
-{
-	struct signal *signal = data;
-	if (signal->current) {
-		wl_list_remove(&signal->current->link);
-		wl_list_init(&signal->current->link);
-		wl_priv_signal_add(&signal->signal, signal->current);
-	}
-	signal->count++;
-}
-
-static void notify_empty(struct wl_listener *l, void *data)
-{
-	struct signal *signal = data;
-	signal->count++;
-}
-
-TEST(signal_readd_listener)
-{
-	/* Readding a listener is supported, that is it doesn't trigger an
-	 * infinite loop or other weird things, but if in a listener you
-	 * readd another listener, that will not be fired in the current
-	 * signal emission. */
-
-	test_set_timeout(4);
-
-	struct signal signal;
-
-	signal.l1.notify = notify_readd;
-	signal.l2.notify = notify_readd;
-
-	INIT
-	wl_priv_signal_add(&signal.signal, &signal.l1);
-
-	signal.current = &signal.l1;
-	CHECK_EMIT(1)
-	CHECK_EMIT(1)
-
-	INIT
-	wl_priv_signal_add(&signal.signal, &signal.l1);
-
-	signal.current = &signal.l2;
-	CHECK_EMIT(1)
-	signal.current = NULL;
-	CHECK_EMIT(2)
-
-	INIT
-	wl_priv_signal_add(&signal.signal, &signal.l2);
-
-	signal.current = &signal.l1;
-	CHECK_EMIT(1)
-	/* l2 was added before l1, so l2 is fired first, which by readding l1
-	 * removes it from the current list that is being fired, so 1 is correct */
-	CHECK_EMIT(1)
-
-	INIT
-	wl_priv_signal_add(&signal.signal, &signal.l1);
-	wl_priv_signal_add(&signal.signal, &signal.l2);
-
-	signal.l1.notify = notify_empty;
-	signal.current = &signal.l2;
-	CHECK_EMIT(2)
-	CHECK_EMIT(2)
-
-	INIT
-	wl_priv_signal_add(&signal.signal, &signal.l1);
-	wl_priv_signal_add(&signal.signal, &signal.l2);
-
-	signal.l1.notify = notify_empty;
-	signal.current = &signal.l1;
-	CHECK_EMIT(2)
-	/* same as before, by readding l1 in the first emit, it now is fired
-	 * after l2, so on the second emit it is not fired at all. */
-	CHECK_EMIT(1)
-}
-
-static void notify_addandget(struct wl_listener *l, void *data)
-{
-	struct signal *signal = data;
-	wl_list_remove(&signal->current->link);
-	wl_list_init(&signal->current->link);
-	wl_priv_signal_add(&signal->signal, signal->current);
-
-	assert(wl_priv_signal_get(&signal->signal, signal->current->notify) != NULL);
-
-	signal->count++;
-}
-
-static void notify_get(struct wl_listener *l, void *data)
-{
-	struct signal *signal = data;
-	assert(wl_priv_signal_get(&signal->signal, signal->current->notify) == signal->current);
-	signal->count++;
-}
-
-TEST(signal_get_listener)
-{
-	test_set_timeout(4);
-
-	struct signal signal;
-
-	signal.l1.notify = notify_addandget;
-	signal.l2.notify = notify_get;
-
-	INIT
-	wl_priv_signal_add(&signal.signal, &signal.l1);
-
-	signal.current = &signal.l2;
-	CHECK_EMIT(1)
-
-	INIT
-	wl_priv_signal_add(&signal.signal, &signal.l2);
-
-	signal.current = &signal.l2;
-	CHECK_EMIT(1)
-
-	INIT
-	signal.l1.notify = notify_get;
-	signal.l2.notify = notify_empty;
-	wl_priv_signal_add(&signal.signal, &signal.l1);
-	wl_priv_signal_add(&signal.signal, &signal.l2);
-
-	CHECK_EMIT(2)
-
-	INIT
-	signal.l1.notify = notify_empty;
-	signal.l2.notify = notify_get;
-	wl_priv_signal_add(&signal.signal, &signal.l1);
-	wl_priv_signal_add(&signal.signal, &signal.l2);
-
-	signal.current = &signal.l1;
-	CHECK_EMIT(2)
-}
diff --git a/tests/signal-test.c b/tests/signal-test.c
index 7bbaa9f..52ed79b 100644
--- a/tests/signal-test.c
+++ b/tests/signal-test.c
@@ -25,8 +25,8 @@
 
 #include <assert.h>
 
-#include "wayland-server.h"
 #include "test-runner.h"
+#include "wayland-private.h"
 
 static void
 signal_notify(struct wl_listener *listener, void *data)
@@ -115,3 +115,223 @@ 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(&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)
+}
+
+static void notify_readd(struct wl_listener *l, void *data)
+{
+	struct signal *signal = data;
+	if (signal->current) {
+		wl_list_remove(&signal->current->link);
+		wl_list_init(&signal->current->link);
+		wl_signal_add(&signal->signal, signal->current);
+	}
+	signal->count++;
+}
+
+static void notify_empty(struct wl_listener *l, void *data)
+{
+	struct signal *signal = data;
+	signal->count++;
+}
+
+TEST(signal_readd_listener)
+{
+	/* Readding a listener is supported, that is it doesn't trigger an
+	 * infinite loop or other weird things, but if in a listener you
+	 * readd another listener, that will not be fired in the current
+	 * signal emission. */
+
+	test_set_timeout(4);
+
+	struct signal signal;
+
+	signal.l1.notify = notify_readd;
+	signal.l2.notify = notify_readd;
+
+	INIT
+	wl_signal_add(&signal.signal, &signal.l1);
+
+	signal.current = &signal.l1;
+	CHECK_EMIT(1)
+	CHECK_EMIT(1)
+
+	INIT
+	wl_signal_add(&signal.signal, &signal.l1);
+
+	signal.current = &signal.l2;
+	CHECK_EMIT(1)
+	signal.current = NULL;
+	CHECK_EMIT(2)
+
+	INIT
+	wl_signal_add(&signal.signal, &signal.l2);
+
+	signal.current = &signal.l1;
+	CHECK_EMIT(1)
+	/* l2 was added before l1, so l2 is fired first, which by readding l1
+	 * removes it from the current list that is being fired, so 1 is correct */
+	CHECK_EMIT(1)
+
+	INIT
+	wl_signal_add(&signal.signal, &signal.l1);
+	wl_signal_add(&signal.signal, &signal.l2);
+
+	signal.l1.notify = notify_empty;
+	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);
+
+	signal.l1.notify = notify_empty;
+	signal.current = &signal.l1;
+	CHECK_EMIT(2)
+	/* same as before, by readding l1 in the first emit, it now is fired
+	 * after l2, so on the second emit it is not fired at all. */
+	CHECK_EMIT(1)
+}
+
+static void notify_addandget(struct wl_listener *l, void *data)
+{
+	struct signal *signal = data;
+	wl_list_remove(&signal->current->link);
+	wl_list_init(&signal->current->link);
+	wl_signal_add(&signal->signal, signal->current);
+
+	assert(wl_signal_get(&signal->signal, signal->current->notify) != NULL);
+
+	signal->count++;
+}
+
+static void notify_get(struct wl_listener *l, void *data)
+{
+	struct signal *signal = data;
+	assert(wl_signal_get(&signal->signal, signal->current->notify) == signal->current);
+	signal->count++;
+}
+
+TEST(signal_get_listener)
+{
+	test_set_timeout(4);
+
+	struct signal signal;
+
+	signal.l1.notify = notify_addandget;
+	signal.l2.notify = notify_get;
+
+	INIT
+	wl_signal_add(&signal.signal, &signal.l1);
+
+	signal.current = &signal.l2;
+	CHECK_EMIT(1)
+
+	INIT
+	wl_signal_add(&signal.signal, &signal.l2);
+
+	signal.current = &signal.l2;
+	CHECK_EMIT(1)
+
+	INIT
+	signal.l1.notify = notify_get;
+	signal.l2.notify = notify_empty;
+	wl_signal_add(&signal.signal, &signal.l1);
+	wl_signal_add(&signal.signal, &signal.l2);
+
+	CHECK_EMIT(2)
+
+	INIT
+	signal.l1.notify = notify_empty;
+	signal.l2.notify = notify_get;
+	wl_signal_add(&signal.signal, &signal.l1);
+	wl_signal_add(&signal.signal, &signal.l2);
+
+	signal.current = &signal.l1;
+	CHECK_EMIT(2)
+}
-- 
2.16.1




More information about the wayland-devel mailing list