Use weston_compositor_add_destroy_listener_once() in plugins

This introduces a new convention of checking through the compositor destroy
listener if the plugin is already initialized. If the plugin is already
initialized, then the plugin entry function succeeds as a no-op. This makes it
safe to load the same plugin multiple times in a running compositor.

Currently module loading functions return failure if a plugin is already
loaded, but that will change in the future. Therefore we need this other method
of ensuring we do not double-initialize a plugin which would lead to list
corruptions the very least.

All plugins are converted to use the new helper, except:
- those that do not have a destroy listener already, and
- hmi-controller which does the same open-coded as the common code pattern
  did not fit there.

Plugins should always have a compositor destroy listener registered since they
very least allocate a struct to hold their data. Hence omissions are
highlighted in code.

Backends do not need this because weston_compositor_load_backend() already
protects against double-init. GL-renderer does not export a standard module
init function so cannot be initialized the usual way and therefore is not
vulnerable to double-init.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>

15 files changed, 125 insertions, 30 deletions

diff --git a/compositor/cms-colord.c b/compositor/cms-colord.c
index 57a77f44..d4efdb43 100644
--- a/compositor/cms-colord.c
+++ b/compositor/cms-colord.c
@@ -528,6 +528,14 @@ wet_module_init(struct weston_compositor *ec,
 	if (cms == NULL)
 		return -1;
 	cms->ec = ec;
+
+	if (!weston_compositor_add_destroy_listener_once(ec,
+							 &cms->destroy_listener,
+							 colord_notifier_destroy)) {
+		free(cms);
+		return 0;
+	}
+
 #if !GLIB_CHECK_VERSION(2,36,0)
 	g_type_init();
 #endif
@@ -543,10 +551,6 @@ wet_module_init(struct weston_compositor *ec,
 	cms->devices = g_hash_table_new_full(g_str_hash, g_str_equal,
 					     g_free, colord_cms_output_destroy);
 
-	/* destroy */
-	cms->destroy_listener.notify = colord_notifier_destroy;
-	wl_signal_add(&ec->destroy_signal, &cms->destroy_listener);
-
 	/* devices added */
 	cms->output_created_listener.notify = colord_notifier_output_created;
 	wl_signal_add(&ec->output_created_signal, &cms->output_created_listener);
diff --git a/compositor/cms-static.c b/compositor/cms-static.c
index 2f357c41..540d6ad3 100644
--- a/compositor/cms-static.c
+++ b/compositor/cms-static.c
@@ -105,8 +105,13 @@ wet_module_init(struct weston_compositor *ec,
 		return -1;
 
 	cms->ec = ec;
-	cms->destroy_listener.notify = cms_notifier_destroy;
-	wl_signal_add(&ec->destroy_signal, &cms->destroy_listener);
+
+	if (!weston_compositor_add_destroy_listener_once(ec,
+							 &cms->destroy_listener,
+							 cms_notifier_destroy)) {
+		free(cms);
+		return 0;
+	}
 
 	cms->output_created_listener.notify = cms_notifier_output_created;
 	wl_signal_add(&ec->output_created_signal, &cms->output_created_listener);
diff --git a/compositor/screen-share.c b/compositor/screen-share.c
index a5d6fbcf..d6fc1228 100644
--- a/compositor/screen-share.c
+++ b/compositor/screen-share.c
@@ -114,6 +114,9 @@ struct ss_shm_buffer {
 
 struct screen_share {
 	struct weston_compositor *compositor;
+	/* XXX: missing compositor destroy listener
+	 * https://gitlab.freedesktop.org/wayland/weston/issues/298
+	 */
 	char *command;
 };
 
diff --git a/compositor/systemd-notify.c b/compositor/systemd-notify.c
index 6284a6d9..61196d81 100644
--- a/compositor/systemd-notify.c
+++ b/compositor/systemd-notify.c
@@ -127,10 +127,12 @@ wet_module_init(struct weston_compositor *compositor,
 	if (notifier == NULL)
 		return -1;
 
-	notifier->compositor_destroy_listener.notify =
-		weston_compositor_destroy_listener;
-	wl_signal_add(&compositor->destroy_signal,
-		      &notifier->compositor_destroy_listener);
+	if (!weston_compositor_add_destroy_listener_once(compositor,
+							 &notifier->compositor_destroy_listener,
+							 weston_compositor_destroy_listener)) {
+		free(notifier);
+		return 0;
+	}
 
 	if (add_systemd_sockets(compositor) < 0)
 		return -1;
diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index 00ee1079..337e168c 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -5090,8 +5090,13 @@ wet_shell_init(struct weston_compositor *ec,
 
 	shell->compositor = ec;
 
-	shell->destroy_listener.notify = shell_destroy;
-	wl_signal_add(&ec->destroy_signal, &shell->destroy_listener);
+	if (!weston_compositor_add_destroy_listener_once(ec,
+							 &shell->destroy_listener,
+							 shell_destroy)) {
+		free(shell);
+		return 0;
+	}
+
 	shell->idle_listener.notify = idle_handler;
 	wl_signal_add(&ec->idle_signal, &shell->idle_listener);
 	shell->wake_listener.notify = wake_handler;
diff --git a/fullscreen-shell/fullscreen-shell.c b/fullscreen-shell/fullscreen-shell.c
index 6c04cf3d..e785246a 100644
--- a/fullscreen-shell/fullscreen-shell.c
+++ b/fullscreen-shell/fullscreen-shell.c
@@ -42,6 +42,9 @@ struct fullscreen_shell {
 	struct wl_client *client;
 	struct wl_listener client_destroyed;
 	struct weston_compositor *compositor;
+	/* XXX: missing compositor destroy listener
+	 * https://gitlab.freedesktop.org/wayland/weston/issues/299
+	 */
 
 	struct weston_layer layer;
 	struct wl_list output_list;
diff --git a/include/libweston/libweston.h b/include/libweston/libweston.h
index e914d6a3..6b5cc4bf 100644
--- a/include/libweston/libweston.h
+++ b/include/libweston/libweston.h
@@ -1773,6 +1773,11 @@ struct weston_compositor *
 weston_compositor_create(struct wl_display *display,
 			 struct weston_log_context *log_ctx, void *user_data);
 
+bool
+weston_compositor_add_destroy_listener_once(struct weston_compositor *compositor,
+					    struct wl_listener *listener,
+					    wl_notify_func_t destroy_handler);
+
 enum weston_compositor_backend {
 	WESTON_BACKEND_DRM,
 	WESTON_BACKEND_FBDEV,
diff --git a/ivi-shell/hmi-controller.c b/ivi-shell/hmi-controller.c
index 4f90e0ac..230e788b 100644
--- a/ivi-shell/hmi-controller.c
+++ b/ivi-shell/hmi-controller.c
@@ -1981,6 +1981,10 @@ wet_module_init(struct weston_compositor *ec,
 	struct hmi_controller *hmi_ctrl = NULL;
 	struct wl_event_loop *loop = NULL;
 
+	/* ad hoc weston_compositor_add_destroy_listener_once() */
+	if (wl_signal_get(&ec->destroy_signal, hmi_controller_destroy))
+		return 0;
+
 	hmi_ctrl = hmi_controller_create(ec);
 	if (hmi_ctrl == NULL)
 		return -1;
diff --git a/ivi-shell/ivi-shell.c b/ivi-shell/ivi-shell.c
index 99431d0b..d3b5cde2 100644
--- a/ivi-shell/ivi-shell.c
+++ b/ivi-shell/ivi-shell.c
@@ -623,10 +623,14 @@ wet_shell_init(struct weston_compositor *compositor,
 	if (shell == NULL)
 		return -1;
 
-	init_ivi_shell(compositor, shell);
+	if (!weston_compositor_add_destroy_listener_once(compositor,
+							 &shell->destroy_listener,
+							 shell_destroy)) {
+		free(shell);
+		return 0;
+	}
 
-	shell->destroy_listener.notify = shell_destroy;
-	wl_signal_add(&compositor->destroy_signal, &shell->destroy_listener);
+	init_ivi_shell(compositor, shell);
 
 	shell->wake_listener.notify = wake_handler;
 	wl_signal_add(&compositor->wake_signal, &shell->wake_listener);
diff --git a/libweston/compositor.c b/libweston/compositor.c
index 3531a213..af0da062 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -7639,6 +7639,41 @@ weston_load_module(const char *name, const char *entrypoint)
 	return init;
 }
 
+/** Add a compositor destroy listener only once
+ *
+ * \param compositor The compositor whose destroy to watch for.
+ * \param listener The listener struct to initialize.
+ * \param destroy_handler The callback when compositor is destroyed.
+ * \return True if listener is added, or false if there already is a listener
+ * with the given \c destroy_handler.
+ *
+ * This function does nothing and returns false if the given callback function
+ * is already present in the weston_compositor destroy callbacks list.
+ * Otherwise, this function initializes the given listener with the given
+ * callback pointer and adds it to the compositor's destroy callbacks list.
+ *
+ * This can be used to ensure that plugin initialization is done only once
+ * in case the same plugin is loaded multiple times. If this function returns
+ * false, the plugin should be already initialized successfully.
+ *
+ * All plugins should register a destroy listener for cleaning up. Note, that
+ * the plugin destruction order is not guaranteed: plugins that depend on other
+ * plugins must be able to be torn down in arbitrary order.
+ *
+ * \sa weston_compositor_tear_down, weston_compositor_destroy
+ */
+WL_EXPORT bool
+weston_compositor_add_destroy_listener_once(struct weston_compositor *compositor,
+					    struct wl_listener *listener,
+					    wl_notify_func_t destroy_handler)
+{
+	if (wl_signal_get(&compositor->destroy_signal, destroy_handler))
+		return false;
+
+	listener->notify = destroy_handler;
+	wl_signal_add(&compositor->destroy_signal, listener);
+	return true;
+}
 
 /** Tear down the compositor.
  *
diff --git a/pipewire/pipewire-plugin.c b/pipewire/pipewire-plugin.c
index 1eb5de57..552f5745 100644
--- a/pipewire/pipewire-plugin.c
+++ b/pipewire/pipewire-plugin.c
@@ -797,6 +797,13 @@ weston_module_init(struct weston_compositor *compositor)
 	if (!pipewire)
 		return -1;
 
+	if (!weston_compositor_add_destroy_listener_once(compositor,
+							 &pipewire->destroy_listener,
+							 weston_pipewire_destroy)) {
+		free(pipewire);
+		return 0;
+	}
+
 	pipewire->virtual_output_api = api;
 	pipewire->compositor = compositor;
 	wl_list_init(&pipewire->output_list);
@@ -821,11 +828,10 @@ weston_module_init(struct weston_compositor *compositor)
 			"Debug messages from pipewire plugin\n",
 			NULL, NULL, NULL);
 
-	pipewire->destroy_listener.notify = weston_pipewire_destroy;
-	wl_signal_add(&compositor->destroy_signal, &pipewire->destroy_listener);
 	return 0;
 
 failed:
+	wl_list_remove(&pipewire->destroy_listener.link);
 	free(pipewire);
 	return -1;
 }
diff --git a/remoting/remoting-plugin.c b/remoting/remoting-plugin.c
index 8b82572a..0928a6b1 100644
--- a/remoting/remoting-plugin.c
+++ b/remoting/remoting-plugin.c
@@ -913,6 +913,13 @@ weston_module_init(struct weston_compositor *compositor)
 	if (!remoting)
 		return -1;
 
+	if (!weston_compositor_add_destroy_listener_once(compositor,
+							 &remoting->destroy_listener,
+							 weston_remoting_destroy)) {
+		free(remoting);
+		return 0;
+	}
+
 	remoting->virtual_output_api = api;
 	remoting->compositor = compositor;
 	wl_list_init(&remoting->output_list);
@@ -932,11 +939,10 @@ weston_module_init(struct weston_compositor *compositor)
 		goto failed;
 	}
 
-	remoting->destroy_listener.notify = weston_remoting_destroy;
-	wl_signal_add(&compositor->destroy_signal, &remoting->destroy_listener);
 	return 0;
 
 failed:
+	wl_list_remove(&remoting->destroy_listener.link);
 	free(remoting);
 	return -1;
 }
diff --git a/tests/weston-test-desktop-shell.c b/tests/weston-test-desktop-shell.c
index ac0ece7b..f3d881b8 100644
--- a/tests/weston-test-desktop-shell.c
+++ b/tests/weston-test-desktop-shell.c
@@ -181,8 +181,12 @@ wet_shell_init(struct weston_compositor *ec,
 	if (!dts)
 		return -1;
 
-	dts->compositor_destroy_listener.notify = shell_destroy;
-	wl_signal_add(&ec->destroy_signal, &dts->compositor_destroy_listener);
+	if (!weston_compositor_add_destroy_listener_once(ec,
+							 &dts->compositor_destroy_listener,
+							 shell_destroy)) {
+		free(dts);
+		return 0;
+	}
 
 	weston_layer_init(&dts->layer, ec);
 	weston_layer_init(&dts->background_layer, ec);
diff --git a/tests/weston-test.c b/tests/weston-test.c
index 38505f0b..4b9f1135 100644
--- a/tests/weston-test.c
+++ b/tests/weston-test.c
@@ -46,6 +46,9 @@
 
 struct weston_test {
 	struct weston_compositor *compositor;
+	/* XXX: missing compositor destroy listener
+	 * https://gitlab.freedesktop.org/wayland/weston/issues/300
+	 */
 	struct weston_layer layer;
 	struct weston_process process;
 	struct weston_seat seat;
diff --git a/xwayland/launcher.c b/xwayland/launcher.c
index 1d51ee8f..2f2cebbe 100644
--- a/xwayland/launcher.c
+++ b/xwayland/launcher.c
@@ -367,19 +367,24 @@ weston_module_init(struct weston_compositor *compositor)
 	wxs->wl_display = display;
 	wxs->compositor = compositor;
 
+	if (!weston_compositor_add_destroy_listener_once(compositor,
+							 &wxs->destroy_listener,
+							 weston_xserver_destroy)) {
+		free(wxs);
+		return 0;
+	}
+
 	if (weston_xwayland_get_api(compositor) != NULL ||
 	    weston_xwayland_surface_get_api(compositor) != NULL) {
 		weston_log("The xwayland module APIs are already loaded.\n");
-		free(wxs);
-		return -1;
+		goto out_free;
 	}
 
 	ret = weston_plugin_api_register(compositor, WESTON_XWAYLAND_API_NAME,
 					 &api, sizeof(api));
 	if (ret < 0) {
 		weston_log("Failed to register the xwayland module API.\n");
-		free(wxs);
-		return -1;
+		goto out_free;
 	}
 
 	ret = weston_plugin_api_register(compositor,
@@ -387,13 +392,9 @@ weston_module_init(struct weston_compositor *compositor)
 					 &surface_api, sizeof(surface_api));
 	if (ret < 0) {
 		weston_log("Failed to register the xwayland surface API.\n");
-		free(wxs);
-		return -1;
+		goto out_free;
 	}
 
-	wxs->destroy_listener.notify = weston_xserver_destroy;
-	wl_signal_add(&compositor->destroy_signal, &wxs->destroy_listener);
-
 	wxs->wm_debug =
 		weston_compositor_add_log_scope(wxs->compositor->weston_log_ctx,
 						"xwm-wm-x11",
@@ -401,4 +402,9 @@ weston_module_init(struct weston_compositor *compositor)
 						NULL, NULL, NULL);
 
 	return 0;
+
+out_free:
+	wl_list_remove(&wxs->destroy_listener.link);
+	free(wxs);
+	return -1;
 }