On Fri, 13 Jun 2014 18:10:26 +0200 George Kiagiadakis <[email protected]> wrote:
> This is to avoid recursing into weston_compositor_build_view_list() > and therefore fix crashing when destroying a stack of visible subsurfaces > due to weston_compositor_build_view_list() being called recursively > and corrupting the lists it works on. > > https://bugs.freedesktop.org/show_bug.cgi?id=79684 > --- > src/compositor.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/src/compositor.c b/src/compositor.c > index 3c5c8e3..2fbfdbf 100644 > --- a/src/compositor.c > +++ b/src/compositor.c > @@ -1662,8 +1662,10 @@ surface_free_unused_subsurface_views(struct > weston_surface *surface) > if (sub->surface == surface) > continue; > > - wl_list_for_each_safe(view, nv, &sub->unused_views, > surface_link) > + wl_list_for_each_safe(view, nv, &sub->unused_views, > surface_link) { > + weston_view_unmap (view); > weston_view_destroy(view); > + } > > surface_free_unused_subsurface_views(sub->surface); > } > @@ -2764,8 +2766,10 @@ weston_subsurface_destroy(struct weston_subsurface > *sub) > assert(sub->parent_destroy_listener.notify == > subsurface_handle_parent_destroy); > > - wl_list_for_each_safe(view, next, &sub->surface->views, > surface_link) > + wl_list_for_each_safe(view, next, &sub->surface->views, > surface_link) { > + weston_view_unmap(view); > weston_view_destroy(view); > + } > > if (sub->parent) > weston_subsurface_unlink_parent(sub); Hi, this patch was already merged and it fixes a real issue, but I am not completely sure the latter hunk is totally correct alone. weston_subsurface_destroy() is called when a sub-surface gets destroyed for any reason. The above hunk prevents the compositor->view_list from being updated, and this could be a theoretical problem, if the destruction is caused directly by a client destroying a wl_sub_surface or wl_surface, and that sub-surface has sub-surfaces of its own. These nested sub-surfaces would need to be removed from the view_list too, as they should become unmapped. Weston_compositor_pick_view() could erroneusly target this supposedly unmapped-but-not-actually sub-sub-surface. In practise, I think hitting this problem will be very hard. It is probably quite rare to use nested sub-surfaces, and the very next compositor repaint will fix up the view_list, so we won't even get a visual glitch. The only thing that could happen AFAIU is that some input events targets a wl_surface they shouldn't, and that wl_surface will disappear very soon anyway. Also, since destroying a surface so that it unmaps causes a repaint, so the window for the fault is really theoretical at best. So, all is probably just fine. *shrug* Thanks, pq _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
