[weston,1/3] libweston: Remove signals from the list during de-init

Submitted by Harsha Manjula Mallikarjun (RBEI/ECF3) on Aug. 7, 2018, 1:35 p.m.

Details

Message ID 1533648904-31286-2-git-send-email-harsha.manjulamallikarjun@in.bosch.com
State Accepted
Commit b8b2c72709a16382fe7a75a3476236bd31c0e54c
Headers show
Series "Fix invalid memory access during de-init" ( rev: 1 ) in Wayland (DEPRECATED)

Not browsing as part of any series.

Commit Message

Harsha Manjula Mallikarjun (RBEI/ECF3) Aug. 7, 2018, 1:35 p.m.
During de-init ensure removal of added signals from list. Otherwise
a dongling pointer is left behind which will affect other plugins.

Signed-off-by: Harsha M M <harsha.manjulamallikarjun@in.bosch.com>
---
 compositor/text-backend.c         | 3 +++
 compositor/weston-screenshooter.c | 2 ++
 desktop-shell/shell.c             | 1 +
 3 files changed, 6 insertions(+)

Patch hide | download patch | download mbox

diff --git a/compositor/text-backend.c b/compositor/text-backend.c
index 5424242..4b2e854 100644
--- a/compositor/text-backend.c
+++ b/compositor/text-backend.c
@@ -452,6 +452,7 @@  text_input_manager_notifier_destroy(struct wl_listener *listener, void *data)
 			     struct text_input_manager,
 			     destroy_listener);
 
+	wl_list_remove(&text_input_manager->destroy_listener.link);
 	wl_global_destroy(text_input_manager->text_input_manager_global);
 
 	free(text_input_manager);
@@ -1060,6 +1061,8 @@  text_backend_configuration(struct text_backend *text_backend)
 WL_EXPORT void
 text_backend_destroy(struct text_backend *text_backend)
 {
+	wl_list_remove(&text_backend->seat_created_listener.link);
+
 	if (text_backend->input_method.client) {
 		/* disable respawn */
 		wl_list_remove(&text_backend->client_listener.link);
diff --git a/compositor/weston-screenshooter.c b/compositor/weston-screenshooter.c
index 0994cb4..981aff8 100644
--- a/compositor/weston-screenshooter.c
+++ b/compositor/weston-screenshooter.c
@@ -162,6 +162,8 @@  screenshooter_destroy(struct wl_listener *listener, void *data)
 	struct screenshooter *shooter =
 		container_of(listener, struct screenshooter, destroy_listener);
 
+	wl_list_remove(&shooter->destroy_listener.link);
+
 	wl_global_destroy(shooter->global);
 	free(shooter);
 }
diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index ea3c453..9a44715 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -4911,6 +4911,7 @@  shell_destroy(struct wl_listener *listener, void *data)
 		wl_client_destroy(shell->child.client);
 	}
 
+	wl_list_remove(&shell->destroy_listener.link);
 	wl_list_remove(&shell->idle_listener.link);
 	wl_list_remove(&shell->wake_listener.link);
 	wl_list_remove(&shell->transform_listener.link);

Comments

On Tue, 7 Aug 2018 19:05:02 +0530
Harsha M M <harsha.manjulamallikarjun@in.bosch.com> wrote:

> During de-init ensure removal of added signals from list. Otherwise
> a dongling pointer is left behind which will affect other plugins.
> 
> Signed-off-by: Harsha M M <harsha.manjulamallikarjun@in.bosch.com>
> ---
>  compositor/text-backend.c         | 3 +++
>  compositor/weston-screenshooter.c | 2 ++
>  desktop-shell/shell.c             | 1 +
>  3 files changed, 6 insertions(+)
> 
> diff --git a/compositor/text-backend.c b/compositor/text-backend.c
> index 5424242..4b2e854 100644
> --- a/compositor/text-backend.c
> +++ b/compositor/text-backend.c
> @@ -452,6 +452,7 @@ text_input_manager_notifier_destroy(struct wl_listener *listener, void *data)
>  			     struct text_input_manager,
>  			     destroy_listener);
>  
> +	wl_list_remove(&text_input_manager->destroy_listener.link);
>  	wl_global_destroy(text_input_manager->text_input_manager_global);
>  
>  	free(text_input_manager);
> @@ -1060,6 +1061,8 @@ text_backend_configuration(struct text_backend *text_backend)
>  WL_EXPORT void
>  text_backend_destroy(struct text_backend *text_backend)
>  {
> +	wl_list_remove(&text_backend->seat_created_listener.link);
> +
>  	if (text_backend->input_method.client) {
>  		/* disable respawn */
>  		wl_list_remove(&text_backend->client_listener.link);
> diff --git a/compositor/weston-screenshooter.c b/compositor/weston-screenshooter.c
> index 0994cb4..981aff8 100644
> --- a/compositor/weston-screenshooter.c
> +++ b/compositor/weston-screenshooter.c
> @@ -162,6 +162,8 @@ screenshooter_destroy(struct wl_listener *listener, void *data)
>  	struct screenshooter *shooter =
>  		container_of(listener, struct screenshooter, destroy_listener);
>  
> +	wl_list_remove(&shooter->destroy_listener.link);
> +
>  	wl_global_destroy(shooter->global);
>  	free(shooter);
>  }
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index ea3c453..9a44715 100644
> --- a/desktop-shell/shell.c
> +++ b/desktop-shell/shell.c
> @@ -4911,6 +4911,7 @@ shell_destroy(struct wl_listener *listener, void *data)
>  		wl_client_destroy(shell->child.client);
>  	}
>  
> +	wl_list_remove(&shell->destroy_listener.link);
>  	wl_list_remove(&shell->idle_listener.link);
>  	wl_list_remove(&shell->wake_listener.link);
>  	wl_list_remove(&shell->transform_listener.link);

Hi,

I would have preferred this patch to be split by the signal from which
the listeners are being removed, but I think it's ok anyway.

The seat_created fix seems obviously good.

The weston_compositor::destroy_signal I have checked that we do already
have at least one listener that gets removed on the callback, so we do
need to have all listeners removed.

Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>


Thanks,
pq