Re: [PATCH weston] desktop-shell: remove surface listener when surface is destroyed

[Debra Patterson]

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston] desktop-shell: remove surface listener when surface is destroyed

[Pekka Paalanen]

On Wed, 31 Jan 2018 15:15:45 +0900
Tomohito Esaki  wrote:

> There may be race condition between destroying surface and destroying
> output. If handle_output_destroy() is called after surface is destroyed,
> illegal memory access occurs when surface destroy signals is
> unregistered from the panel/background. This patch fixes this issue and
> removes unnecessary initialization for panel surface listener.
> 
> Signed-off-by: Tomohito Esaki 
> ---
>  desktop-shell/shell.c | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index a2a93e2..de76ebe 100644
> --- a/desktop-shell/shell.c
> +++ b/desktop-shell/shell.c
> @@ -2939,6 +2939,7 @@ handle_background_surface_destroy(struct wl_listener 
> *listener, void *data)
>   container_of(listener, struct shell_output, 
> background_surface_listener);
>  
>   weston_log("background surface gone\n");
> + wl_list_remove(>background_surface_listener.link);
>   output->background_surface = NULL;
>  }
>  
> @@ -3023,6 +3024,7 @@ handle_panel_surface_destroy(struct wl_listener 
> *listener, void *data)
>   container_of(listener, struct shell_output, panel_surface_listener);
>  
>   weston_log("panel surface gone\n");
> + wl_list_remove(>panel_surface_listener.link);
>   output->panel_surface = NULL;
>  }
>  
> @@ -4714,8 +4716,10 @@ handle_output_destroy(struct wl_listener *listener, 
> void *data)
>  
>   shell_for_each_layer(shell, shell_output_destroy_move_layer, output);
>  
> - wl_list_remove(_listener->panel_surface_listener.link);
> - wl_list_remove(_listener->background_surface_listener.link);
> + if (output_listener->panel_surface)
> + wl_list_remove(_listener->panel_surface_listener.link);
> + if (output_listener->background_surface)
> + 
> wl_list_remove(_listener->background_surface_listener.link);
>   wl_list_remove(_listener->destroy_listener.link);
>   wl_list_remove(_listener->link);
>   free(output_listener);
> @@ -4761,7 +4765,6 @@ create_shell_output(struct desktop_shell *shell,
>   shell_output->output = output;
>   shell_output->shell = shell;
>   shell_output->destroy_listener.notify = handle_output_destroy;
> - wl_list_init(_output->panel_surface_listener.link);
>   wl_signal_add(>destroy_signal,
> _output->destroy_listener);
>   wl_list_insert(shell->output_list.prev, _output->link);

Hi,

looks good, pushed with my R-b:
   551b8054..bf31f6cf  master -> master


Thanks,
pq


pgp8cWvqkVIBM.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH weston] desktop-shell: remove surface listener when surface is destroyed

[Tomohito Esaki]

There may be race condition between destroying surface and destroying
output. If handle_output_destroy() is called after surface is destroyed,
illegal memory access occurs when surface destroy signals is
unregistered from the panel/background. This patch fixes this issue and
removes unnecessary initialization for panel surface listener.

Signed-off-by: Tomohito Esaki 
---
 desktop-shell/shell.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index a2a93e2..de76ebe 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -2939,6 +2939,7 @@ handle_background_surface_destroy(struct wl_listener 
*listener, void *data)
container_of(listener, struct shell_output, 
background_surface_listener);
 
weston_log("background surface gone\n");
+   wl_list_remove(>background_surface_listener.link);
output->background_surface = NULL;
 }
 
@@ -3023,6 +3024,7 @@ handle_panel_surface_destroy(struct wl_listener 
*listener, void *data)
container_of(listener, struct shell_output, panel_surface_listener);
 
weston_log("panel surface gone\n");
+   wl_list_remove(>panel_surface_listener.link);
output->panel_surface = NULL;
 }
 
@@ -4714,8 +4716,10 @@ handle_output_destroy(struct wl_listener *listener, void 
*data)
 
shell_for_each_layer(shell, shell_output_destroy_move_layer, output);
 
-   wl_list_remove(_listener->panel_surface_listener.link);
-   wl_list_remove(_listener->background_surface_listener.link);
+   if (output_listener->panel_surface)
+   wl_list_remove(_listener->panel_surface_listener.link);
+   if (output_listener->background_surface)
+   
wl_list_remove(_listener->background_surface_listener.link);
wl_list_remove(_listener->destroy_listener.link);
wl_list_remove(_listener->link);
free(output_listener);
@@ -4761,7 +4765,6 @@ create_shell_output(struct desktop_shell *shell,
shell_output->output = output;
shell_output->shell = shell;
shell_output->destroy_listener.notify = handle_output_destroy;
-   wl_list_init(_output->panel_surface_listener.link);
wl_signal_add(>destroy_signal,
  _output->destroy_listener);
wl_list_insert(shell->output_list.prev, _output->link);
-- 
2.7.4

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel