From 6ffbba3ac13bc16d247ea50ef09cb149d892e125 Mon Sep 17 00:00:00 2001 From: Pekka Paalanen Date: Wed, 6 Nov 2019 12:59:32 +0200 Subject: [PATCH] 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 --- compositor/cms-colord.c | 12 ++++++---- compositor/cms-static.c | 9 ++++++-- compositor/screen-share.c | 3 +++ compositor/systemd-notify.c | 10 +++++---- desktop-shell/shell.c | 9 ++++++-- fullscreen-shell/fullscreen-shell.c | 3 +++ include/libweston/libweston.h | 5 +++++ ivi-shell/hmi-controller.c | 4 ++++ ivi-shell/ivi-shell.c | 10 ++++++--- libweston/compositor.c | 35 +++++++++++++++++++++++++++++ pipewire/pipewire-plugin.c | 10 +++++++-- remoting/remoting-plugin.c | 10 +++++++-- tests/weston-test-desktop-shell.c | 8 +++++-- tests/weston-test.c | 3 +++ xwayland/launcher.c | 24 ++++++++++++-------- 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, - ¬ifier->compositor_destroy_listener); + if (!weston_compositor_add_destroy_listener_once(compositor, + ¬ifier->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; }