[RFC] server: Add special case destroy signal emitter

Submitted by wl@ongy.net on Feb. 23, 2018, 9:44 a.m.

Details

Message ID 686d86f4cc759913d76ca051efc2af4f@localhost
State New
Headers show
Series "server: Add special case destroy signal emitter" ( rev: 2 ) in Wayland

Not browsing as part of any series.

Commit Message

wl@ongy.net Feb. 23, 2018, 9:44 a.m.
Hi, I have a v2 RFC _emit_final based on your idea.
It passes `make check` of libwayland and weston.
It also passes the remove-without-free test I sent in another mail.

---

(This patch isn't quite intended to be merged, but to get feedback on
the approach)

This adds a wl_priv_signal_emit_final and a wl_signal_emit_final.
The `_final` emit functions have the additional asumption that every
listener currently in the list will be removed after the _emit. Either
gracefully, or simply free()d under the asumption that a destroy signal
will not be emitted more than once.

These functions take advantage of this and allow to use both tyles in
the same signal list.
It obsoletes the wl_priv_signal for destroy signals.

Non destroy wl_priv_signals currently have a stronger guarantee than
normal signals, which forces us to either keep the type around, or use
this as destroy signal path, and another safe version for other signals.
---
 src/wayland-private.h |  5 +++++
 src/wayland-server.c  | 50 +++++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 52 insertions(+), 3 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/wayland-private.h b/src/wayland-private.h
index 12b9032..76adb32 100644
--- a/src/wayland-private.h
+++ b/src/wayland-private.h
@@ -250,6 +250,11 @@  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_signal_emit_final(struct wl_list *signal, void *data);
+
+void
+wl_priv_signal_emit_final(struct wl_priv_signal *signal, void *data);
 void
 wl_priv_signal_emit(struct wl_priv_signal *signal, void *data);
 
diff --git a/src/wayland-server.c b/src/wayland-server.c
index eb1e500..62fc71a 100644
--- a/src/wayland-server.c
+++ b/src/wayland-server.c
@@ -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_priv_signal_emit_final(&resource->destroy_signal, resource);
 
 	if (resource->destroy)
 		resource->destroy(resource);
@@ -841,7 +841,7 @@  wl_client_destroy(struct wl_client *client)
 {
 	uint32_t serial = 0;
 
-	wl_priv_signal_emit(&client->destroy_signal, client);
+	wl_priv_signal_emit_final(&client->destroy_signal, client);
 
 	wl_client_flush(client);
 	wl_map_for_each(&client->objects, destroy_resource, &serial);
@@ -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_priv_signal_emit_final(&display->destroy_signal, display);
 
 	wl_list_for_each_safe(s, next, &display->socket_list, link) {
 		wl_socket_destroy(s);
@@ -1992,6 +1992,50 @@  wl_priv_signal_get(struct wl_priv_signal *signal, wl_notify_func_t notify)
 	return NULL;
 }
 
+/**
+ * This emits a signal under the asumpton that every listener in it will be
+ * removed from the list in some way.
+ * 
+ * It supports listeners that remove themselves without issues.
+ *
+ * The situation where a listener is only free()d, but not removed from the
+ * list is also supported, to not break existing library users.
+ *
+ * The notify function can remove arbitrary elements from the wl_signal, and
+ * the wl_signal will be in a valid state at all times (s.t. later elements in
+ * the list not beeing free()d without removal, but that broke at any time).
+ *
+ * The idea is based on wl_priv_signal_emit, we iterate until a list is empty.
+ * In this case we don't need the emit_list, since we know our actual signal
+ * list will be empty in the end.
+ */
+void
+wl_signal_emit_final(struct wl_list *signal, void *data)
+{
+	struct wl_listener *l;
+	struct wl_list *pos;
+
+	while (!wl_list_empty(signal)) {
+		pos = signal->next;
+		
+		/* Remove and ensure validity of position element */
+		wl_list_remove(pos);
+		wl_list_init(pos);
+
+		l = wl_container_of(pos, l, link);
+		l->notify(l, data);
+	}
+}
+
+/**
+ * see wl_signal_emit_final for more details
+ */
+void
+wl_priv_signal_emit_final(struct wl_priv_signal *signal, void *data)
+{
+	wl_signal_emit_final(&signal->listener_list, data);
+}
+
 /** Emit the signal, calling all the installed listeners
  *
  * Iterate over all the listeners added to this \a signal and call