From: Semi Malinen <[email protected]>

Instead of desktop shell assigning view outputs directly,
use a new method, weston_view_set_output(). The method can
set up an output destroy listener to make sure that views
do not have stale output pointers.

Without this patch it is possible to end up in a scenario
where, e.g. configure_static_view() accesses memory that
has already been freed. The scenario can be provoked by
repeatedly plugging and unplugging a display. The faulty
memory accesses are reported by valgrind.

Signed-off-by: Semi Malinen <[email protected]>
Signed-off-by: Fabien Lahoudere <[email protected]>
---
 desktop-shell/shell.c  | 11 +++++++----
 libweston/compositor.c | 43 +++++++++++++++++++++++++++++++++++++++++--
 libweston/compositor.h |  4 ++++
 3 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index b846e30..1e153cb 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -580,7 +580,7 @@ create_focus_surface(struct weston_compositor *ec,
                free(fsurf);
                return NULL;
        }
-       fsurf->view->output = output;
+       weston_view_set_output(fsurf->view, output);
        fsurf->view->is_mapped = true;
 
        weston_surface_set_size(surface, output->width, output->height);
@@ -2464,7 +2464,7 @@ map(struct desktop_shell *shell, struct shell_surface 
*shsurf,
        shsurf->view->is_mapped = true;
        if (shsurf->state.maximized) {
                surface->output = shsurf->output;
-               shsurf->view->output = shsurf->output;
+               weston_view_set_output(shsurf->view, shsurf->output);
        }
 
        if (!shell->locked) {
@@ -2881,6 +2881,9 @@ configure_static_view(struct weston_view *ev, struct 
weston_layer *layer, int x,
 {
        struct weston_view *v, *next;
 
+       if (!ev->output)
+               return;
+
        wl_list_for_each_safe(v, next, &layer->view_list.link, layer_link.link) 
{
                if (v->output == ev->output && v != ev) {
                        weston_view_unmap(v);
@@ -2970,7 +2973,7 @@ desktop_shell_set_background(struct wl_client *client,
        surface->committed_private = shell;
        weston_surface_set_label_func(surface, background_get_label);
        surface->output = weston_head_from_resource(output_resource)->output;
-       view->output = surface->output;
+       weston_view_set_output(view, surface->output);
 
        sh_output = find_shell_output_from_weston_output(shell, 
surface->output);
        if (sh_output->background_surface) {
@@ -3067,7 +3070,7 @@ desktop_shell_set_panel(struct wl_client *client,
        surface->committed_private = shell;
        weston_surface_set_label_func(surface, panel_get_label);
        surface->output = weston_head_from_resource(output_resource)->output;
-       view->output = surface->output;
+       weston_view_set_output(view, surface->output);
 
        sh_output = find_shell_output_from_weston_output(shell, 
surface->output);
        if (sh_output->panel_surface) {
diff --git a/libweston/compositor.c b/libweston/compositor.c
index dbf61ef..9be66fc 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -1021,6 +1021,45 @@ weston_surface_update_output_mask(struct weston_surface 
*es, uint32_t mask)
        }
 }
 
+static void
+notify_view_output_destroy(struct wl_listener *listener, void *data)
+{
+       struct weston_view *view =
+               container_of(listener,
+                    struct weston_view, output_destroy_listener);
+
+       view->output = NULL;
+       view->output_destroy_listener.notify = NULL;
+}
+
+/** Set the primary output of the view
+ *
+ * \param view The view whose primary output to set
+ * \param output The new primary output for the view
+ *
+ * Set \a output to be the primary output of the \a view.
+ *
+ * Notice that the assignment may be temporary; the primary output could be
+ * automatically changed. Hence, one cannot rely on the value persisting.
+ *
+ * Passing NULL as /a output will set the primary output to NULL.
+ */
+WL_EXPORT void
+weston_view_set_output(struct weston_view *view, struct weston_output *output)
+{
+       if (view->output_destroy_listener.notify) {
+               wl_list_remove(&view->output_destroy_listener.link);
+               view->output_destroy_listener.notify = NULL;
+       }
+       view->output = output;
+       if (output) {
+               view->output_destroy_listener.notify =
+                       notify_view_output_destroy;
+               wl_signal_add(&output->destroy_signal,
+                             &view->output_destroy_listener);
+       }
+}
+
 /** Recalculate which output(s) the surface has views displayed on
  *
  * \param es  The surface to remap to outputs
@@ -1112,7 +1151,7 @@ weston_view_assign_output(struct weston_view *ev)
        }
        pixman_region32_fini(&region);
 
-       ev->output = new_output;
+       weston_view_set_output(ev, new_output);
        ev->output_mask = mask;
 
        weston_surface_assign_output(ev->surface);
@@ -1822,7 +1861,7 @@ weston_view_unmap(struct weston_view *view)
                return;
 
        weston_view_damage_below(view);
-       view->output = NULL;
+       weston_view_set_output(view, NULL);
        view->plane = NULL;
        view->is_mapped = false;
        weston_layer_entry_remove(&view->layer_link);
diff --git a/libweston/compositor.h b/libweston/compositor.h
index ca23407..bfad890 100644
--- a/libweston/compositor.h
+++ b/libweston/compositor.h
@@ -1194,6 +1194,7 @@ struct weston_view {
         * view, inheriting the primary output for related views in shells, etc.
         */
        struct weston_output *output;
+       struct wl_listener output_destroy_listener;
 
        /*
         * A more complete representation of all outputs this surface is
@@ -1409,6 +1410,9 @@ void
 weston_version(int *major, int *minor, int *micro);
 
 void
+weston_view_set_output(struct weston_view *view, struct weston_output *output);
+
+void
 weston_view_update_transform(struct weston_view *view);
 
 void
-- 
1.8.3.1

_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to