Hi Roman, Thanks for your reply!
On 18 April 2018 at 13:02, Roman Gilg <[email protected]> wrote: > On Wed, Apr 18, 2018 at 11:25 AM, Olivier Fourdan <[email protected]> > wrote: > > This is a quick heads up, I was experiencing random crashes in Xwayland > > (very annoying in gnome-shell and it takes gnome-shell and the entire > > session with it). while running skype with current Xwayladn from 1.20 > rc4. > > Sorry for the annoyances. But I'm of course very glad you found them > before release. Thanks! > > > This is due to commit 82df2ce3: > > > > Author: Roman Gilg <[email protected]> > > Date: Tue Aug 22 15:38:26 2017 +0200 > > > > xwayland: Avoid repeatedly looping through window ancestor chain > > > > Calling xwl_window_from_window means looping through the window > ancestor > > chain whenever it is called on a child window or on an automatically > > redirected window. > > > > Since these properties and the potential ancestor's xwl_window are > > constant > > between window realization and unrealization, we can omit the > looping by > > always putting the respective xwl_window in the Window's private > field > > on > > its realization. If the Window doesn't feature an xwl_window on its > own, > > it's the xwl_window of its first ancestor with one. > > > > Signed-off-by: Roman Gilg <[email protected]> > > Reviewed-by: Pekka Paalanen <[email protected]> > > > > I think the assumption there is invalid in the case of a UnrealizeTree(). > > In UnrealizeTree() it seems indeed the tree is unrealized top to > bottom. What's the reason for doing it this way? Naively I would think > that unrealizing a window tree should run in opposed direction to > realizing one. > I don't know the reason for this. > > Reverting that patch (and adjusting xwayland-present.c accordingly) > solves > > the “invalid read” problem (as we don;t point to freed memoery anymore), > but > > unfortunately that breaks xwayland-present logic that now leaves pending > > buffers and events after the unrealize()... > > Reverting the patch and always calling xwl_window_from_window in the > present code (of course this means that we loop quite often in the > xwayland-present code) should work, since xwl_present_cleanup is > called in xwl_unrealize_window also already for the top parent window. > Yeah, I have a patch ready for that, with some minor tweaks as well. > To fix the logic in xwl_present_cleanup we would need comparison > between event->present_window and xwl_window->present_window. And > destroy the xwl_window->present_frame_callback also if cleanup is > called on the top parent window. > > So should we do this or try to always unrealize bottom-to-top in > UnrealizeTree()? I suspect changing the logic in UnrealizeTree() might unvocer even more problems, sp close to a release. I'd rather fix that in Xwayland code instead. I'll post my patch so you can elaborate on that if you will. Cheers, Olivier
_______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
