On Wed, 18 Apr 2018 12:38:46 -0400 Cedric Bail <ced...@ddlm.me> said:
> On April 18, 2018 12:48 AM, Carsten Haitzler <ras...@rasterman.com> wrote:
> > On Tue, 17 Apr 2018 19:33:02 -0400 Cedric Bail ced...@ddlm.me said:
> > > On March 29, 2018 10:58 AM, Carsten Haitzler ras...@rasterman.com wrote:
> > > > On Thu, 29 Mar 2018 13:17:43 -0400 Cedric Bail ced...@ddlm.me said:
> > > > > On March 28, 2018 8:06 PM, Carsten Haitzler ras...@rasterman.com
> > > > > wrote:
> > > > > > On Wed, 28 Mar 2018 12:37:35 -0400 Cedric Bail ced...@ddlm.me said:
> > > > > > > On March 27, 2018 10:32 PM, Carsten Haitzler ras...@rasterman.com
> > > > > > > wrote:
> > > > > > > > On Tue, 20 Mar 2018 17:57:47 -0700 Cedric BAIL
> > > > > > > > cedric.b...@free.fr said:
> > > > > > > >
> > > > > > > > Ideally yes, but at least for the _example, it requires to
> > > > > > > > spend some more time to move them to use EFL_MAIN, which should
> > > > > > > > be done in a different patch not related to fixing the usage of
> > > > > > > > efl_add.
> > > > > >
> > > > > > some use EFL_MAIN ... otherwise efl_main_loop_get() will work. but
> > > > > > moving to efl_add_ref() isn't the "right direction" for most of
> > > > > > these.
> > > > >
> > > > > Yeah, I haven't been a big fan of efl_main_loop_get and did tend to
> > > > > avoid it. I should use it more often in the future.
> > > >
> > > > it'd be good to stop doing that... :) long term at least. the slow and
> > > > steady steps are to fix our parenting in efl and examples. :) i fixed it
> > > > for you - or well i fixed everything i saw you changed. it may not be
> > > > perfect, but it's a step in the right direction imho :) point out if i
> > > > got something wrong. i didn't do "Extensive" work like you say and
> > > > convert to EFL_MAIN or within efl modify code to pass parents around
> > > > many levels down or something.
> > >
> > > So I have been working on finishing correcting the lifecycle of eo object
> > > by making sure that efl_invalidate does happen before any efl_destructor
> > > code (Currently in master, it is possible to get efl_invalidate to be
> > > called after the object destructor as it is called inside
> > > efl.object.destructor). This means that efl_del and efl_parent_{get,set}
> > > are a typical wrong piece of code in the destructore. The patch
> > > 2fb5cc3ad09f6aaf82b5d1131ac5ed22ed848bd4 is wrong in a lot of place that
> > > I had to give up fixing it due to time constraint (basically it assume
> > > our use before invalidate was efl_add/efl_del, while the closest to our
> > > pattern was efl_add_ref with efl_unref in the desstructor). I have a
> > > branch devs/cedric/lifecycle with all the change in it including a revert
> > > of your patch. I can't land this patch at the moment as the old eo base
> > > efl_future lifecycle is a complete wreck with this change, so I will
> > > focus back on removing efl_future. If you had time in the mean time to
> > > check your patch and see how to fix it, it would be great.
> >
> > wait wait. first. some objects. actually a fair few objects need to know
> > their parents to find their loop provider etc. an they need to do this on
> > "shut down" to delete stuff etc. from here. so my question will be:
> >
> > where should an object do this "shut down" in the invalidate or in
> > destructor. you seem to say that it mys be in invalidate which means
> > invalidate effectifely is the "meat" of the destructor, just the object
> > keeps itself around with some minimal content until destructor is called.
> > so what you are doing is ensuring there is ALWAYS the sequence of
> > invalidate then destroy, right?
>
> So yes, invalidate should always happen before the destructor.
>
> > so i'm not sure what's wrong with that patch - can you detail it? i simply
> > moved to having a parent instead of no parent in many cases, and used
> > efl_del again like it used to be which should clear out the only reference
> > to the object by unparenting it etc. etc. ...
>
> The problem is that efl_del in a destructor will always be either wrong or
> bad. Wrong as in you are not the parent/owner of that reference and are
wait a sec... at least at the time of the patches in question here, efl was
still basically all using destructor only. no invalidate existed (or was used).
it's still basically not used except for canvas objects, vg node objects, base
class, smart layout objects, model containers and efl app. so let's say its
essentially not used. the invalidate stuff was put into the efl tree after the
patches in question here, so at least my partial revert of your changes (back
to using parents and efl del) was correct at the time, or as correct as was
possible. is this correct? was there anything wrong at that stage?
now calling efl_del in a destructor... i am unsure why it would often or always
be wrong? if you are going to manually destroy some object (a child or some
other linked object). at least in a world where invalidate is essentially
unused (see above) then the only place it can happen is in the destructor... my
reading of efl_object_invalidate in the base class is that it will clear
futures and unhook the parent of the object but do nothing to children, so that
means children have not been deleted yet as you say below... as the only
efl_del()'s in a destructor are for child objects OR for linked objects that
are not actually children but need deletion anyway... so i'm still confused. :(
> trying to destroy it manually during destructor. Bad as in the child object
> has already lost its parent during the invalidate call on the parent, and if
ummm how can this happen:
static void
_efl_object_invalidate(Eo *obj, Efl_Object_Data *pd)
{
_efl_pending_futures_clear(pd);
efl_parent_set(obj, NULL);
pd->invalidate = EINA_TRUE;
}
the PARENT obj (obj above) loses its parent, and if it has efl_del in the
destructor of obj that deletes other child objects of "obj" i don;'t see how
these are bad/wrong... so as above. i'm still confused. the invalidate on obj
doesn't iterate over children invalidating them too... at least my quick read of
the code doesn't say that... if it did then you'd be right.
> it is the only reference you had on it, it will be dead when reaching the
> destructor. So during destructor calling efl_del is likely accessing a dead
> object. Basically if we do efl_add, we should not need to call efl_del in the
> majority of case, just NULL the reference in the structure during invalidate,
> that's it. It does simplify the code a lot actually, but it differ from the
> code pattern we have and require more work than just replacing
> efl_add_ref/efl_unref by efl_add/efl_del.
ummm it would make sense if the children were iterated over and invalidated...
it's not what i see, so... ummm i'm confused. :)
> > if you reverted the patch you would have lost all the "correct" parenting so
> > i'm not sure that that is going to be better at all, unless you kept that
> > and it's a partial revert really. i haven't looked at your branch yet, but
> > it's kind of not making a lot of sense in this email right now. i am not
> > sure what needs fixing where and how from this email at all, so it'd be
> > nice for you to explain some more...
>
> In most case where efl_add_ref was used with efl_unref, the parenting was not
> a problem and the code was working. It was not optimally using eo capability
> for sure.
but since it needed to be done, it's now done anyway. :) so it's probably not
worth undoing that, but instead looking at this del problem you say we have,
but i can't seem to grasp... unless you've changed the above in your branch and
i should dig around there (the invalidate on base class iterates over children
invalidating them for example)?
> Cedric
>
> ------------------------------------------------------------------------------
> 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
