On Wed, 4 Apr 2018 10:50:26 +0800 zou lan <[email protected]> wrote:
> Hi Pekka > > Thank you for your comment. They are very helpful. > > >that's a good find. If I understand correctly, we do not handle the > >case where a weston_view is effectively unmapped by removing it from a > >weston_layer. As weston_compositor::view_list is never torn down > >properly but simply rebuilt, the unmapped view is left with dangling > >pointers, and those will be hit when finally destroying the view. > > Your explanation is exactly correct. > > I have verified the optional 1 can't work. > > optional 2 > >> weston_view_damage_below(shsurf->fullscreen.black_view); > >> + > >> wl_list_remove(&shsurf->fullscreen.black_view->link); > > > Removing without init might cause issues with a potential double-remove > >on e.g. view destruction. > > yes. I add the initialization for the list. > > + wl_list_init (&shsurf->fullscreen.black_view->link); > > I agree with you weston_view_unmap maybe better for optional 2. Since I > find crashes caused by invalid pointer, I only concern how to make the > view.link to point the right address. > > No matter what functions use here, this only handle the black views. > Are there other views have the same problems? Especially in ivi shell, I > find the crash exist in many scenarios. > > How would weston to resolve the potential risks? Thank you. Hi, weston_view_unmap() is *the* way to unmap a view, any view. Every bit of code that wants to unmap a view should call weston_view_unmap() to make sure it is done correctly. Open-coding bits of weston_view_unmap() here and there will only lead to bugs and is harder to maintain. So there needs to be a very good reason why something would not call weston_view_unmap() and instead open-code something similar. If there is a such a reason, it should be documented as a comment in the code. My recommendation is to inspect all places where weston_layer_entry_remove() is called, see if they are actually unmapping a view, and if they are, either replace code in that place with a call to weston_view_unmap() or add a comment on why this unmap cannot be done by calling weston_view_unmap(). Mentioning weston_view_unmap() in a comment is also good for posterity, because grepping for it will also reveal places where unmap is done without calling it. Unfortunately we don't have the symmetry with mapping a view. Mapping has been open-coded everywhere, since it used to vary for every place doing it. It is in my hopes to unify and clean that up some day as well. Thanks, pq
pgp1HF0RXPBlH.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
