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 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 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.
> 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.
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
