Thanks for providing your reproduction case, I found it to be very helpful! By rapidly crashing or killing applications inside efl_wl objects I was able to find a number of issues, including the one described in this commit.
I've reverted this patch; the crash was an important marker for detecting issues resulting from flawed spec implementations, so it may be useful again in the future for resolving similar cases. The changes here would also have broken seat-related functions, as there is the assumption that the active_surface member belongs to the seat's active_client, and having a set active_client and an unset active_surface is not possible; simply unsetting active_client would similarly be wrong in the case that multiple surfaces exist for a given client. All surfaces should be deactivated when unmapped, and it is not possible according to spec for an unmapped surface to be activated. This has been resolved, along with some related spec violations. In the case where this is somehow reached again, it will still be a spec violation, and at that point the best solution is to remove any deviations from the spec. Thanks again for your help in tracking this down! On Wed, Jan 17, 2018 at 4:24 AM Carsten Haitzler <ras...@rasterman.com> wrote: > On Tue, 16 Jan 2018 14:56:49 +0000 Mike Blumenkrantz > <michael.blumenkra...@gmail.com> said: > > > I'd be interested in knowing how you came across this issue since that > > functionality should be handled automatically on surface hide. This patch > > is not a complete solution and will break some seat methods, so it would > be > > great if we could get that cleaned up. > > messing with the new "process gadgets" that require wayland. just kill the > gadget processes (accidentally, segv , whatever) and this was hit. > > the code was missing to handle the case of destroy the surf while it's > still > registered as the active one. > > i'm not sure how this would break anything? can you explain > > if you (the proverbial you) are busy freeing the comp surf object (which is > where this patch sits) and keep the dangling pointer to it in another > struct > (the comp struct) after the free is done... and then you later use this ptr > (used in various places) it is a bug. i did hit it. so ensuring if the surf > being destroyed is the one registered and then NULL it is safe. > > if you want code to NULL it BEFORE being destroyed... then that's fine. > this > code wouldn't keep hurting anything? this ensures that at death the > reference > there is removed. it seemingly wasn't in other code paths. > > > On Tue, Jan 16, 2018 at 3:50 AM Carsten Haitzler <ras...@rasterman.com> > > wrote: > > > > > raster pushed a commit to branch master. > > > > > > > > > > http://git.enlightenment.org/core/efl.git/commit/?id=574ef9a5e53ac2ecabc14fd02be7e55cd90a9b25 > > > > > > commit 574ef9a5e53ac2ecabc14fd02be7e55cd90a9b25 > > > Author: Carsten Haitzler (Rasterman) <ras...@rasterman.com> > > > Date: Tue Jan 16 14:50:31 2018 +0900 > > > > > > efl wl - fix segv where garbage ptr to comp surface is still there > > > > > > comp surface destruction doesnt clear the pointer to one in the > comp > > > struct that then is garbage later. fix. > > > --- > > > src/lib/efl_wl/efl_wl.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/src/lib/efl_wl/efl_wl.c b/src/lib/efl_wl/efl_wl.c > > > index 28a0814710..40667cabf9 100644 > > > --- a/src/lib/efl_wl/efl_wl.c > > > +++ b/src/lib/efl_wl/efl_wl.c > > > @@ -2321,6 +2321,9 @@ comp_surface_smart_del(Evas_Object *obj) > > > { > > > Comp_Surface *cs = evas_object_smart_data_get(obj); > > > > > > + if ((cs->c) && (cs->c->active_surface == cs)) > > > + cs->c->active_surface = NULL; > > > + > > > array_clear(&cs->input_rects); > > > array_clear(&cs->opaque_rects); > > > eina_tiler_free(cs->opaque); > > > > > > -- > > > > > > > > > > > > ------------------------------------------------------------------------------ > > Check out the vibrant tech community on one of the world's most > > engaging tech sites, Slashdot.org! http://sdm.link/slashdot > > _______________________________________________ > > enlightenment-devel mailing list > > enlightenment-devel@lists.sourceforge.net > > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > > > > > -- > ------------- Codito, ergo sum - "I code, therefore I am" -------------- > Carsten Haitzler - ras...@rasterman.com > > ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
