On 05/07/16 12:04, Jean-Philippe André wrote: > On 5 July 2016 at 19:50, Tom Hacohen <t...@osg.samsung.com> wrote: > >> >> >> On 05/07/16 11:46, Jean-Philippe André wrote: >>> On 5 July 2016 at 19:39, Tom Hacohen <t...@osg.samsung.com> wrote: >>> >>>> Not the best way to ping me. I read this just because I saw "eo" in the >>>> subject line, not the ping. >>>> >>> >>> Lol. The @tag was supposed to ask phabricator to send you a mail :) >>> >>> Anyhow, I know nothing about del intercept. Don't ask me. I have no >>>> idea. :| All I know is that I don't like it. :) >>>> >>> >>> Ah. No. Not asking about that. I was asking about the use of eo_del() >>> within the class_destructor :) >>> It's bad, smells bad, but at the same time can't see a way around it >>> (except having an EAPI efl_shutdown()). >>> >> >> Looking at the code, it looks like a non-eo question, and just depends >> on how this is implemented. Del intercept will be called, if you want to >> avoid it, you need to null it. It has nothing to do with eo_del. > > > I'm not talking about del_intercept :) > > >> I don't >> think there's a problem with calling it in the class destructor (nothing >> comes to mind), except for obviously if there's code that depends on the >> class being fully constructed, it may fail. >> > > eo_del() / eo_unref() require the entire class hierarchy to be constructed. > We're talking about the class destructor here, not the object destructor. >
I know. > See the previous commit: 85a9bd54303dc0a221b0825b39 > I need to walk the class list in reverse order, otherwise elm_test crashes > when you exit it. I know, I already saw it. > > Basically: calling eo_del() from a class destructor seems like a risky > thing. But I need to free the object and its internal data. > > In theory, the class should still be alive at that point. But what about > the parent classes? That was a problem, but I imagine there can be > complications (eg. with an eo_override). You should walk in reverse. Walking in the right order was a mistake. Maybe it's time to do what we kinda avoided in the past and keep a reference count for classes (internally, no api), so classes will only be deleted after their children have. > > > >> >> -- >> Tom. >> >>> -- >>> Tom. >>> >>> On 05/07/16 11:39, Jean-Philippe ANDRÉ wrote: >>>> jpeg pushed a commit to branch master. >>>> >>>> >>> >> http://git.enlightenment.org/core/efl.git/commit/?id=aaec7d940fc28f0b07c6dc0670cbce75f7b9ba79 >>>> >>>> commit aaec7d940fc28f0b07c6dc0670cbce75f7b9ba79 >>>> Author: Jean-Philippe Andre <jp.an...@samsung.com> >>>> Date: Tue Jul 5 19:19:18 2016 +0900 >>>> >>>> efl: Remove del_intercept before calling eo_del >>>> >>>> In class destructor. Still not sure if we should do this >>>> or just set the pointer to NULL. >>>> >>>> Ping @TAsn >>>> --- >>>> src/lib/evas/canvas/efl_event_key.c | 1 + >>>> src/lib/evas/canvas/efl_event_pointer.c | 1 + >>>> 2 files changed, 2 insertions(+) >>>> >>>> diff --git a/src/lib/evas/canvas/efl_event_key.c >>> b/src/lib/evas/canvas/efl_event_key.c >>>> index 78659f9..421e557 100644 >>>> --- a/src/lib/evas/canvas/efl_event_key.c >>>> +++ b/src/lib/evas/canvas/efl_event_key.c >>>> @@ -63,6 +63,7 @@ EOLIAN static void >>>> _efl_event_key_class_destructor(Eo_Class *klass EINA_UNUSED) >>>> { >>>> // this is a strange situation... >>>> + eo_del_intercept_set(s_cached_event, NULL); >>>> eo_del(s_cached_event); >>>> s_cached_event = NULL; >>>> } >>>> diff --git a/src/lib/evas/canvas/efl_event_pointer.c >>> b/src/lib/evas/canvas/efl_event_pointer.c >>>> index d102c5c..db1d662 100644 >>>> --- a/src/lib/evas/canvas/efl_event_pointer.c >>>> +++ b/src/lib/evas/canvas/efl_event_pointer.c >>>> @@ -74,6 +74,7 @@ EOLIAN static void >>>> _efl_event_pointer_class_destructor(Eo_Class *klass EINA_UNUSED) >>>> { >>>> // this is a strange situation... >>>> + eo_del_intercept_set(s_cached_event, NULL); >>>> eo_del(s_cached_event); >>>> s_cached_event = NULL; >>>> } >>>> >>> >>> >>> >> ------------------------------------------------------------------------------ >>> Attend Shape: An AT&T Tech Expo July 15-16. Meet us at AT&T Park in San >>> Francisco, CA to explore cutting-edge tech and listen to tech luminaries >>> present their vision of the future. This family event has something for >>> everyone, including kids. Get more information and register today. >>> http://sdm.link/attshape >>> _______________________________________________ >>> enlightenment-devel mailing list >>> enlightenment-devel@lists.sourceforge.net >>> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel >>> >>> >>> >> >> >> >> ------------------------------------------------------------------------------ >> Attend Shape: An AT&T Tech Expo July 15-16. Meet us at AT&T Park in San >> Francisco, CA to explore cutting-edge tech and listen to tech luminaries >> present their vision of the future. This family event has something for >> everyone, including kids. Get more information and register today. >> http://sdm.link/attshape >> _______________________________________________ >> enlightenment-devel mailing list >> enlightenment-devel@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel >> > > > ------------------------------------------------------------------------------ Attend Shape: An AT&T Tech Expo July 15-16. Meet us at AT&T Park in San Francisco, CA to explore cutting-edge tech and listen to tech luminaries present their vision of the future. This family event has something for everyone, including kids. Get more information and register today. http://sdm.link/attshape _______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
