libweston/compositor.c

@@ -68,6 +68,7 @@
 #include "shared/os-compatibility.h"
 #include "shared/string-helpers.h"
 #include "shared/timespec-util.h"
+#include "shared/signal.h"
 #include "git-version.h"
 #include <libweston/version.h>
 #include <libweston/plugin-registry.h>
@@ -2266,7 +2267,7 @@ weston_view_destroy(struct weston_view *view)
 {
 	struct weston_paint_node *pnode, *pntmp;
 
-	wl_signal_emit(&view->destroy_signal, view);
+	weston_signal_emit_mutable(&view->destroy_signal, view);
 
 	assert(wl_list_empty(&view->geometry.child_list));
 
@@ -2308,7 +2309,7 @@ weston_surface_destroy(struct weston_surface *surface)
 
 	assert(surface->resource == NULL);
 
-	wl_signal_emit(&surface->destroy_signal, surface);
+	weston_signal_emit_mutable(&surface->destroy_signal, surface);
 
 	assert(wl_list_empty(&surface->subsurface_list_pending));
 	assert(wl_list_empty(&surface->subsurface_list));
@@ -2377,7 +2378,7 @@ weston_buffer_destroy_handler(struct wl_listener *listener, void *data)
 	struct weston_buffer *buffer =
 		container_of(listener, struct weston_buffer, destroy_listener);
 
-	wl_signal_emit(&buffer->destroy_signal, buffer);
+	weston_signal_emit_mutable(&buffer->destroy_signal, buffer);
 	free(buffer);
 }
 
@@ -5592,7 +5593,7 @@ weston_head_detach(struct weston_head *head)
 WL_EXPORT void
 weston_head_release(struct weston_head *head)
 {
-	wl_signal_emit(&head->destroy_signal, head);
+	weston_signal_emit_mutable(&head->destroy_signal, head);
 
 	weston_head_detach(head);
 
@@ -6413,8 +6414,8 @@ weston_compositor_remove_output(struct weston_output *output)
 	wl_list_insert(compositor->pending_output_list.prev, &output->link);
 	output->enabled = false;
 
-	wl_signal_emit(&compositor->output_destroyed_signal, output);
-	wl_signal_emit(&output->destroy_signal, output);
+	weston_signal_emit_mutable(&compositor->output_destroyed_signal, output);
+	weston_signal_emit_mutable(&output->destroy_signal, output);
 
 	wl_list_for_each(head, &output->head_list, output_link)
 		weston_head_remove_global(head);
@@ -6926,7 +6927,7 @@ weston_output_release(struct weston_output *output)
 
 	output->destroying = 1;
 
-	wl_signal_emit(&output->user_destroy_signal, output);
+	weston_signal_emit_mutable(&output->user_destroy_signal, output);
 
 	if (output->idle_repaint_source)
 		wl_event_source_remove(output->idle_repaint_source);
@@ -8207,7 +8208,7 @@ weston_compositor_destroy(struct weston_compositor *compositor)
 	/* prevent further rendering while shutting down */
 	compositor->state = WESTON_COMPOSITOR_OFFSCREEN;
 
-	wl_signal_emit(&compositor->destroy_signal, compositor);
+	weston_signal_emit_mutable(&compositor->destroy_signal, compositor);
 
 	weston_compositor_xkb_destroy(compositor);
 

shared/meson.build

 srcs_libshared = [
 	'config-parser.c',
 	'option-parser.c',
+	'signal.c',
 	'file-util.c',
 	'os-compatibility.c',
 	'xalloc.c',
 ]
-deps_libshared = dep_wayland_client
+deps_libshared = [dep_wayland_client, dep_wayland_server]
 
 lib_libshared = static_library(
 	'shared',

shared/signal.c

+/*
+ * Copyright 2018 Simon Ser
+ * Copyright 2021 Collabora, Ltd.
+ *
+ * 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 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.
+ */
+
+/* Implementation copied from wlroots util/signal.c file */
+
+#include "signal.h"
+
+static void
+handle_noop(struct wl_listener *listener, void *data)
+{
+	/* Do nothing */
+}
+
+void
+weston_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);
+}

shared/signal.h

+/*
+ * Copyright 2018 Simon Ser
+ * Copyright 2021 Collabora, Ltd.
+ *
+ * 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 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.
+ */
+
+#ifndef WESTON_SIGNAL_H
+#define WESTON_SIGNAL_H
+
+#include <wayland-server-core.h>
+
+#ifdef  __cplusplus
+extern "C" {
+#endif
+
+/* 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.
+ */
+
+void
+weston_signal_emit_mutable(struct wl_signal *signal, void *data);
+
+#ifdef  __cplusplus
+}
+#endif
+
+#endif /* WESTON_SIGNAL_H */

tests/meson.build

@@ -216,6 +216,14 @@ tests = [
 			dep_libm,
 		],
 	},
+	{	'name': 'safe-signal', },
+	{	'name': 'safe-signal-output-removal',
+		'sources': [
+		  'safe-signal-output-removal-test.c',
+		  '../shared/shell-utils.c',
+		],
+		'dep_objs': [ dep_lib_desktop ]
+	},
 ]
 
 tests_standalone = [

tests/safe-signal-output-removal-test.c

+/*
+ * Copyright 2021 Collabora, Ltd.
+ *
+ * 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 "config.h"
+
+#include <unistd.h>
+#include <assert.h>
+#include <stdlib.h>
+#include <stdio.h>
+
+#include "../shared/signal.h"
+#include "../shared/shell-utils.h"
+#include "weston-test-client-helper.h"
+#include "weston-test-fixture-compositor.h"
+
+struct test_output {
+       struct weston_compositor *compositor;
+       struct weston_output *output;
+       struct wl_listener output_destroy_listener;
+       struct weston_view *view;
+};
+
+static enum test_result_code
+fixture_setup(struct weston_test_harness *harness)
+{
+       struct compositor_setup setup;
+
+       compositor_setup_defaults(&setup);
+       setup.shell = SHELL_TEST_DESKTOP;
+
+       return weston_test_harness_execute_as_plugin(harness, &setup);
+}
+
+DECLARE_FIXTURE_SETUP(fixture_setup);
+
+static void
+output_destroy(struct test_output *t_output)
+{
+       t_output->output = NULL;
+       t_output->output_destroy_listener.notify = NULL;
+
+       if (t_output->view)
+               weston_surface_destroy(t_output->view->surface);
+
+       wl_list_remove(&t_output->output_destroy_listener.link);
+       free(t_output);
+}
+
+static void
+notify_output_destroy(struct wl_listener *listener, void *data)
+{
+       struct test_output *t_output =
+               container_of(listener, struct test_output, output_destroy_listener);
+
+       output_destroy(t_output);
+}
+
+static void
+output_create_view(struct test_output *t_output)
+{
+       struct weston_solid_color_surface solid_surface = {};
+
+       solid_surface.r = 0.5;
+       solid_surface.g = 0.5;
+       solid_surface.b = 0.5;
+
+       solid_surface.get_label = NULL;
+       solid_surface.surface_committed = NULL;
+       solid_surface.surface_private = NULL;
+
+       t_output->view =
+               create_solid_color_surface(t_output->compositor,
+                                          &solid_surface, 0, 0, 320, 240);
+       weston_view_set_output(t_output->view, t_output->output);
+
+       /* weston_compositor_remove_output() has to be patched with
+	* weston_signal_emit_mutable() to avoid signal corruption */
+       weston_output_destroy(t_output->output);
+}
+
+static void
+output_create(struct weston_output *output)
+{
+       struct test_output *t_output;
+
+       t_output = zalloc(sizeof *t_output);
+       if (t_output == NULL)
+               return;
+
+       t_output->output = output;
+       t_output->compositor = output->compositor;
+
+       t_output->output_destroy_listener.notify = notify_output_destroy;
+       wl_signal_add(&t_output->output->destroy_signal,
+                     &t_output->output_destroy_listener);
+
+       output_create_view(t_output);
+}
+
+static void
+create_outputs(struct weston_compositor *compositor)
+{
+       struct weston_output *output, *output_next;
+
+       wl_list_for_each_safe(output, output_next, &compositor->output_list, link)
+               output_create(output);
+}
+
+PLUGIN_TEST(real_usecase_one)
+{
+       create_outputs(compositor);
+}

tests/safe-signal-test.c

+/*
+ * Copyright 2021 Collabora, Ltd.
+ *
+ * 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 "config.h"
+
+#include <unistd.h>
+#include <assert.h>
+#include <stdlib.h>
+#include <stdio.h>
+
+#include "../shared/signal.h"
+#include "weston-test-client-helper.h"
+
+struct test_surface_state {
+	struct wl_signal signal_destroy;
+	struct wl_listener surface_destroy_listener;
+};
+
+static enum test_result_code
+fixture_setup(struct weston_test_harness *harness)
+{
+	return weston_test_harness_execute_standalone(harness);
+}
+
+DECLARE_FIXTURE_SETUP(fixture_setup);
+
+static void
+destroy_test_surface(struct test_surface_state *st)
+{
+	weston_signal_emit_mutable(&st->signal_destroy, st);
+}
+
+static void
+notify_test_destroy(struct wl_listener *listener, void *data)
+{
+	struct test_surface_state *st = data;
+
+	wl_list_remove(&st->surface_destroy_listener.link);
+	free(st);
+}
+
+static struct test_surface_state *
+create_surface(void)
+{
+	struct test_surface_state *st = zalloc(sizeof(*st));
+
+	wl_signal_init(&st->signal_destroy);
+	return st;
+}
+
+static void
+add_destroy_listener(struct test_surface_state *st)
+{
+	st->surface_destroy_listener.notify = notify_test_destroy;
+	wl_signal_add(&st->signal_destroy,
+		      &st->surface_destroy_listener);
+}
+
+TEST(real_usecase_standalone)
+{
+	struct test_surface_state *st, *st_new;
+
+	st = create_surface();
+	add_destroy_listener(st);
+
+	st_new = create_surface();
+	add_destroy_listener(st_new);
+
+	destroy_test_surface(st);
+}