On Thu, 20 Aug 2015 09:09:44 +0300 Daniel Zaoui <daniel.za...@samsung.com> said:
> Hi, > > On Wed, 19 Aug 2015 20:53:53 -0700 > Carsten Haitzler <ras...@rasterman.com> wrote: > > > raster pushed a commit to branch master. > > > > http://git.enlightenment.org/core/efl.git/commit/?id=8689d54471aafdd7a5b5a27ce116bf2ab68c1042 > > > > commit 8689d54471aafdd7a5b5a27ce116bf2ab68c1042 > > Author: Carsten Haitzler (Rasterman) <ras...@rasterman.com> > > Date: Thu Aug 20 12:50:52 2015 +0900 > > > > eo - destruction - ensure child is removed from parent child list > > > > this follows on from cbc1a217bfc8b5c6dd94f0448f19245c43eb05e0 as > > this code was correct, but was then causing bugs due to children > > staying in their parent lists. this should never have happened and > > this is really bad. this fixes this and ensures children on > > destruction are gone from their parent lists. > > > > @fix > > --- > > src/lib/eo/eo_base_class.c | 14 ++++++++++---- > > 1 file changed, 10 insertions(+), 4 deletions(-) > > > > diff --git a/src/lib/eo/eo_base_class.c b/src/lib/eo/eo_base_class.c > > index fe52203..9f8252b 100644 > > --- a/src/lib/eo/eo_base_class.c > > +++ b/src/lib/eo/eo_base_class.c > > @@ -977,7 +977,6 @@ EOLIAN static void > > _eo_base_destructor(Eo *obj, Eo_Base_Data *pd) > > { > > Eo *child; > > - Eo_Base_Data *child_pd; > > > > DBG("%p - %s.", obj, eo_class_name_get(MY_CLASS)); > > > > @@ -987,11 +986,18 @@ _eo_base_destructor(Eo *obj, Eo_Base_Data *pd) > > while (pd->children) > > { > > child = eina_list_data_get(pd->children); > > - child_pd = eo_data_scope_get(child, EO_BASE_CLASS); > > - pd->children = eina_list_remove_list(pd->children, > > pd->children); > > - child_pd->parent_list = NULL; > > eo_do(child, eo_parent_set(NULL)); > > } > > + // remove child from its parent on destruction - ha to be done > > + if (pd->parent) > > + { > > + Eo_Base_Data *parent_pd; > > + > > + parent_pd = eo_data_scope_get(pd->parent, EO_BASE_CLASS); > > + parent_pd->children = > > eina_list_remove_list(parent_pd->children, > > + pd->parent_list); > > + pd->parent_list = NULL; > > + } > > > > _eo_generic_data_del_all(pd); > > _wref_destruct(pd); > > > > The parent should never be !NULL when reaching the destructor. Imo, this code > has not to be here. Instead, an error message should be displayed in the case > the parent is still connected to the object. There is a bug but definitely > the solution doesn't have to be here. I think this issue may happen if eo_del > is never called and eo_unref is called instead. We need to check inside > _eo_unref that the parent is NULL and display an error message. > > Tom, any thoughts? eo_suite simple examples show this case. in fact i think the test case is broken too... but as such i followed the code, and read it. if you destruct a child, the parent still has a list entry pointing to it. _eo_unref() gets to 0, and _eo_del_internal() calls the destructor... this does nothing in the way of removing from a parent (read it). the base class destructor is the only place to do this... and so i added it. look at START_TEST(eo_refs) here: obj = eo_add(SIMPLE_CLASS, NULL); obj2 = eo_add(SIMPLE_CLASS, obj); eo_unref(obj2); eo_ref(obj2); eo_del(obj2); eo_unref(obj); we create obj, then obj2 as a child of obj, then unref obj2. after this unref of obj2, obj2 was still in the child list of obj. this isn't a complex case. its an insanely simple one. our own test cases never caught this issue until i "fixed" the code above that pointed this out. the base class destructor never removes an object from its parent list. no code in the destructor to do that at all. i fixed that :) -- ------------- Codito, ergo sum - "I code, therefore I am" -------------- The Rasterman (Carsten Haitzler) ras...@rasterman.com ------------------------------------------------------------------------------ _______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
