Re: [E-devel] efl_add causing confusion

[Jean-Philippe André]

[ignoring everything else in this thread]

Hi,

I have some early prototype work in my branch devs/jpeg/efl_invalidate to
experiment with efl_invalidate.
I am not sure how to exploit it, or if this idea is right, so it's on hold
for now.
If you're interested, have a look, and let us know what your experiments
lead to.

Best regards,

-- 
Jean-Philippe André
--
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

Re: [E-devel] efl_add causing confusion

[Carsten Haitzler]

On Thu, 11 Jan 2018 18:25:37 -0200 Gustavo Sverzut Barbieri
 said:

> On Thu, Jan 11, 2018 at 5:54 AM, Carsten Haitzler 
> wrote:
> > On Wed, 10 Jan 2018 16:52:26 -0200 Gustavo Sverzut Barbieri
> ...
> >> however, in a more generic OOP system this makes things unusable, as
> >> we're discussing in this thread.
> >>
> >> parents: wisely we're advertising almost all objects should have a
> >> parent, like a window, a widget, a loop... maybe it will boils down to
> >> a thread/process.
> >>
> >> however, in these cases children is using/knowing about parents,
> >> parents rarely knows/deals with children -- UNLIKE EVAS.
> >>
> >> --> That said, IMO parents should NOT have a reference to their children.
> >>
> >> if we wish to do cascade deletion, which may be very helpful (delete a
> >> loop, get rid of all users... like done for evas + objects), then we
> >
> > that is exactly what eo is doing. it's doing "automated cascade deletion"
> > from any point in a tree. if a parent goes, then the children go too (or at
> > least conceptually should go). yes. in theory - you delete a loop and
> > everything related to it should go away (that basically would be everything
> > in that thread) ...
> 
> there is a difference in this and the Python/Lua and the likes
> mentioned before, they remove the reference, yes, but they don't lead
> to resource destruction/invalidation. If there's another reference to
> the object, they keep alive.

this would be the same. if you do another efl_ref() or efl_xref() or
efl_key_ref_set() ... they all mean there is another reference to the object
just like in a gc'd lang there being another "live variable" with that obj ref
init so the gc will not clean it up. same thing. it's just done manually by
saying you have a ref (or don't anymore).

> which is different case when the object is actually dependent on the
> parent, like a timer->loop or rectangle->evas.

it's the same in the case the ONLY reference to that object is in the parent
which is actually the common case in most languages - a box in lua (in some
hypothetical lua toolkit) would have a reference to its child objects (buttons
or whatever), and that'd probably be the only reference 99% of the time...

> the cascade delete I meant is "if I delete the loop, it must delete
> all children, be timers OR some random class that just used loop as
> parent".

well it can't if there are "excess references" as i mentioned... :) these ones
may be orphaned.

> currently it's not, being the root cause of Andrews' question on
> whether to unref/del or not... cascade delete is NOT usual in OOP, but
> helps our use cases by a great deal... so may be considered a good
> solution for our purpose-specific OOP that is Eo.

that's the case we care about.

> > though as i mentioned already - if people add extra refs to objects
> > and then don't remove them when this happens some objects would get
> > orphaned at this point and this is a bit of a problem.
> 
> as I wrote in other email, keep handles alive, but invalidated. If we
> follow the GObject guidelines, invalidating object (what they call
> "dispose") should have it to release reference to all other objects,
> but keep around essential data, like Private_Data, maybe allow some
> getters to work, etc. If we opt to force cascade deletion, then the
> base class should also cascade invalidate all children.

yes... but even if invalidated... the object will still be around consuming an
eoid (a table entry) and memory for the object... possibly forever if someone
forgot about this extra ref. it may be terminated but still around. like a
zombie process. zombie objects.

> >> can dissociate terminate/invalidate from reference, but the last
> >> reference would invalidate:
> >>
> >> --> NOTE: using efl_new() instead of efl_add() as I think this should
> >> be renamed for clarity...
> >>
> >> basic usage:
> >> o = efl_new(cls, parent, ...); // refcnt = 1, the caller's reference
> >> efl_unref(o); // refcnt = 0, efl_invalidate(o) is auto-called
> >>
> >>
> >> explicit invalidate:
> >> o = efl_new(cls, parent, ...); // refcnt = 1, the caller's reference
> >> efl_invalidate(o); // refcnt = 1
> >> efl_unref(o); // refcnt = 0, no call to efl_invalidate(o)
> >>
> >> canvas usage #1 (basic usage):
> >> o = efl_new(EFL_UI_BUTTON_CLASS, window, ...); // refcnt = 1, the
> >> caller's reference
> >> efl_unref(o); // refcnt = 0, efl_invalidate(o) is auto-called,
> >> button is removed from window
> >>
> >> canvas usage #2 (invalidate window):
> >> o = efl_new(EFL_UI_BUTTON_CLASS, window, ...); // refcnt = 1, the
> >> caller's reference
> >> efl_invalidate(window); // o.refcnt = 1, but efl_invalidate(o) is
> >> called efl_unref(o); // refcnt = 0, no call efl_invalidate(o)
> >>
> >> problem with this approach is that most people will forget to
> >> efl_unref(o), then while it will be invalidated, its memory will leak.
> >
> > i thought 

Re: [E-devel] efl_add causing confusion

[Gustavo Sverzut Barbieri]

On Thu, Jan 11, 2018 at 5:54 AM, Carsten Haitzler  wrote:
> On Wed, 10 Jan 2018 16:52:26 -0200 Gustavo Sverzut Barbieri
...
>> however, in a more generic OOP system this makes things unusable, as
>> we're discussing in this thread.
>>
>> parents: wisely we're advertising almost all objects should have a
>> parent, like a window, a widget, a loop... maybe it will boils down to
>> a thread/process.
>>
>> however, in these cases children is using/knowing about parents,
>> parents rarely knows/deals with children -- UNLIKE EVAS.
>>
>> --> That said, IMO parents should NOT have a reference to their children.
>>
>> if we wish to do cascade deletion, which may be very helpful (delete a
>> loop, get rid of all users... like done for evas + objects), then we
>
> that is exactly what eo is doing. it's doing "automated cascade deletion" from
> any point in a tree. if a parent goes, then the children go too (or at least
> conceptually should go). yes. in theory - you delete a loop and everything
> related to it should go away (that basically would be everything in that
> thread) ...

there is a difference in this and the Python/Lua and the likes
mentioned before, they remove the reference, yes, but they don't lead
to resource destruction/invalidation. If there's another reference to
the object, they keep alive.

which is different case when the object is actually dependent on the
parent, like a timer->loop or rectangle->evas.

the cascade delete I meant is "if I delete the loop, it must delete
all children, be timers OR some random class that just used loop as
parent".

currently it's not, being the root cause of Andrews' question on
whether to unref/del or not... cascade delete is NOT usual in OOP, but
helps our use cases by a great deal... so may be considered a good
solution for our purpose-specific OOP that is Eo.


> though as i mentioned already - if people add extra refs to objects
> and then don't remove them when this happens some objects would get orphaned 
> at
> this point and this is a bit of a problem.

as I wrote in other email, keep handles alive, but invalidated. If we
follow the GObject guidelines, invalidating object (what they call
"dispose") should have it to release reference to all other objects,
but keep around essential data, like Private_Data, maybe allow some
getters to work, etc. If we opt to force cascade deletion, then the
base class should also cascade invalidate all children.


>> can dissociate terminate/invalidate from reference, but the last
>> reference would invalidate:
>>
>> --> NOTE: using efl_new() instead of efl_add() as I think this should
>> be renamed for clarity...
>>
>> basic usage:
>> o = efl_new(cls, parent, ...); // refcnt = 1, the caller's reference
>> efl_unref(o); // refcnt = 0, efl_invalidate(o) is auto-called
>>
>>
>> explicit invalidate:
>> o = efl_new(cls, parent, ...); // refcnt = 1, the caller's reference
>> efl_invalidate(o); // refcnt = 1
>> efl_unref(o); // refcnt = 0, no call to efl_invalidate(o)
>>
>> canvas usage #1 (basic usage):
>> o = efl_new(EFL_UI_BUTTON_CLASS, window, ...); // refcnt = 1, the
>> caller's reference
>> efl_unref(o); // refcnt = 0, efl_invalidate(o) is auto-called,
>> button is removed from window
>>
>> canvas usage #2 (invalidate window):
>> o = efl_new(EFL_UI_BUTTON_CLASS, window, ...); // refcnt = 1, the
>> caller's reference
>> efl_invalidate(window); // o.refcnt = 1, but efl_invalidate(o) is called
>> efl_unref(o); // refcnt = 0, no call efl_invalidate(o)
>>
>> problem with this approach is that most people will forget to
>> efl_unref(o), then while it will be invalidated, its memory will leak.
>
> i thought cedric mentioned this already - efl_del would invalidate then unref
> in one call, so you don't forget. ? i dont see why we should move to efl_new
> from efl_add either. :)

in this specific example is to make clear the reference ownership. You
called "new", you call "unref". Invalidate doesn't change that (root
of our usability issues, as pointed out by Andrew).

the name "new" instead of "add" is to make it clear you're not adding
stuff to parent, just using it as a parent handle... it won't keep a
reference, etc... which I'd imply from something called "add".


>> while this can be solved within evas legacy wrapper, the usability is
>> not the best. Other systems "solve" this with one of two options:
>>
>>  - reference transference: efl_adopt_ref(receiver, o), would give your
>> reference of "o" to "receiver", which is supposed to release it on its
>> "invalidate". That is: keep an Eina_List *adopted, that is
>> automatically managed and efl_unref() on its own invalidate.
>>
>>  - floating references: object starts "unowned" and the first "user"
>> should own it, for details:
>> https://developer.gnome.org/gobject/stable/gobject-The-Base-Object-Type.html#floating-ref
>
> that is how eo currently works. eo tracks the first time an object gets a
> parent. if we 

Re: [E-devel] efl_add causing confusion

[Felipe Magno de Almeida]

On Thu, Jan 11, 2018 at 6:00 PM, Gustavo Sverzut Barbieri
 wrote:
> On Thu, Jan 11, 2018 at 5:56 AM, Jean-Philippe André  
> wrote:
> ...

[snip]

>>> WRONG/BUG usage:
>>> o = efl_add(cls, parent, ...); // refcnt = 1, the PARENT's reference
>>> efl_unref(o); // refcnt = 0, but parent held the reference
>>>
>>
>> so this should ERR like now?
>
> YES, but unref/ref will be demoted, not handled like it's done now.
>
> basically now we all handle efl_unref() at some point, as Andrew said
> it's confusing when you need to do or not.
>
> to "spot for bugs" it will be possible to grep efl_ref/efl_unref ;-)
>
> to mitigate those, my recommendation is to generate new eoid in
> efl_ref() and invalidate it on the efl_unref()... like if they were 2
> different handles, but they point to the same underlying object.
> prevents too-much-unref, since the handle will be invalid.
>
> we can have the compiler to help by using proper function attributes,
> like done for malloc/free, warning unused results, etc.

This is an interesting idea. I wonder how expensive this is.
And if we could have a reference that "lives until next loop run"?

Regards,

[snip]

>
> --
> Gustavo Sverzut Barbieri
> --
> Mobile: +55 (16) 99354-9890
>
> --
> 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



-- 
Felipe Magno de Almeida

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

Re: [E-devel] efl_add causing confusion

[Gustavo Sverzut Barbieri]

On Thu, Jan 11, 2018 at 5:56 AM, Jean-Philippe André  wrote:
...
>> if we wish to do cascade deletion, which may be very helpful (delete a
>> loop, get rid of all users... like done for evas + objects), then we
>> can dissociate terminate/invalidate from reference, but the last
>> reference would invalidate:
>>
>> --> NOTE: using efl_new() instead of efl_add() as I think this should
>> be renamed for clarity...
>>
>> basic usage:
>> o = efl_new(cls, parent, ...); // refcnt = 1, the caller's reference
>> efl_unref(o); // refcnt = 0, efl_invalidate(o) is auto-called
>>
>>
>> explicit invalidate:
>> o = efl_new(cls, parent, ...); // refcnt = 1, the caller's reference
>> efl_invalidate(o); // refcnt = 1
>> efl_unref(o); // refcnt = 0, no call to efl_invalidate(o)
>>
>> canvas usage #1 (basic usage):
>> o = efl_new(EFL_UI_BUTTON_CLASS, window, ...); // refcnt = 1, the
>> caller's reference
>> efl_unref(o); // refcnt = 0, efl_invalidate(o) is auto-called,
>> button is removed from window
>>
>> canvas usage #2 (invalidate window):
>> o = efl_new(EFL_UI_BUTTON_CLASS, window, ...); // refcnt = 1, the
>> caller's reference
>> efl_invalidate(window); // o.refcnt = 1, but efl_invalidate(o) is
>> called
>> efl_unref(o); // refcnt = 0, no call efl_invalidate(o)
>>
>> problem with this approach is that most people will forget to
>> efl_unref(o), then while it will be invalidated, its memory will leak.
>>
>> while this can be solved within evas legacy wrapper, the usability is
>> not the best. Other systems "solve" this with one of two options:
>>
>>  - reference transference: efl_adopt_ref(receiver, o), would give your
>> reference of "o" to "receiver", which is supposed to release it on its
>> "invalidate". That is: keep an Eina_List *adopted, that is
>> automatically managed and efl_unref() on its own invalidate.
>>
>>  - floating references: object starts "unowned" and the first "user"
>> should own it, for details:
>> https://developer.gnome.org/gobject/stable/gobject-The-Base-
>> Object-Type.html#floating-ref
>>
>> GObject UI and gstreamer use this "floating" and I always disliked it,
>> finding it too error prone. It "looks easy" to use, but the situation
>> is similar to what we have in EFL and led to this thread... so I'd
>> avoid it.
>>
>> since unlike Gtk and GStreamer we're suggesting elements to be created
>> with a parent, as opposed to adding to a parent, this is simpler.
>> Compare:
>>
>> // GTK:
>>container = create_container ();
>>container_add_child (container, create_child());
>>
>>// EFL:
>>container = efl_new(CONTAINER_CLS, loop, );
>>child = efl_new(CHILD_CLS, container, efl_container_pack(container,
>> efl_added), efl_adopt_ref(container, efl_added));
>>
>> this is explicit, clear what's happening. Since "efl_adopt_ref()" was
>> written explicitly there, or called afterwards, it's easy to spot you
>> don't own the reference anymore. Maybe coccinelle, coverity and the
>> likes can told that rule to spot errors.
>>
>
> ah i really don't like efl_adopt_ref
>
>
> others may claim efl_adopt_ref() is always the case, in that case
>> keeping efl_invalidate() UNRELATED to efl_unref() is also helpful:
>>
>> --> NOTE: using efl_add(), since here parents keep the reference
>>
>> WRONG/BUG usage:
>> o = efl_add(cls, parent, ...); // refcnt = 1, the PARENT's reference
>> efl_unref(o); // refcnt = 0, but parent held the reference
>>
>
> so this should ERR like now?

YES, but unref/ref will be demoted, not handled like it's done now.

basically now we all handle efl_unref() at some point, as Andrew said
it's confusing when you need to do or not.

to "spot for bugs" it will be possible to grep efl_ref/efl_unref ;-)

to mitigate those, my recommendation is to generate new eoid in
efl_ref() and invalidate it on the efl_unref()... like if they were 2
different handles, but they point to the same underlying object.
prevents too-much-unref, since the handle will be invalid.

we can have the compiler to help by using proper function attributes,
like done for malloc/free, warning unused results, etc.



> basic usage:
>> o = efl_add(cls, parent, ...); // refcnt = 1, the PARENT's reference
>> efl_invalidate(o); // refcnt = 0, since parent will unref based on
>> invalidate event
>>
>> cascade delete usage:
>> o = efl_add(cls, parent, ...); // refcnt = 1, the PARENT's reference
>> efl_invalidate(parent); // refcnt = 0, cascade invalidate, leading
>> to parents unref children.
>>
>> ref usage (when you can't monitor EFL_EVENT_INVALIDATE or weak-ref):
>> o = efl_add(cls, parent, ...); // refcnt = 1, the PARENT's reference
>> efl_ref(o); // refcnt = 2, PARENT + caller
>> efl_invalidate(o); // refcnt = 1, since parent will unref based on
>> invalidate event
>> efl_unref(o); // refcnt = 0
>>
>
> probably more like:
>   efl_ref(o)
>   efl_invalidate(o) -> triggers "invalidate"
>   some "invalidate" cb 

Re: [E-devel] efl_add causing confusion

[Carsten Haitzler]

On Thu, 11 Jan 2018 11:16:02 +0900 Jean-Philippe André  said:

> Hi
> 
> On Wed, Jan 10, 2018 at 11:39 PM, Carsten Haitzler 
> wrote:
> 
> > On Wed, 10 Jan 2018 16:31:33 +0900 Jean-Philippe André 
> > said:
> >
> > > Hi,
> > >
> > > I created a new event "destruct" in Efl.Object.
> > > It's triggered just before removing the callbacks, in the base class
> > > destructor, as it's both easier and safer this way. I guess this can be
> > > used in bindings too.
> >
> > shouldn't  it be called AFTER all the callbacks EXCEPT this one have been
> > removed? so only callbacks left on the list are these ones? and
> > double-check
> > that callback removal is the very last thing before the actual object data
> > free
> > (ie last thing in base class destructor) ?
> >
> 
> Well that's "almost" the case.
> What's left of the object at this point are:
>  - name & comment
>  - generic data pointers
>  - list of event callbacks
>  - pending futures to cancel

i might sat they futures should be canceled before these "destruct" callbacks
are called. and as i said. delete all other cb types first so the only ones
left are this type?

> The futures should probably be canceled before (I didn't spot that until
> just now).

aaah we do agree. :)

> It's debatable if the generic data should be cleaned up first (in
> particular for the eo values, which trigger efl_unref). Could be cleaned up
> first.

i think the generic data should be cleaned up before this destruct cb. i don't
see why name and comment shouldn't also be nuked first too. if you want allthis
data still then there is the del event when it's still all alive. :)

> But there is no point in cleaning up the list of callbacks as the cleanup
> process is a call to mempool free.
> I doubt it would be better to send the event after the eo id itself becomes
> invalid.

i'm only worries about somehow something doing an event call inside this
destruct cb (well further down the call stack from it most likely) and it'd be
good to ensure no event cb's can be triggered accidentally other than these.

> > > As for the rest, I need to think about it more.
> > >
> > > Best regards,
> > >
> > >
> > > On Wed, Jan 10, 2018 at 1:50 AM, Carsten Haitzler 
> > > wrote:
> > >
> > > > On Tue, 9 Jan 2018 21:03:53 +0900 Jean-Philippe André <
> > j...@videolan.org>
> > > > said:
> > > >
> > > > > On Tue, Jan 9, 2018 at 3:17 PM, Carsten Haitzler <
> > ras...@rasterman.com>
> > > > > wrote:
> > > > >
> > > > > > On Mon, 8 Jan 2018 18:15:13 +0900 Jean-Philippe André <
> > > > j...@videolan.org>
> > > > > > said:
> > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > >
> > > > > > > Some of the argumentation here seems to lack a bit of a
> > scientific
> > > > > > > approach...
> > > > > > > So, I looked for all occurrences of efl_add() (see PS for
> > > > methodology).
> > > > > > >
> > > > > > > In all of EFL, without specific filters:
> > > > > > >
> > > > > > > Overall: 244 unique classes / 1572 occurrences
> > > > > > > No parent: 119 unique classes / 472 occurrences
> > > > > > > 48% of classes used without parent, accounting for 30% of all
> > uses.
> > > > > >
> > > > > > i argue that the vast majority of those no parent cases should
> > have a
> > > > > > parent. i
> > > > > > actually did a grep -r through efl too looking and almost all the
> > cases
> > > > > > with a
> > > > > > null parent i noted were bugs and should be fixed. :) i
> > specifically
> > > > noted
> > > > > > some
> > > > > > of those examples (the efl_anim stuff for example - i filed a bug
> > for
> > > > it
> > > > > > too).
> > > > > > so while i didn't do numbers... i did do an analysis. quickly in
> > about
> > > > 2
> > > > > > minutes. for a more detailed analysis. note when i say "loop
> > should be
> > > > > > parent"
> > > > > > i mean loop directly OR some object that ultimately has a parent
> > that
> > > > is
> > > > > > loop
> > > > > > at the top of the object tree:
> > > > > >
> > > > > > src/benchmarks/eo/eo_bench_* (11)
> > > > > >   just benchmarking so not real code. (ignore)
> > > > > > src/bin/elementary/test_*.c (86)
> > > > > >   all are bugs. every one should have loop as parent.
> > > > > > src/examples/ecore/ecore_audio*.c: (7)
> > > > > >   all are bugs. loop should be parent (loop drives i/o for audio
> > and
> > > > cb's)
> > > > > > src/examples/ecore/ecore_idler_example.c: (1)
> > > > > >   bug. loop as parent (commented out though)
> > > > > > src/examples/ecore/ecore_poller_*.c (3)
> > > > > >   bug. loop should be parent
> > > > > > src/examples/ecore/efl_io_*.c (8)
> > > > > >   bug. loop should be parent
> > > > > > src/examples/ecore/efl_net_*.c (6)
> > > > > >   bug. loop should be parent
> > > > > > src/examples/eio/eio_sentry.c (1)
> > > > > >   unknown
> > > > > > src/examples/elementary/efl_ui_*.c (4)
> > > > > >   bug. loop should be parent
> > > > > > src/examples/elementary/file*.c (9)
> > > 

Re: [E-devel] efl_add causing confusion

[Carsten Haitzler]

On Wed, 10 Jan 2018 16:52:26 -0200 Gustavo Sverzut Barbieri
 said:

> hi all,
> 
> jpeg, i think that this "destruct" event should have a counterpart
> method, like in GObject they have dispose + finalize, we could have
> something like that to allow two clearly distinct phases:
>- one to release references to others (our invalidate/terminate),
> announces that everyone keeping a reference should stop using this
> object (~weak ref behavior)

what is what currently del events are for. they allow you to implement weak
refs yourself in any way you like. or that's the idea. eo also explicitly has a
weak ref system where you register (and unregister) pointers to objects as weak
refs and eo will NULL them in deletion of the referenced target since this is
so common. you just need to remember to unregister the weak ref ptr when it is
being freed (well the parent structure is generally).

>- one to free memory handles and all backing structures, this one
> can be called after all references are gone.
> 
> https://developer.gnome.org/gobject/stable/howto-gobject-destruction.html
> 
> as per parent, as I wrote to some in private, I believe the current
> situation is that Eo tries to cope with an unusual system that was
> used in Evas_Object, which the objects were added to the canvas, which
> held the only reference to it... not by the caller (who created the
> visual object).
> 
> this is unusual in OOP, but improved usability of evas users, nobody
> had to manage object lifecycle, just delete the canvas and they all
> would go away. OTOH, to delete the visual object, an specific
> evas_object_del() was used, that would remove the canvas ref and
> destroy the object.

i don't think it's that unusual. in python, js or lua or java you'd store a
child object or widget inside the parent object's class data. when the parent
is deleted the reference to the child is also. they will all clean up children
because the gc will no longer find a reference to the child (if this was the
only reference) so it works the same really. it just is done by refcounting and
not a gc.

> however, in a more generic OOP system this makes things unusable, as
> we're discussing in this thread.
> 
> parents: wisely we're advertising almost all objects should have a
> parent, like a window, a widget, a loop... maybe it will boils down to
> a thread/process.
> 
> however, in these cases children is using/knowing about parents,
> parents rarely knows/deals with children -- UNLIKE EVAS.
> 
> --> That said, IMO parents should NOT have a reference to their children.
> 
> if we wish to do cascade deletion, which may be very helpful (delete a
> loop, get rid of all users... like done for evas + objects), then we

that is exactly what eo is doing. it's doing "automated cascade deletion" from
any point in a tree. if a parent goes, then the children go too (or at least
conceptually should go). yes. in theory - you delete a loop and everything
related to it should go away (that basically would be everything in that
thread) ... though as i mentioned already - if people add extra refs to objects
and then don't remove them when this happens some objects would get orphaned at
this point and this is a bit of a problem.

what do we do about these orphans? i think we may need to police them. i think
that objects that were a child of a parent, lose that parent (becomes NULL) but
the object is still alive with references should be explicitly tracked by eo
somehow in a list, alongside information where they were orphaned (a
backtrace?) when debugging is on (or maybe always?). and then some api's to
iterate over these orphans as well as just dump out their info (type/class,
eoid, their child trees if they have children, bt where the obj that was
orphaned was placed into this orphan list). this is basically a leak catcher.
catches excess refs that accidentally keep objects alive. i don't think we can
avoid this issue. it's like any of the "oo" stuff you mention above. is someone
keeps a reference (a variable with a ref to an obj) in java (or js, or lua
etc.), even if the parent object died, that child would stay around because
that ref exists. it may not be INTENDED to be like this and that was probably
intended as a weak reference or they just forgot to clean up/null their
reference, but having a way to debug this will be probably important for the
future. unless someone has a magic way of ensuring this never happens (i doubt
there is as it happens to this day in many languages).

> can dissociate terminate/invalidate from reference, but the last
> reference would invalidate:
> 
> --> NOTE: using efl_new() instead of efl_add() as I think this should
> be renamed for clarity...
> 
> basic usage:
> o = efl_new(cls, parent, ...); // refcnt = 1, the caller's reference
> efl_unref(o); // refcnt = 0, efl_invalidate(o) is auto-called
> 
> 
> explicit invalidate:
> o = efl_new(cls, parent, ...); // refcnt = 1, the caller's reference

Re: [E-devel] efl_add causing confusion

[Jean-Philippe André]

On Thu, Jan 11, 2018 at 3:52 AM, Gustavo Sverzut Barbieri <
barbi...@gmail.com> wrote:

> hi all,
>
> jpeg, i think that this "destruct" event should have a counterpart
> method, like in GObject they have dispose + finalize, we could have
> something like that to allow two clearly distinct phases:
>- one to release references to others (our invalidate/terminate),
> announces that everyone keeping a reference should stop using this
> object (~weak ref behavior)
>- one to free memory handles and all backing structures, this one
> can be called after all references are gone.
>
> https://developer.gnome.org/gobject/stable/howto-gobject-destruction.html


Yes I think I get the idea. Basically each object implements an invalidator
and a destructor.

For evas object the invalidator would hide and delete (invalidate and
unref) all children, as well as do anything the object/widget needs to do
to clean itself up.
The destructor should come later. In case of evas object, we have manual
free which should be the destructor, but happens 2 frames later. Not sure
how to do that nicely.

as per parent, as I wrote to some in private, I believe the current
> situation is that Eo tries to cope with an unusual system that was
> used in Evas_Object, which the objects were added to the canvas, which
> held the only reference to it... not by the caller (who created the
> visual object).
>
> this is unusual in OOP, but improved usability of evas users, nobody
> had to manage object lifecycle, just delete the canvas and they all
> would go away. OTOH, to delete the visual object, an specific
> evas_object_del() was used, that would remove the canvas ref and
> destroy the object.
>
> however, in a more generic OOP system this makes things unusable, as
> we're discussing in this thread.
>
> parents: wisely we're advertising almost all objects should have a
> parent, like a window, a widget, a loop... maybe it will boils down to
> a thread/process.
>
> however, in these cases children is using/knowing about parents,
> parents rarely knows/deals with children -- UNLIKE EVAS.
>
> --> That said, IMO parents should NOT have a reference to their children.
>
> if we wish to do cascade deletion, which may be very helpful (delete a
> loop, get rid of all users... like done for evas + objects), then we
> can dissociate terminate/invalidate from reference, but the last
> reference would invalidate:
>
> --> NOTE: using efl_new() instead of efl_add() as I think this should
> be renamed for clarity...
>
> basic usage:
> o = efl_new(cls, parent, ...); // refcnt = 1, the caller's reference
> efl_unref(o); // refcnt = 0, efl_invalidate(o) is auto-called
>
>
> explicit invalidate:
> o = efl_new(cls, parent, ...); // refcnt = 1, the caller's reference
> efl_invalidate(o); // refcnt = 1
> efl_unref(o); // refcnt = 0, no call to efl_invalidate(o)
>
> canvas usage #1 (basic usage):
> o = efl_new(EFL_UI_BUTTON_CLASS, window, ...); // refcnt = 1, the
> caller's reference
> efl_unref(o); // refcnt = 0, efl_invalidate(o) is auto-called,
> button is removed from window
>
> canvas usage #2 (invalidate window):
> o = efl_new(EFL_UI_BUTTON_CLASS, window, ...); // refcnt = 1, the
> caller's reference
> efl_invalidate(window); // o.refcnt = 1, but efl_invalidate(o) is
> called
> efl_unref(o); // refcnt = 0, no call efl_invalidate(o)
>
> problem with this approach is that most people will forget to
> efl_unref(o), then while it will be invalidated, its memory will leak.
>
> while this can be solved within evas legacy wrapper, the usability is
> not the best. Other systems "solve" this with one of two options:
>
>  - reference transference: efl_adopt_ref(receiver, o), would give your
> reference of "o" to "receiver", which is supposed to release it on its
> "invalidate". That is: keep an Eina_List *adopted, that is
> automatically managed and efl_unref() on its own invalidate.
>
>  - floating references: object starts "unowned" and the first "user"
> should own it, for details:
> https://developer.gnome.org/gobject/stable/gobject-The-Base-
> Object-Type.html#floating-ref
>
> GObject UI and gstreamer use this "floating" and I always disliked it,
> finding it too error prone. It "looks easy" to use, but the situation
> is similar to what we have in EFL and led to this thread... so I'd
> avoid it.
>
> since unlike Gtk and GStreamer we're suggesting elements to be created
> with a parent, as opposed to adding to a parent, this is simpler.
> Compare:
>
> // GTK:
>container = create_container ();
>container_add_child (container, create_child());
>
>// EFL:
>container = efl_new(CONTAINER_CLS, loop, );
>child = efl_new(CHILD_CLS, container, efl_container_pack(container,
> efl_added), efl_adopt_ref(container, efl_added));
>
> this is explicit, clear what's happening. Since "efl_adopt_ref()" was
> written explicitly there, or called afterwards, it's easy to spot you
> don't own the reference anymore. 

Re: [E-devel] efl_add causing confusion

[Jean-Philippe André]

Hi Gustavo,

Just replying to the first comment in this mail.

On Thu, Jan 11, 2018 at 3:52 AM, Gustavo Sverzut Barbieri <
barbi...@gmail.com> wrote:

> hi all,
>
> jpeg, i think that this "destruct" event should have a counterpart
> method, like in GObject they have dispose + finalize, we could have
> something like that to allow two clearly distinct phases:
>- one to release references to others (our invalidate/terminate),
> announces that everyone keeping a reference should stop using this
> object (~weak ref behavior)
>

"invalidate/terminate" event


>- one to free memory handles and all backing structures, this one
> can be called after all references are gone.
>

"destruct" event


>
> https://developer.gnome.org/gobject/stable/howto-gobject-destruction.html


Absolutely. Now we have "del" (called when destructor starts) and
"destruct" (called when destructor finishes).
If we implement efl_invalidate then we will also have an "invalidate"
event, which could indeed be used to do weak refs.


I'll think about the below comments further.


>
>
> as per parent, as I wrote to some in private, I believe the current
> situation is that Eo tries to cope with an unusual system that was
> used in Evas_Object, which the objects were added to the canvas, which
> held the only reference to it... not by the caller (who created the
> visual object).
>
> this is unusual in OOP, but improved usability of evas users, nobody
> had to manage object lifecycle, just delete the canvas and they all
> would go away. OTOH, to delete the visual object, an specific
> evas_object_del() was used, that would remove the canvas ref and
> destroy the object.
>
> however, in a more generic OOP system this makes things unusable, as
> we're discussing in this thread.
>
> parents: wisely we're advertising almost all objects should have a
> parent, like a window, a widget, a loop... maybe it will boils down to
> a thread/process.
>
> however, in these cases children is using/knowing about parents,
> parents rarely knows/deals with children -- UNLIKE EVAS.
>
> --> That said, IMO parents should NOT have a reference to their children.
>
> if we wish to do cascade deletion, which may be very helpful (delete a
> loop, get rid of all users... like done for evas + objects), then we
> can dissociate terminate/invalidate from reference, but the last
> reference would invalidate:
>
> --> NOTE: using efl_new() instead of efl_add() as I think this should
> be renamed for clarity...
>
> basic usage:
> o = efl_new(cls, parent, ...); // refcnt = 1, the caller's reference
> efl_unref(o); // refcnt = 0, efl_invalidate(o) is auto-called
>
>
> explicit invalidate:
> o = efl_new(cls, parent, ...); // refcnt = 1, the caller's reference
> efl_invalidate(o); // refcnt = 1
> efl_unref(o); // refcnt = 0, no call to efl_invalidate(o)
>
> canvas usage #1 (basic usage):
> o = efl_new(EFL_UI_BUTTON_CLASS, window, ...); // refcnt = 1, the
> caller's reference
> efl_unref(o); // refcnt = 0, efl_invalidate(o) is auto-called,
> button is removed from window
>
> canvas usage #2 (invalidate window):
> o = efl_new(EFL_UI_BUTTON_CLASS, window, ...); // refcnt = 1, the
> caller's reference
> efl_invalidate(window); // o.refcnt = 1, but efl_invalidate(o) is
> called
> efl_unref(o); // refcnt = 0, no call efl_invalidate(o)
>
> problem with this approach is that most people will forget to
> efl_unref(o), then while it will be invalidated, its memory will leak.
>
> while this can be solved within evas legacy wrapper, the usability is
> not the best. Other systems "solve" this with one of two options:
>
>  - reference transference: efl_adopt_ref(receiver, o), would give your
> reference of "o" to "receiver", which is supposed to release it on its
> "invalidate". That is: keep an Eina_List *adopted, that is
> automatically managed and efl_unref() on its own invalidate.
>
>  - floating references: object starts "unowned" and the first "user"
> should own it, for details:
> https://developer.gnome.org/gobject/stable/gobject-The-Base-
> Object-Type.html#floating-ref
>
> GObject UI and gstreamer use this "floating" and I always disliked it,
> finding it too error prone. It "looks easy" to use, but the situation
> is similar to what we have in EFL and led to this thread... so I'd
> avoid it.
>
> since unlike Gtk and GStreamer we're suggesting elements to be created
> with a parent, as opposed to adding to a parent, this is simpler.
> Compare:
>
> // GTK:
>container = create_container ();
>container_add_child (container, create_child());
>
>// EFL:
>container = efl_new(CONTAINER_CLS, loop, );
>child = efl_new(CHILD_CLS, container, efl_container_pack(container,
> efl_added), efl_adopt_ref(container, efl_added));
>
> this is explicit, clear what's happening. Since "efl_adopt_ref()" was
> written explicitly there, or called afterwards, it's easy to spot you
> don't own the reference anymore. Maybe coccinelle, 

Re: [E-devel] efl_add causing confusion

[Jean-Philippe André]

Hi

On Wed, Jan 10, 2018 at 11:39 PM, Carsten Haitzler 
wrote:

> On Wed, 10 Jan 2018 16:31:33 +0900 Jean-Philippe André 
> said:
>
> > Hi,
> >
> > I created a new event "destruct" in Efl.Object.
> > It's triggered just before removing the callbacks, in the base class
> > destructor, as it's both easier and safer this way. I guess this can be
> > used in bindings too.
>
> shouldn't  it be called AFTER all the callbacks EXCEPT this one have been
> removed? so only callbacks left on the list are these ones? and
> double-check
> that callback removal is the very last thing before the actual object data
> free
> (ie last thing in base class destructor) ?
>

Well that's "almost" the case.
What's left of the object at this point are:
 - name & comment
 - generic data pointers
 - list of event callbacks
 - pending futures to cancel

The futures should probably be canceled before (I didn't spot that until
just now).
It's debatable if the generic data should be cleaned up first (in
particular for the eo values, which trigger efl_unref). Could be cleaned up
first.

But there is no point in cleaning up the list of callbacks as the cleanup
process is a call to mempool free.
I doubt it would be better to send the event after the eo id itself becomes
invalid.


> > As for the rest, I need to think about it more.
> >
> > Best regards,
> >
> >
> > On Wed, Jan 10, 2018 at 1:50 AM, Carsten Haitzler 
> > wrote:
> >
> > > On Tue, 9 Jan 2018 21:03:53 +0900 Jean-Philippe André <
> j...@videolan.org>
> > > said:
> > >
> > > > On Tue, Jan 9, 2018 at 3:17 PM, Carsten Haitzler <
> ras...@rasterman.com>
> > > > wrote:
> > > >
> > > > > On Mon, 8 Jan 2018 18:15:13 +0900 Jean-Philippe André <
> > > j...@videolan.org>
> > > > > said:
> > > > >
> > > > > > Hi,
> > > > > >
> > > > > >
> > > > > > Some of the argumentation here seems to lack a bit of a
> scientific
> > > > > > approach...
> > > > > > So, I looked for all occurrences of efl_add() (see PS for
> > > methodology).
> > > > > >
> > > > > > In all of EFL, without specific filters:
> > > > > >
> > > > > > Overall: 244 unique classes / 1572 occurrences
> > > > > > No parent: 119 unique classes / 472 occurrences
> > > > > > 48% of classes used without parent, accounting for 30% of all
> uses.
> > > > >
> > > > > i argue that the vast majority of those no parent cases should
> have a
> > > > > parent. i
> > > > > actually did a grep -r through efl too looking and almost all the
> cases
> > > > > with a
> > > > > null parent i noted were bugs and should be fixed. :) i
> specifically
> > > noted
> > > > > some
> > > > > of those examples (the efl_anim stuff for example - i filed a bug
> for
> > > it
> > > > > too).
> > > > > so while i didn't do numbers... i did do an analysis. quickly in
> about
> > > 2
> > > > > minutes. for a more detailed analysis. note when i say "loop
> should be
> > > > > parent"
> > > > > i mean loop directly OR some object that ultimately has a parent
> that
> > > is
> > > > > loop
> > > > > at the top of the object tree:
> > > > >
> > > > > src/benchmarks/eo/eo_bench_* (11)
> > > > >   just benchmarking so not real code. (ignore)
> > > > > src/bin/elementary/test_*.c (86)
> > > > >   all are bugs. every one should have loop as parent.
> > > > > src/examples/ecore/ecore_audio*.c: (7)
> > > > >   all are bugs. loop should be parent (loop drives i/o for audio
> and
> > > cb's)
> > > > > src/examples/ecore/ecore_idler_example.c: (1)
> > > > >   bug. loop as parent (commented out though)
> > > > > src/examples/ecore/ecore_poller_*.c (3)
> > > > >   bug. loop should be parent
> > > > > src/examples/ecore/efl_io_*.c (8)
> > > > >   bug. loop should be parent
> > > > > src/examples/ecore/efl_net_*.c (6)
> > > > >   bug. loop should be parent
> > > > > src/examples/eio/eio_sentry.c (1)
> > > > >   unknown
> > > > > src/examples/elementary/efl_ui_*.c (4)
> > > > >   bug. loop should be parent
> > > > > src/examples/elementary/file*.c (9)
> > > > >   bug. loop should be parent
> > > > > src/lib/ecore/ecore.c (1)
> > > > >   bug. singleton vpath object will fail with multiple loops, so
> loop
> > > > > should be
> > > > > parent, but this is unused atm so it doesn't matter yet.
> > > > > src/lib/ecore/efl_loop.c (1)
> > > > >   correct null parent usage
> > > > > src/lib/ecore/efl_model_composite_*.c (2)
> > > > >   relatively sure these are bugs. in fact these files do a lot of:
> > > > >   Efl_Promise *promise = efl_add(EFL_PROMISE_CLASS,
> > > efl_main_loop_get());
> > > > >   and similar assuming only the main loop... which is wrong. they
> > > should
> > > > > get
> > > > >   the correct loop, not always the main loop. lots of examples of
> this
> > > > > through
> > > > >   efl.
> > > > > src/lib/ecore_con/ecore_con_*.c (5)
> > > > >   bug. should use loop as parent
> > > > > src/lib/ector/cairo/ector_cairo_surface.c: (3)
> > > > > src/lib/ector/gl/ector_gl_surface.c: (3)
> > > > > 

Re: [E-devel] efl_add causing confusion

[Gustavo Sverzut Barbieri]

hi all,

jpeg, i think that this "destruct" event should have a counterpart
method, like in GObject they have dispose + finalize, we could have
something like that to allow two clearly distinct phases:
   - one to release references to others (our invalidate/terminate),
announces that everyone keeping a reference should stop using this
object (~weak ref behavior)
   - one to free memory handles and all backing structures, this one
can be called after all references are gone.

https://developer.gnome.org/gobject/stable/howto-gobject-destruction.html

as per parent, as I wrote to some in private, I believe the current
situation is that Eo tries to cope with an unusual system that was
used in Evas_Object, which the objects were added to the canvas, which
held the only reference to it... not by the caller (who created the
visual object).

this is unusual in OOP, but improved usability of evas users, nobody
had to manage object lifecycle, just delete the canvas and they all
would go away. OTOH, to delete the visual object, an specific
evas_object_del() was used, that would remove the canvas ref and
destroy the object.

however, in a more generic OOP system this makes things unusable, as
we're discussing in this thread.

parents: wisely we're advertising almost all objects should have a
parent, like a window, a widget, a loop... maybe it will boils down to
a thread/process.

however, in these cases children is using/knowing about parents,
parents rarely knows/deals with children -- UNLIKE EVAS.

--> That said, IMO parents should NOT have a reference to their children.

if we wish to do cascade deletion, which may be very helpful (delete a
loop, get rid of all users... like done for evas + objects), then we
can dissociate terminate/invalidate from reference, but the last
reference would invalidate:

--> NOTE: using efl_new() instead of efl_add() as I think this should
be renamed for clarity...

basic usage:
o = efl_new(cls, parent, ...); // refcnt = 1, the caller's reference
efl_unref(o); // refcnt = 0, efl_invalidate(o) is auto-called


explicit invalidate:
o = efl_new(cls, parent, ...); // refcnt = 1, the caller's reference
efl_invalidate(o); // refcnt = 1
efl_unref(o); // refcnt = 0, no call to efl_invalidate(o)

canvas usage #1 (basic usage):
o = efl_new(EFL_UI_BUTTON_CLASS, window, ...); // refcnt = 1, the
caller's reference
efl_unref(o); // refcnt = 0, efl_invalidate(o) is auto-called,
button is removed from window

canvas usage #2 (invalidate window):
o = efl_new(EFL_UI_BUTTON_CLASS, window, ...); // refcnt = 1, the
caller's reference
efl_invalidate(window); // o.refcnt = 1, but efl_invalidate(o) is called
efl_unref(o); // refcnt = 0, no call efl_invalidate(o)

problem with this approach is that most people will forget to
efl_unref(o), then while it will be invalidated, its memory will leak.

while this can be solved within evas legacy wrapper, the usability is
not the best. Other systems "solve" this with one of two options:

 - reference transference: efl_adopt_ref(receiver, o), would give your
reference of "o" to "receiver", which is supposed to release it on its
"invalidate". That is: keep an Eina_List *adopted, that is
automatically managed and efl_unref() on its own invalidate.

 - floating references: object starts "unowned" and the first "user"
should own it, for details:
https://developer.gnome.org/gobject/stable/gobject-The-Base-Object-Type.html#floating-ref

GObject UI and gstreamer use this "floating" and I always disliked it,
finding it too error prone. It "looks easy" to use, but the situation
is similar to what we have in EFL and led to this thread... so I'd
avoid it.

since unlike Gtk and GStreamer we're suggesting elements to be created
with a parent, as opposed to adding to a parent, this is simpler.
Compare:

// GTK:
   container = create_container ();
   container_add_child (container, create_child());

   // EFL:
   container = efl_new(CONTAINER_CLS, loop, );
   child = efl_new(CHILD_CLS, container, efl_container_pack(container,
efl_added), efl_adopt_ref(container, efl_added));

this is explicit, clear what's happening. Since "efl_adopt_ref()" was
written explicitly there, or called afterwards, it's easy to spot you
don't own the reference anymore. Maybe coccinelle, coverity and the
likes can told that rule to spot errors.

others may claim efl_adopt_ref() is always the case, in that case
keeping efl_invalidate() UNRELATED to efl_unref() is also helpful:

--> NOTE: using efl_add(), since here parents keep the reference

WRONG/BUG usage:
o = efl_add(cls, parent, ...); // refcnt = 1, the PARENT's reference
efl_unref(o); // refcnt = 0, but parent held the reference

basic usage:
o = efl_add(cls, parent, ...); // refcnt = 1, the PARENT's reference
efl_invalidate(o); // refcnt = 0, since parent will unref based on
invalidate event

cascade delete usage:
o = efl_add(cls, parent, ...); // refcnt = 1, the PARENT's reference
  

Re: [E-devel] efl_add causing confusion

[Carsten Haitzler]

On Wed, 10 Jan 2018 16:31:33 +0900 Jean-Philippe André  said:

> Hi,
> 
> I created a new event "destruct" in Efl.Object.
> It's triggered just before removing the callbacks, in the base class
> destructor, as it's both easier and safer this way. I guess this can be
> used in bindings too.

shouldn't  it be called AFTER all the callbacks EXCEPT this one have been
removed? so only callbacks left on the list are these ones? and double-check
that callback removal is the very last thing before the actual object data free
(ie last thing in base class destructor) ?

> As for the rest, I need to think about it more.
> 
> Best regards,
> 
> 
> On Wed, Jan 10, 2018 at 1:50 AM, Carsten Haitzler 
> wrote:
> 
> > On Tue, 9 Jan 2018 21:03:53 +0900 Jean-Philippe André 
> > said:
> >
> > > On Tue, Jan 9, 2018 at 3:17 PM, Carsten Haitzler 
> > > wrote:
> > >
> > > > On Mon, 8 Jan 2018 18:15:13 +0900 Jean-Philippe André <
> > j...@videolan.org>
> > > > said:
> > > >
> > > > > Hi,
> > > > >
> > > > >
> > > > > Some of the argumentation here seems to lack a bit of a scientific
> > > > > approach...
> > > > > So, I looked for all occurrences of efl_add() (see PS for
> > methodology).
> > > > >
> > > > > In all of EFL, without specific filters:
> > > > >
> > > > > Overall: 244 unique classes / 1572 occurrences
> > > > > No parent: 119 unique classes / 472 occurrences
> > > > > 48% of classes used without parent, accounting for 30% of all uses.
> > > >
> > > > i argue that the vast majority of those no parent cases should have a
> > > > parent. i
> > > > actually did a grep -r through efl too looking and almost all the cases
> > > > with a
> > > > null parent i noted were bugs and should be fixed. :) i specifically
> > noted
> > > > some
> > > > of those examples (the efl_anim stuff for example - i filed a bug for
> > it
> > > > too).
> > > > so while i didn't do numbers... i did do an analysis. quickly in about
> > 2
> > > > minutes. for a more detailed analysis. note when i say "loop should be
> > > > parent"
> > > > i mean loop directly OR some object that ultimately has a parent that
> > is
> > > > loop
> > > > at the top of the object tree:
> > > >
> > > > src/benchmarks/eo/eo_bench_* (11)
> > > >   just benchmarking so not real code. (ignore)
> > > > src/bin/elementary/test_*.c (86)
> > > >   all are bugs. every one should have loop as parent.
> > > > src/examples/ecore/ecore_audio*.c: (7)
> > > >   all are bugs. loop should be parent (loop drives i/o for audio and
> > cb's)
> > > > src/examples/ecore/ecore_idler_example.c: (1)
> > > >   bug. loop as parent (commented out though)
> > > > src/examples/ecore/ecore_poller_*.c (3)
> > > >   bug. loop should be parent
> > > > src/examples/ecore/efl_io_*.c (8)
> > > >   bug. loop should be parent
> > > > src/examples/ecore/efl_net_*.c (6)
> > > >   bug. loop should be parent
> > > > src/examples/eio/eio_sentry.c (1)
> > > >   unknown
> > > > src/examples/elementary/efl_ui_*.c (4)
> > > >   bug. loop should be parent
> > > > src/examples/elementary/file*.c (9)
> > > >   bug. loop should be parent
> > > > src/lib/ecore/ecore.c (1)
> > > >   bug. singleton vpath object will fail with multiple loops, so loop
> > > > should be
> > > > parent, but this is unused atm so it doesn't matter yet.
> > > > src/lib/ecore/efl_loop.c (1)
> > > >   correct null parent usage
> > > > src/lib/ecore/efl_model_composite_*.c (2)
> > > >   relatively sure these are bugs. in fact these files do a lot of:
> > > >   Efl_Promise *promise = efl_add(EFL_PROMISE_CLASS,
> > efl_main_loop_get());
> > > >   and similar assuming only the main loop... which is wrong. they
> > should
> > > > get
> > > >   the correct loop, not always the main loop. lots of examples of this
> > > > through
> > > >   efl.
> > > > src/lib/ecore_con/ecore_con_*.c (5)
> > > >   bug. should use loop as parent
> > > > src/lib/ector/cairo/ector_cairo_surface.c: (3)
> > > > src/lib/ector/gl/ector_gl_surface.c: (3)
> > > > src/lib/ector/software/ector_software_surface.c: (3) (9 total)
> > > >   this is actually ok since ector doesn't use any async events and
> > needs
> > > > no loop
> > > > src/lib/edje/edje_*.c (10)
> > > >   bug. loop should be parent
> > > > src/lib/efl/interfaces/efl_vpath_core.c: (1)
> > > >   bug. vpath needs to be loop driven due to the ability to do async
> > lookups
> > > > src/lib/efl/interfaces/efl_vpath_manager.c (1)
> > > >   bug. same as above
> > > > src/lib/eio/eio_model.c: (1)
> > > >   smells like a bug. eoi would need a loop to drive it
> > > > src/lib/elementary/efl_access.c: (1)
> > > >   bug. access relies on events and async (e.g. dbus) and so must have
> > loop
> > > > src/lib/elementary/efl_ui_*.c: (15)
> > > >   bug. need loop parent
> > > > src/lib/elementary/elm_*.c (5)
> > > >   all look like bugs. need loop parent
> > > > src/lib/evas/canvas/efl_animation*.c (7)
> > > >   bug. need loop parent
> > > > 

Re: [E-devel] efl_add causing confusion

[Jean-Philippe André]

Hi,

I created a new event "destruct" in Efl.Object.
It's triggered just before removing the callbacks, in the base class
destructor, as it's both easier and safer this way. I guess this can be
used in bindings too.

As for the rest, I need to think about it more.

Best regards,


On Wed, Jan 10, 2018 at 1:50 AM, Carsten Haitzler 
wrote:

> On Tue, 9 Jan 2018 21:03:53 +0900 Jean-Philippe André 
> said:
>
> > On Tue, Jan 9, 2018 at 3:17 PM, Carsten Haitzler 
> > wrote:
> >
> > > On Mon, 8 Jan 2018 18:15:13 +0900 Jean-Philippe André <
> j...@videolan.org>
> > > said:
> > >
> > > > Hi,
> > > >
> > > >
> > > > Some of the argumentation here seems to lack a bit of a scientific
> > > > approach...
> > > > So, I looked for all occurrences of efl_add() (see PS for
> methodology).
> > > >
> > > > In all of EFL, without specific filters:
> > > >
> > > > Overall: 244 unique classes / 1572 occurrences
> > > > No parent: 119 unique classes / 472 occurrences
> > > > 48% of classes used without parent, accounting for 30% of all uses.
> > >
> > > i argue that the vast majority of those no parent cases should have a
> > > parent. i
> > > actually did a grep -r through efl too looking and almost all the cases
> > > with a
> > > null parent i noted were bugs and should be fixed. :) i specifically
> noted
> > > some
> > > of those examples (the efl_anim stuff for example - i filed a bug for
> it
> > > too).
> > > so while i didn't do numbers... i did do an analysis. quickly in about
> 2
> > > minutes. for a more detailed analysis. note when i say "loop should be
> > > parent"
> > > i mean loop directly OR some object that ultimately has a parent that
> is
> > > loop
> > > at the top of the object tree:
> > >
> > > src/benchmarks/eo/eo_bench_* (11)
> > >   just benchmarking so not real code. (ignore)
> > > src/bin/elementary/test_*.c (86)
> > >   all are bugs. every one should have loop as parent.
> > > src/examples/ecore/ecore_audio*.c: (7)
> > >   all are bugs. loop should be parent (loop drives i/o for audio and
> cb's)
> > > src/examples/ecore/ecore_idler_example.c: (1)
> > >   bug. loop as parent (commented out though)
> > > src/examples/ecore/ecore_poller_*.c (3)
> > >   bug. loop should be parent
> > > src/examples/ecore/efl_io_*.c (8)
> > >   bug. loop should be parent
> > > src/examples/ecore/efl_net_*.c (6)
> > >   bug. loop should be parent
> > > src/examples/eio/eio_sentry.c (1)
> > >   unknown
> > > src/examples/elementary/efl_ui_*.c (4)
> > >   bug. loop should be parent
> > > src/examples/elementary/file*.c (9)
> > >   bug. loop should be parent
> > > src/lib/ecore/ecore.c (1)
> > >   bug. singleton vpath object will fail with multiple loops, so loop
> > > should be
> > > parent, but this is unused atm so it doesn't matter yet.
> > > src/lib/ecore/efl_loop.c (1)
> > >   correct null parent usage
> > > src/lib/ecore/efl_model_composite_*.c (2)
> > >   relatively sure these are bugs. in fact these files do a lot of:
> > >   Efl_Promise *promise = efl_add(EFL_PROMISE_CLASS,
> efl_main_loop_get());
> > >   and similar assuming only the main loop... which is wrong. they
> should
> > > get
> > >   the correct loop, not always the main loop. lots of examples of this
> > > through
> > >   efl.
> > > src/lib/ecore_con/ecore_con_*.c (5)
> > >   bug. should use loop as parent
> > > src/lib/ector/cairo/ector_cairo_surface.c: (3)
> > > src/lib/ector/gl/ector_gl_surface.c: (3)
> > > src/lib/ector/software/ector_software_surface.c: (3) (9 total)
> > >   this is actually ok since ector doesn't use any async events and
> needs
> > > no loop
> > > src/lib/edje/edje_*.c (10)
> > >   bug. loop should be parent
> > > src/lib/efl/interfaces/efl_vpath_core.c: (1)
> > >   bug. vpath needs to be loop driven due to the ability to do async
> lookups
> > > src/lib/efl/interfaces/efl_vpath_manager.c (1)
> > >   bug. same as above
> > > src/lib/eio/eio_model.c: (1)
> > >   smells like a bug. eoi would need a loop to drive it
> > > src/lib/elementary/efl_access.c: (1)
> > >   bug. access relies on events and async (e.g. dbus) and so must have
> loop
> > > src/lib/elementary/efl_ui_*.c: (15)
> > >   bug. need loop parent
> > > src/lib/elementary/elm_*.c (5)
> > >   all look like bugs. need loop parent
> > > src/lib/evas/canvas/efl_animation*.c (7)
> > >   bug. need loop parent
> > > src/lib/evas/canvas/efl_canvas_vg.c: (1)
> > >   bug. need loop parent
> > >
> > > src/lib/evas/canvas/evas_main.c: (1)
> > >   bug. need loop parent
> > > src/lib/evas/canvas/evas_vg_node.c: (1)
> > >   bug. vg nodes should have a parent object - always
> > > src/lib/evas/gesture/efl_gesture_manager.c: (1)
> > >   bug. need loop parent
> > > src/modules/evas/engines/gl_generic/evas_engine.c (3)
> > > src/modules/evas/engines/software_generic/evas_engine.c: (3) (6 total)
> > >   correct. ector objects
> > > src/tests/ecore/ecore_test_ecore_audio.c (16)
> > >   bug. need loop parent
> > > 

Re: [E-devel] efl_add causing confusion

[Carsten Haitzler]

On Tue, 9 Jan 2018 21:03:53 +0900 Jean-Philippe André  said:

> On Tue, Jan 9, 2018 at 3:17 PM, Carsten Haitzler 
> wrote:
> 
> > On Mon, 8 Jan 2018 18:15:13 +0900 Jean-Philippe André 
> > said:
> >
> > > Hi,
> > >
> > >
> > > Some of the argumentation here seems to lack a bit of a scientific
> > > approach...
> > > So, I looked for all occurrences of efl_add() (see PS for methodology).
> > >
> > > In all of EFL, without specific filters:
> > >
> > > Overall: 244 unique classes / 1572 occurrences
> > > No parent: 119 unique classes / 472 occurrences
> > > 48% of classes used without parent, accounting for 30% of all uses.
> >
> > i argue that the vast majority of those no parent cases should have a
> > parent. i
> > actually did a grep -r through efl too looking and almost all the cases
> > with a
> > null parent i noted were bugs and should be fixed. :) i specifically noted
> > some
> > of those examples (the efl_anim stuff for example - i filed a bug for it
> > too).
> > so while i didn't do numbers... i did do an analysis. quickly in about 2
> > minutes. for a more detailed analysis. note when i say "loop should be
> > parent"
> > i mean loop directly OR some object that ultimately has a parent that is
> > loop
> > at the top of the object tree:
> >
> > src/benchmarks/eo/eo_bench_* (11)
> >   just benchmarking so not real code. (ignore)
> > src/bin/elementary/test_*.c (86)
> >   all are bugs. every one should have loop as parent.
> > src/examples/ecore/ecore_audio*.c: (7)
> >   all are bugs. loop should be parent (loop drives i/o for audio and cb's)
> > src/examples/ecore/ecore_idler_example.c: (1)
> >   bug. loop as parent (commented out though)
> > src/examples/ecore/ecore_poller_*.c (3)
> >   bug. loop should be parent
> > src/examples/ecore/efl_io_*.c (8)
> >   bug. loop should be parent
> > src/examples/ecore/efl_net_*.c (6)
> >   bug. loop should be parent
> > src/examples/eio/eio_sentry.c (1)
> >   unknown
> > src/examples/elementary/efl_ui_*.c (4)
> >   bug. loop should be parent
> > src/examples/elementary/file*.c (9)
> >   bug. loop should be parent
> > src/lib/ecore/ecore.c (1)
> >   bug. singleton vpath object will fail with multiple loops, so loop
> > should be
> > parent, but this is unused atm so it doesn't matter yet.
> > src/lib/ecore/efl_loop.c (1)
> >   correct null parent usage
> > src/lib/ecore/efl_model_composite_*.c (2)
> >   relatively sure these are bugs. in fact these files do a lot of:
> >   Efl_Promise *promise = efl_add(EFL_PROMISE_CLASS, efl_main_loop_get());
> >   and similar assuming only the main loop... which is wrong. they should
> > get
> >   the correct loop, not always the main loop. lots of examples of this
> > through
> >   efl.
> > src/lib/ecore_con/ecore_con_*.c (5)
> >   bug. should use loop as parent
> > src/lib/ector/cairo/ector_cairo_surface.c: (3)
> > src/lib/ector/gl/ector_gl_surface.c: (3)
> > src/lib/ector/software/ector_software_surface.c: (3) (9 total)
> >   this is actually ok since ector doesn't use any async events and needs
> > no loop
> > src/lib/edje/edje_*.c (10)
> >   bug. loop should be parent
> > src/lib/efl/interfaces/efl_vpath_core.c: (1)
> >   bug. vpath needs to be loop driven due to the ability to do async lookups
> > src/lib/efl/interfaces/efl_vpath_manager.c (1)
> >   bug. same as above
> > src/lib/eio/eio_model.c: (1)
> >   smells like a bug. eoi would need a loop to drive it
> > src/lib/elementary/efl_access.c: (1)
> >   bug. access relies on events and async (e.g. dbus) and so must have loop
> > src/lib/elementary/efl_ui_*.c: (15)
> >   bug. need loop parent
> > src/lib/elementary/elm_*.c (5)
> >   all look like bugs. need loop parent
> > src/lib/evas/canvas/efl_animation*.c (7)
> >   bug. need loop parent
> > src/lib/evas/canvas/efl_canvas_vg.c: (1)
> >   bug. need loop parent
> >
> > src/lib/evas/canvas/evas_main.c: (1)
> >   bug. need loop parent
> > src/lib/evas/canvas/evas_vg_node.c: (1)
> >   bug. vg nodes should have a parent object - always
> > src/lib/evas/gesture/efl_gesture_manager.c: (1)
> >   bug. need loop parent
> > src/modules/evas/engines/gl_generic/evas_engine.c (3)
> > src/modules/evas/engines/software_generic/evas_engine.c: (3) (6 total)
> >   correct. ector objects
> > src/tests/ecore/ecore_test_ecore_audio.c (16)
> >   bug. need loop parent
> > src/tests/ecore/ecore_test_promise2.c: (2)
> >   bug. need loop parent
> > src/tests/ecore_con/ecore_con_test_efl_net_ip_address.c: (14)
> >   bug. need loop parent
> > src/tests/efl/efl_test_model_*.c: (5)
> >   bug. need loop parent
> > src/tests/efl_js/benchmark_object_impl.cc: (1)
> >   artificial benchmarking (so ignore)
> > src/tests/efl_mono/libefl_mono_native_test.c: (1)
> >   artificial benchmarking (ignore)
> > src/tests/eina_cxx/eina_cxx_test_*.cc (54)
> >   artificial testing of basic input/output through eo api
> > src/tests/eio/eio_test_sentry.c: (20)
> >   bug. should have parent loop
> > 

Re: [E-devel] efl_add causing confusion

[Jean-Philippe André]

On Tue, Jan 9, 2018 at 3:17 PM, Carsten Haitzler 
wrote:

> On Mon, 8 Jan 2018 18:15:13 +0900 Jean-Philippe André 
> said:
>
> > Hi,
> >
> >
> > Some of the argumentation here seems to lack a bit of a scientific
> > approach...
> > So, I looked for all occurrences of efl_add() (see PS for methodology).
> >
> > In all of EFL, without specific filters:
> >
> > Overall: 244 unique classes / 1572 occurrences
> > No parent: 119 unique classes / 472 occurrences
> > 48% of classes used without parent, accounting for 30% of all uses.
>
> i argue that the vast majority of those no parent cases should have a
> parent. i
> actually did a grep -r through efl too looking and almost all the cases
> with a
> null parent i noted were bugs and should be fixed. :) i specifically noted
> some
> of those examples (the efl_anim stuff for example - i filed a bug for it
> too).
> so while i didn't do numbers... i did do an analysis. quickly in about 2
> minutes. for a more detailed analysis. note when i say "loop should be
> parent"
> i mean loop directly OR some object that ultimately has a parent that is
> loop
> at the top of the object tree:
>
> src/benchmarks/eo/eo_bench_* (11)
>   just benchmarking so not real code. (ignore)
> src/bin/elementary/test_*.c (86)
>   all are bugs. every one should have loop as parent.
> src/examples/ecore/ecore_audio*.c: (7)
>   all are bugs. loop should be parent (loop drives i/o for audio and cb's)
> src/examples/ecore/ecore_idler_example.c: (1)
>   bug. loop as parent (commented out though)
> src/examples/ecore/ecore_poller_*.c (3)
>   bug. loop should be parent
> src/examples/ecore/efl_io_*.c (8)
>   bug. loop should be parent
> src/examples/ecore/efl_net_*.c (6)
>   bug. loop should be parent
> src/examples/eio/eio_sentry.c (1)
>   unknown
> src/examples/elementary/efl_ui_*.c (4)
>   bug. loop should be parent
> src/examples/elementary/file*.c (9)
>   bug. loop should be parent
> src/lib/ecore/ecore.c (1)
>   bug. singleton vpath object will fail with multiple loops, so loop
> should be
> parent, but this is unused atm so it doesn't matter yet.
> src/lib/ecore/efl_loop.c (1)
>   correct null parent usage
> src/lib/ecore/efl_model_composite_*.c (2)
>   relatively sure these are bugs. in fact these files do a lot of:
>   Efl_Promise *promise = efl_add(EFL_PROMISE_CLASS, efl_main_loop_get());
>   and similar assuming only the main loop... which is wrong. they should
> get
>   the correct loop, not always the main loop. lots of examples of this
> through
>   efl.
> src/lib/ecore_con/ecore_con_*.c (5)
>   bug. should use loop as parent
> src/lib/ector/cairo/ector_cairo_surface.c: (3)
> src/lib/ector/gl/ector_gl_surface.c: (3)
> src/lib/ector/software/ector_software_surface.c: (3) (9 total)
>   this is actually ok since ector doesn't use any async events and needs
> no loop
> src/lib/edje/edje_*.c (10)
>   bug. loop should be parent
> src/lib/efl/interfaces/efl_vpath_core.c: (1)
>   bug. vpath needs to be loop driven due to the ability to do async lookups
> src/lib/efl/interfaces/efl_vpath_manager.c (1)
>   bug. same as above
> src/lib/eio/eio_model.c: (1)
>   smells like a bug. eoi would need a loop to drive it
> src/lib/elementary/efl_access.c: (1)
>   bug. access relies on events and async (e.g. dbus) and so must have loop
> src/lib/elementary/efl_ui_*.c: (15)
>   bug. need loop parent
> src/lib/elementary/elm_*.c (5)
>   all look like bugs. need loop parent
> src/lib/evas/canvas/efl_animation*.c (7)
>   bug. need loop parent
> src/lib/evas/canvas/efl_canvas_vg.c: (1)
>   bug. need loop parent
>
> src/lib/evas/canvas/evas_main.c: (1)
>   bug. need loop parent
> src/lib/evas/canvas/evas_vg_node.c: (1)
>   bug. vg nodes should have a parent object - always
> src/lib/evas/gesture/efl_gesture_manager.c: (1)
>   bug. need loop parent
> src/modules/evas/engines/gl_generic/evas_engine.c (3)
> src/modules/evas/engines/software_generic/evas_engine.c: (3) (6 total)
>   correct. ector objects
> src/tests/ecore/ecore_test_ecore_audio.c (16)
>   bug. need loop parent
> src/tests/ecore/ecore_test_promise2.c: (2)
>   bug. need loop parent
> src/tests/ecore_con/ecore_con_test_efl_net_ip_address.c: (14)
>   bug. need loop parent
> src/tests/efl/efl_test_model_*.c: (5)
>   bug. need loop parent
> src/tests/efl_js/benchmark_object_impl.cc: (1)
>   artificial benchmarking (so ignore)
> src/tests/efl_mono/libefl_mono_native_test.c: (1)
>   artificial benchmarking (ignore)
> src/tests/eina_cxx/eina_cxx_test_*.cc (54)
>   artificial testing of basic input/output through eo api
> src/tests/eio/eio_test_sentry.c: (20)
>   bug. should have parent loop
> src/tests/eldbus/eldbus_test_*.c: (3)
>   bug. should have loop parent
> src/tests/elementary/elm_test_*.c: (31)
>   bug. should have loop parent
> src/tests/eo/access/access_main.c: (1)
>   bug. should have loop parent
> src/tests/eo/children/children_main.c: (1)
>   artificial eo testing - ignore
> 

Re: [E-devel] efl_add causing confusion

[Carsten Haitzler]

On Mon, 8 Jan 2018 18:15:13 +0900 Jean-Philippe André  said:

> Hi,
> 
> 
> Some of the argumentation here seems to lack a bit of a scientific
> approach...
> So, I looked for all occurrences of efl_add() (see PS for methodology).
> 
> In all of EFL, without specific filters:
> 
> Overall: 244 unique classes / 1572 occurrences
> No parent: 119 unique classes / 472 occurrences
> 48% of classes used without parent, accounting for 30% of all uses.

i argue that the vast majority of those no parent cases should have a parent. i
actually did a grep -r through efl too looking and almost all the cases with a
null parent i noted were bugs and should be fixed. :) i specifically noted some
of those examples (the efl_anim stuff for example - i filed a bug for it too).
so while i didn't do numbers... i did do an analysis. quickly in about 2
minutes. for a more detailed analysis. note when i say "loop should be parent"
i mean loop directly OR some object that ultimately has a parent that is loop
at the top of the object tree:

src/benchmarks/eo/eo_bench_* (11)
  just benchmarking so not real code. (ignore)
src/bin/elementary/test_*.c (86)
  all are bugs. every one should have loop as parent.
src/examples/ecore/ecore_audio*.c: (7)
  all are bugs. loop should be parent (loop drives i/o for audio and cb's)
src/examples/ecore/ecore_idler_example.c: (1)
  bug. loop as parent (commented out though)
src/examples/ecore/ecore_poller_*.c (3)
  bug. loop should be parent
src/examples/ecore/efl_io_*.c (8)
  bug. loop should be parent
src/examples/ecore/efl_net_*.c (6)
  bug. loop should be parent
src/examples/eio/eio_sentry.c (1)
  unknown
src/examples/elementary/efl_ui_*.c (4)
  bug. loop should be parent
src/examples/elementary/file*.c (9)
  bug. loop should be parent
src/lib/ecore/ecore.c (1)
  bug. singleton vpath object will fail with multiple loops, so loop should be
parent, but this is unused atm so it doesn't matter yet.
src/lib/ecore/efl_loop.c (1)
  correct null parent usage
src/lib/ecore/efl_model_composite_*.c (2)
  relatively sure these are bugs. in fact these files do a lot of:
  Efl_Promise *promise = efl_add(EFL_PROMISE_CLASS, efl_main_loop_get());
  and similar assuming only the main loop... which is wrong. they should get
  the correct loop, not always the main loop. lots of examples of this through
  efl.
src/lib/ecore_con/ecore_con_*.c (5)
  bug. should use loop as parent
src/lib/ector/cairo/ector_cairo_surface.c: (3)
src/lib/ector/gl/ector_gl_surface.c: (3)
src/lib/ector/software/ector_software_surface.c: (3) (9 total)
  this is actually ok since ector doesn't use any async events and needs no loop
src/lib/edje/edje_*.c (10)
  bug. loop should be parent
src/lib/efl/interfaces/efl_vpath_core.c: (1)
  bug. vpath needs to be loop driven due to the ability to do async lookups
src/lib/efl/interfaces/efl_vpath_manager.c (1)
  bug. same as above
src/lib/eio/eio_model.c: (1)
  smells like a bug. eoi would need a loop to drive it
src/lib/elementary/efl_access.c: (1)
  bug. access relies on events and async (e.g. dbus) and so must have loop
src/lib/elementary/efl_ui_*.c: (15)
  bug. need loop parent
src/lib/elementary/elm_*.c (5)
  all look like bugs. need loop parent
src/lib/evas/canvas/efl_animation*.c (7)
  bug. need loop parent
src/lib/evas/canvas/efl_canvas_vg.c: (1)
  bug. need loop parent

src/lib/evas/canvas/evas_main.c: (1)
  bug. need loop parent
src/lib/evas/canvas/evas_vg_node.c: (1)
  bug. vg nodes should have a parent object - always
src/lib/evas/gesture/efl_gesture_manager.c: (1)
  bug. need loop parent
src/modules/evas/engines/gl_generic/evas_engine.c (3)
src/modules/evas/engines/software_generic/evas_engine.c: (3) (6 total)
  correct. ector objects
src/tests/ecore/ecore_test_ecore_audio.c (16)
  bug. need loop parent
src/tests/ecore/ecore_test_promise2.c: (2)
  bug. need loop parent
src/tests/ecore_con/ecore_con_test_efl_net_ip_address.c: (14)
  bug. need loop parent
src/tests/efl/efl_test_model_*.c: (5)
  bug. need loop parent
src/tests/efl_js/benchmark_object_impl.cc: (1)
  artificial benchmarking (so ignore)
src/tests/efl_mono/libefl_mono_native_test.c: (1)
  artificial benchmarking (ignore)
src/tests/eina_cxx/eina_cxx_test_*.cc (54)
  artificial testing of basic input/output through eo api
src/tests/eio/eio_test_sentry.c: (20)
  bug. should have parent loop
src/tests/eldbus/eldbus_test_*.c: (3)
  bug. should have loop parent
src/tests/elementary/elm_test_*.c: (31)
  bug. should have loop parent
src/tests/eo/access/access_main.c: (1)
  bug. should have loop parent
src/tests/eo/children/children_main.c: (1)
  artificial eo testing - ignore
src/tests/eo/composite_objects/composite_objects_main.c: (1)
  artificial eo testing - ignore
src/tests/eo/constructors/constructors_main.c: (1)
  artificial testing - ignore
src/tests/eo/function_overrides/function_overrides_main.c: (4)
  artificial testing - ignore
src/tests/eo/interface/interface_main.c: (1)
  artificial testing - ignore

Re: [E-devel] efl_add causing confusion

[Jean-Philippe André]

On Tue, Jan 9, 2018 at 7:30 AM, Cedric Bail <ced...@ddlm.me> wrote:

> >  Original Message 
> > Subject: Re: [E-devel] efl_add causing confusion
> > Local Time: January 8, 2018 1:15 AM
> > UTC Time: January 8, 2018 9:15 AM
> > From: j...@videolan.org
> > To: Enlightenment developer list <enlightenment-de...@lists.sou
> rceforge.net>
>
> 
>
> > - Those numbers differ from the values given in the below email.
> > While I understand where this "maybe 1 or 2% of all objects [are created
> > without a parent]" comes from, it's not based on facts.
> > @noparent makes sense. A NULL check in efl_add would then help.
> > A different API then wouldn't hurt either IMHO (maybe efl_new?
> > efl_add_single? efl_create? or whatever -- efl_add then can NOT be called
> > with NULL).
>
> I am not sure of the semantic you expect here. Could you clarify ?
>

Ah sorry it wasn't clear:

#define efl_add(CLASS, parent, ...)
#define efl_new(CLASS, ...)

efl_add checks that parent != NULL and have EINA_ARG_NONNULL
efl_new simply cannot pass a parent (unless of course you call
efl_parent_set(efl_added))
the internal code can be the same (the null check being added in eo.c, if
we want it -- it relies on @noparent)


> > - The argumentation in this email chain again leads nowhere. The original
> > confusion remains mostly unaddressed.
> > Felipe mentioned that ownership and references are mixed. So the proposal
> > for efl_release (or detach, close, invalidate, ...) makes sense to me.
> > In fact we have issues sometimes with efl_del as inside a destructor we
> > already lost our parent (thus all efl_provider_find and related calls
> will
> > fail).
> > Also, quite a few times I've also been looking for a "deleted" event that
> > would happen after destruction, and not before.
> > I had to introduce a very ugly API very badly called "allow_parent_unref"
> > in efl.object because some objects need a parent but they should be
> > unref'ed by someone else (efl_part objects but not only).
> >
> > So, I think it would make sense to investigate efl_terminate, and
> > evas_object_del would just call efl_terminate, hiding the object or
> > starting destruction, then let the parent (either evas or another canvas
> > object) do the final unref/unparent and destroying everything that's
> left.
>
> I like this direction. It might even allow a way to not have the double
> step destruction we have today.


Do you mean evas object manual free?
evas_object_del -> efl_invalidate (hides and calls existing destructor?)
manual_free -> unref and final cleanup?


> Also it allow to solve the problem we have of referencing parent during
> destruction and weird refcounting for parenting.
>

Referencing the parent? You mean using pd->parent or something like that?
Parent is NULL inside efl_destructor...

>
> If we enforce that path, then during destructor it must be expected that
> the parent is NULL. When setting the parent to NULL, it will trigger an
> invalidate of the object (If the object is not invalidated already). This
> means that efl_del get simplified to just parent set to NULL followed by
> unref. Making it a direct symetric operation to efl_add_ref.
>

Parent is already NULL inside destructor. Which I think can in fact be
problematic sometimes (since efl_provider_find fails).
From your sentence it would seem that the object has two refs? (parent ref
+ user ref)
I'm not thinking of adding a ref to the object. IMO efl_add_ref is only for
bindings, the ref dies after the scope ends (in C++ anyway).

This isn't a fully formed idea yet (in my mind at least)

> Anyway I think experiments only can tell us what's best. I see 3 items:
> >
> > - @noparent tag

> - efl_new (= efl_add without a parent -- requires @noparent if we want
> > strong NULL check)

> - efl_invalidate / efl_terminate (I prefer efl_close :P)
>
> I am now prefering efl_invalidate as efl_terminate is actually used by our
> lifecycle function and efl_close remind me of file.


Sure.

-- 
Jean-Philippe André
--
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

Re: [E-devel] efl_add causing confusion

[Cedric Bail]

>  Original Message 
> Subject: Re: [E-devel] efl_add causing confusion
> Local Time: January 8, 2018 1:15 AM
> UTC Time: January 8, 2018 9:15 AM
> From: j...@videolan.org
> To: Enlightenment developer list <enlightenment-devel@lists.sourceforge.net>



> - Those numbers differ from the values given in the below email.
> While I understand where this "maybe 1 or 2% of all objects [are created
> without a parent]" comes from, it's not based on facts.
> @noparent makes sense. A NULL check in efl_add would then help.
> A different API then wouldn't hurt either IMHO (maybe efl_new?
> efl_add_single? efl_create? or whatever -- efl_add then can NOT be called
> with NULL).

I am not sure of the semantic you expect here. Could you clarify ?

> - The argumentation in this email chain again leads nowhere. The original
> confusion remains mostly unaddressed.
> Felipe mentioned that ownership and references are mixed. So the proposal
> for efl_release (or detach, close, invalidate, ...) makes sense to me.
> In fact we have issues sometimes with efl_del as inside a destructor we
> already lost our parent (thus all efl_provider_find and related calls will
> fail).
> Also, quite a few times I've also been looking for a "deleted" event that
> would happen after destruction, and not before.
> I had to introduce a very ugly API very badly called "allow_parent_unref"
> in efl.object because some objects need a parent but they should be
> unref'ed by someone else (efl_part objects but not only).
>
> So, I think it would make sense to investigate efl_terminate, and
> evas_object_del would just call efl_terminate, hiding the object or
> starting destruction, then let the parent (either evas or another canvas
> object) do the final unref/unparent and destroying everything that's left.

I like this direction. It might even allow a way to not have the double step 
destruction we have today. Also it allow to solve the problem we have of 
referencing parent during destruction and weird refcounting for parenting.

If we enforce that path, then during destructor it must be expected that the 
parent is NULL. When setting the parent to NULL, it will trigger an invalidate 
of the object (If the object is not invalidated already). This means that 
efl_del get simplified to just parent set to NULL followed by unref. Making it 
a direct symetric operation to efl_add_ref.

> Anyway I think experiments only can tell us what's best. I see 3 items:
>
> - @noparent tag
> - efl_new (= efl_add without a parent -- requires @noparent if we want
> strong NULL check)
> - efl_invalidate / efl_terminate (I prefer efl_close :P)

I am now prefering efl_invalidate as efl_terminate is actually used by our 
lifecycle function and efl_close remind me of file.

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

Re: [E-devel] efl_add causing confusion

[Jean-Philippe André]

Hi,


Some of the argumentation here seems to lack a bit of a scientific
approach...
So, I looked for all occurrences of efl_add() (see PS for methodology).

In all of EFL, without specific filters:

Overall: 244 unique classes / 1572 occurrences
No parent: 119 unique classes / 472 occurrences
48% of classes used without parent, accounting for 30% of all uses.


In all of EFL, without EFL_UI_WIN_CLASS or ECORE_XXX:

Overall: 226 unique classes / 1438 occurrences
No parent: 89 unique classes / 358 occurrences
42% of classes used without parent, accounting for 27% of all uses.


Only in src/examples, without EFL_UI_WIN_CLASS or ECORE:

Overall: 44 unique classes / 474 occurrences
No parent: 12 unique classes / 27 occurrences
27% of classes used without parent, accounting for 5.7% of all uses.
(note: only one example with EFL_UI_WIN_CLASS)


In src/bin/elementary, without EFL_UI_WIN_CLASS or ECORE:

Overall: 49 unique classes / 303 occurrences
No parent: 14 unique classes / 54 occurrences
29% of classes used without parent, accounting for 18% of all uses.
(note: those classes are animation and interpolator)
(note2: 34 occurences of WIN class - no parent)


Conclusions:

0. This analysis is flawed by design due to a lack of real usage.

1. Those numbers differ from the values given in the below email.
While I understand where this "maybe 1 or 2% of all objects [are created
without a parent]" comes from, it's not based on facts.
@noparent makes sense. A NULL check in efl_add would then help.
A different API then wouldn't hurt either IMHO (maybe efl_new?
efl_add_single? efl_create? or whatever -- efl_add then can NOT be called
with NULL).

2. The argumentation in this email chain again leads nowhere. The original
confusion remains mostly unaddressed.
Felipe mentioned that ownership and references are mixed. So the proposal
for efl_release (or detach, close, invalidate, ...) makes sense to me.
In fact we have issues sometimes with efl_del as inside a destructor we
already lost our parent (thus all efl_provider_find and related calls will
fail).
Also, quite a few times I've also been looking for a "deleted" event that
would happen after destruction, and not before.
I had to introduce a very ugly API very badly called "allow_parent_unref"
in efl.object because some objects need a parent but they should be
unref'ed by someone else (efl_part objects but not only).

So, I think it would make sense to investigate efl_terminate, and
evas_object_del would just call efl_terminate, hiding the object or
starting destruction, then let the parent (either evas or another canvas
object) do the final unref/unparent and destroying everything that's left.


Last note: @owned is rarely used with objects:
- Efl.Net.Ip_Address create (factory pattern, replaces efl_add, but I think
if there's a parent then we shouldn't tag as @owned)
- efl_duplicate (creates a new object like efl_add)


Anyway I think experiments only can tell us what's best. I see 3 items:
- @noparent tag
- efl_new (= efl_add without a parent -- requires @noparent if we want
strong NULL check)
- efl_invalidate / efl_terminate (I prefer efl_close :P)


Best regards,


On Fri, Jan 5, 2018 at 2:43 PM, Carsten Haitzler 
wrote:

> On Thu, 04 Jan 2018 17:18:35 + Andrew Williams 
> said:
>
> > Hi,
> >
> > I don't think I can do this any longer. Any comment I make about
> usability
> > is met with
> >
> > "it's easy to understand if you know the internal state/functioning of
> the
> > object".
>
> believe what you want... i described it in a single if statement. it
> requires
> no knowledge of internal state. it requires knowing just this:
>
> if (parent) { parent now responsible for unreffing }
> else { you (the caller) are responsible for unreffing when done }
>
> requires nothing more than that. why do you claim you need to know internal
> state? the above if has nothing to do with internal state. it has TOTALLy
> to do
> with externally provided params. it's the same logic as (assuming there is
> an
> efl_child_add that has a parent param that must be non-null, and efl_add
> now
> drops parent param as it's assumed to be NULL):
>
> if (used efl_child_add) { parent now responsible for unreffing }
> else (used efl_add) { you (the caller) are responsible for unreffing when
> done }
>
> both are conditions/data provided by the caller. one is a choice by
> parameter
> and the other is a choice by function call name.
>
> > My premise is still
> >
> > "with efl_add sometimes returning ownership and other times not this is
> > confusing for the developer"
>
> totally based on input provided by the caller. examples:
>
> fopen("filename", "r");
> vs
> fopen("filename", "w");
>
> with one i can only call fread() on the result anxd the other i can only
> call
> fwrite(). by your logic this is highly confusing to developers and libc
> should
> have:
>
> fopen_read("filename");
> +
> fopen_write("filename");
>
> because developers would 

Re: [E-devel] efl_add causing confusion

[Carsten Haitzler]

On Thu, 04 Jan 2018 17:18:35 + Andrew Williams  said:

> Hi,
> 
> I don't think I can do this any longer. Any comment I make about usability
> is met with
> 
> "it's easy to understand if you know the internal state/functioning of the
> object".

believe what you want... i described it in a single if statement. it requires
no knowledge of internal state. it requires knowing just this:

if (parent) { parent now responsible for unreffing }
else { you (the caller) are responsible for unreffing when done }

requires nothing more than that. why do you claim you need to know internal
state? the above if has nothing to do with internal state. it has TOTALLy to do
with externally provided params. it's the same logic as (assuming there is an
efl_child_add that has a parent param that must be non-null, and efl_add now
drops parent param as it's assumed to be NULL):

if (used efl_child_add) { parent now responsible for unreffing }
else (used efl_add) { you (the caller) are responsible for unreffing when done }

both are conditions/data provided by the caller. one is a choice by parameter
and the other is a choice by function call name.

> My premise is still
> 
> "with efl_add sometimes returning ownership and other times not this is
> confusing for the developer"

totally based on input provided by the caller. examples:

fopen("filename", "r");
vs
fopen("filename", "w");

with one i can only call fread() on the result anxd the other i can only call
fwrite(). by your logic this is highly confusing to developers and libc should
have:

fopen_read("filename");
+
fopen_write("filename");

because developers would otherwise be confused.

> I wanted for us to present something that is clear and simple without
> needing to know internal state or reading the EFL source code.
> It seems I will not succeed.

my argument is that more constructors make for something LESS simple. while it
removes the parameter you do not like, it means you have to choose which
constructor to use based on situation and know about both.

also i argue that 95%+ of uses will be with pent not allowed to be NULL.
efl.net is reliant on a loop to drive it. all of the ui is too. the loop and
its related constructs are too. ecore_audio is too. well it is reliant on the
main loop to drive it but it may not actually technically rely on parent object
(yet, but it should). eio is reliant on parent objects too. ledbus too. 

in fact a quick look at out current efl interfaces means EVERY SINGLE object we
have requires a parent EXCEPT the loop object ... and that is created for you
(and will be in future with threads and their loops). so in fact using a
non-NULL parent is in fact invalid in 100% of efl use cases. this is why i keep
pushing back strongly. your argument for confusion is actually covering 0 use
cases (and the only reason objects can be created with NULL parents now is that
we aren't policing or enforcing parents at this point and we SHOULD be. we
SHOULD have a specific "@noparent" tag for classes that indicate an object is
allowed to have no parent - it'll be by far the exception and not the rule...
by exception i mean maybe 1 or 2% of all objects). does this incredibly rare
case justify multiplying the number of constructors we have and making a longer
winded one (efl_child_add) which will be actually 99% of use cases?

i think we're at a position where we see 2 totally different things. i see an
efl api where parent is basically always used.  you don't. you believe it'll be
"random" or basically sometimes NULL, sometimes not.

as i already mentioned in another mail. we're missing the policing of this. we
should actually have eo require a non-null parent for essentially every object
we have with some exceptions (like loop objects... which will be created for
you buy efl itself anyway - but in this case the check, when it finally exists,
should be disabled).

at this point in time, it'd be bad to put in a forced non-null check because
we're still mis-using efl's api a lot and need to clean this up so it's a very
small number of instances to fix before enforcing. we could add ERR logs to
non-null parents and add a lot of noise at this point... but IMHO this is what
we should do.

now what do we do if you, by accident, pass in a null parent on add? as long as
it's not one of the "i allow null parents" object types (which frankly will be
a handful in the end), we probably should not even begin construction and error
log and return NULL as the object as it's invalid. but first we need the
ability to flag objects as allowing null parents and then begin cleaning up our
code to match.

> Andy
> 
> On Thu, 4 Jan 2018 at 06:32 Carsten Haitzler  wrote:
> 
> > On Wed, 03 Jan 2018 09:43:16 + Andrew Williams 
> > said:
> >
> > > Hi Cedric,
> > >
> > > I think I agree, if we can tidy the definitions then everything should
> > get
> > > clearer.
> > >
> > > efl_add_ref being needed for 

Re: [E-devel] efl_add causing confusion

[Cedric Bail]

>  Original Message 
> Subject: Re: [E-devel] efl_add causing confusion
> Local Time: January 4, 2018 10:21 AM
> UTC Time: January 4, 2018 6:21 PM
> From: barbi...@gmail.com
> To: Enlightenment developer list <enlightenment-devel@lists.sourceforge.net>
>
> On Thu, Jan 4, 2018 at 3:12 PM, Andrew Williams a...@andywilliams.me wrote:
>
>> Hi,
>> I would avoid the name efl_release if it is not to do with object lifecycle
>> as that is the word ObjectiveC uses in their retain/release pair for memory
>> management.
>
> +1 I also find "release" to be something related to object lifecycle.
>
> maybe "terminate"? as in:

I talked with Andrew a bit and another possibility would be "invalidate". 
"terminate" sounds a bit lifecycle, but not to much. So either of them sound 
good to me.
--
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

Re: [E-devel] efl_add causing confusion

[Gustavo Sverzut Barbieri]

On Thu, Jan 4, 2018 at 3:12 PM, Andrew Williams  wrote:
> Hi,
>
> I would avoid the name efl_release if it is not to do with object lifecycle
> as that is the word ObjectiveC uses in their retain/release pair for memory
> management.

+1 I also find "release" to be something related to object lifecycle.

maybe "terminate"? as in:

 - network connection: terminate, clear it should be closed and all
 - gui element: terminate, clear it should be hidden and all


-- 
Gustavo Sverzut Barbieri
--
Mobile: +55 (16) 99354-9890

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

Re: [E-devel] efl_add causing confusion

[Andrew Williams]

Hi,

I don't think I can do this any longer. Any comment I make about usability
is met with

"it's easy to understand if you know the internal state/functioning of the
object".

My premise is still

"with efl_add sometimes returning ownership and other times not this is
confusing for the developer"

I wanted for us to present something that is clear and simple without
needing to know internal state or reading the EFL source code.
It seems I will not succeed.

Andy

On Thu, 4 Jan 2018 at 06:32 Carsten Haitzler  wrote:

> On Wed, 03 Jan 2018 09:43:16 + Andrew Williams 
> said:
>
> > Hi Cedric,
> >
> > I think I agree, if we can tidy the definitions then everything should
> get
> > clearer.
> >
> > efl_add_ref being needed for bindings implies that our definition of
> > efl_add was not clean enough in the first place.
>
> we were very clear on that. for bindings it's needed (or if you want to
> write
> code in that style) but it's highly inconvenient and we've been over that.
>
> > If efl_del is "hide and unparent" then maybe what we really need is
> > efl_gfx_hide and efl_parent_unset - but I don't see why we need to
> unparent
> > anyhow...
>
> well if it's the last ref... the unparenting cleanly removes from the
> parent
> AND removes the last ref as a result.
>
> BUT i kind of agree with you here. the unparenting is proving to be a major
> pain in the rear. by that i mean the unparenting happens and then parent is
> NULL and THEN as a result of this reference goes to 0 and destructors get
> called. when the destructors are called, the parent is already NULL and
> this
> has proven an "odd" case i've been dealing with the efl loop work...
> because on
> destruction the object doesn't know what loop it belonged to anymore...
> yes -
> you have to handle parent_set to NULL then but it means this has to do some
> kind of destruction of loop bound resources... :(
>
> IMHO when the destructors are called the object should still have a parent
> and
> be removed from the child list as a very last "after last destructor is
> called"
> step, not "before destructors are called".
>
> also we need to do better parent policing.
>
> 1. objects that can never have a NULL parent need to be marked as such
> 1.2 objects that MUST have a loop parent at the top of their parent tree or
> somewhere in it (provider_find for a loop class must fund a non-null loop
> object).
> 1.3 objects MUST have a loop object and it MUST be the main loop object and
> only that one.
> 2. some objects are intended to be toplevels (with NULL as the parent) and
> should be marked as such (e.g. loop objects).
>
> > If we are delegating to a parent to manage the lifecycle of the object
> then
> > we should step away from the reference and forget it - that is the most
> > "convenient" behaviour I guess.
> >
> > if:
> > efl_add *always* returned an owned reference and took no parent
> > efl_add_child *never* returned an owned reference and required a parent
> >
> > then:
> > efl_add_ref* would no longer be required right? (if the binding requires
> a
> > ref after efl_add_child we have efl_ref that it could wrap up)
> > efl_del would take a reference and dec (probably not needed as we have
> > efl_unref?)
> > efl_del_child seems unlikely to be needed as all that is left is hiding
> > which is a graphics call, not an eo lifecycle one.
> >
> > The more I look at it the more I think we have too much UI related
> thinking
> > in our object lifecycle.
>
> that's because 90% of our objects have been UI related in the past sand
> pretty
> much still are. for us, i think this is the right thing.
>
> > Andy
> >
> > On Wed, 3 Jan 2018 at 05:41 Cedric Bail  wrote:
> >
> > > > The whole efl_del argument just exist because it is kinda poorly
> > > > named. IMO, del means: get this object to an "empty" state.
> > > > Just like close to files and hide and unparent to UI objects. efl_del
> > > > should not steal references under people who owns it, the object
> > > > would get deleted at a later time when everybody using the object
> > > > stops doing so, we could even return errors from efl_del'eted
> > > > objects for methods that do not make sense anymore, causing
> > > > most actions to, possibly, halt earlier rather than later.
> > >
> > > So, if efl_del does not still a references under people who owns it,
> how
> > > do we fix it ? Should it still magically reset its parent to NULL when
> > > there is one and just efl_unref in the other case ? Should it be
> symetric
> > > to efl_add_ref and always reset the parent to NULL along with unref ?
> Or
> > > should it do none of this at all and you have to manually do the
> parent set
> > > and the unref ?
> > >
> > > Trying to figure out what behavior would make it work for binding, I
> would
> > > guess it would be best to just make it symetric to efl_add_ref. This
> will
> > > give a predictable outcome I think, but I am not sure it is enough.
> What 

Re: [E-devel] efl_add causing confusion

[Andrew Williams]

Hi,

I would avoid the name efl_release if it is not to do with object lifecycle
as that is the word ObjectiveC uses in their retain/release pair for memory
management.

Just a thought if we are trying to attract devs from elsewhere.

Andy

On Thu, 4 Jan 2018 at 00:04 Cedric Bail <ced...@ddlm.me> wrote:

> >  Original Message 
> > Subject: Re: [E-devel] efl_add causing confusion
> > Local Time: January 3, 2018 1:43 AM
> > UTC Time: January 3, 2018 9:43 AM
> > From: a...@andywilliams.me
> > To: Enlightenment developer list <
> enlightenment-devel@lists.sourceforge.net>
> >
> > Hi Cedric,
> >
> > I think I agree, if we can tidy the definitions then everything should
> get
> > clearer.
> >
> > efl_add_ref being needed for bindings implies that our definition of
> > efl_add was not clean enough in the first place.
> > If efl_del is "hide and unparent" then maybe what we really need is
> > efl_gfx_hide and efl_parent_unset - but I don't see why we need to
> unparent
> > anyhow...
>
> After some discussion with Felipe we came to the conclusion that efl_del
> is not properly design and the core problem is indeed that it does to many
> things. Our idea is to split it in two part. efl_release that will have
> different meaning on object, could close a socket if it an network object
> for example or hide and remove from scenegraph if it is a canvas object.
> This will not affect the object lifecycle. It will make it emit a new event
> once it is released (This would be a first step, but later on we would
> likely want to block some function from working when an object is released
> and the ability to unrelease an object).
>
> The second function would be what efl_del kind of does, but moved to an
> inline function that call efl_release. This way binding can use release (it
> doesn't randomly mess with the object life cycle) and provide their own
> logic for del. Now in C, it is still not obvious what efl_del should do in
> term of unparenting and unref. Right now, it does a wonderful if there is a
> parent, then unset the parent, if there is no parent, then just unref. But
> in the following example :
>
> obj = efl_add(CLASS, NULL);
> efl_parent_set(obj, some_parent);
> efl_del(obj);
>
> obj will still have one reference, but no parent left, while in the
> example below :
>
> obj = efl_add(CLASS, some_parent);
> efl_del(obj);
>
> The object will have been properly destroyed. This kind of problem happen
> easily in our API when we actually return a new Eo object. Depending on how
> we create it, its reference count might change. My personal take is that
> efl_del should not do anything on the parent. If we give an object to be
> managed by the parent, del should not be the one touching it. The parent
> should. So I would argue that efl_del should just be efl_release +
> efl_unref.
>
> > If we are delegating to a parent to manage the lifecycle of the object
> then
> > we should step away from the reference and forget it - that is the most
> > "convenient" behaviour I guess.
> >
> > if:
> > efl_add always returned an owned reference and took no parent
> > efl_add_child never returned an owned reference and required a parent
>
> This seems to match the change we think are necessary to efl_del.
>
> > then:
> > efl_add_ref* would no longer be required right? (if the binding requires
> a
> > ref after efl_add_child we have efl_ref that it could wrap up)
>
> We could still provide an efl_add_ref helper in C too, but otherwise yes
> to your point.
>
> > efl_del would take a reference and dec (probably not needed as we have
> > efl_unref?)
> > efl_del_child seems unlikely to be needed as all that is left is hiding
> > which is a graphics call, not an eo lifecycle one.
>
> I am not a fan of the idea of an efl_del_child, but as it would be an
> inline helper, it can be done later if we feel it is useful.
>
> This proposal would work for binding. Right now, they rely on efl_unref
> and can't call efl_del due to the unpredictable lifecycle that result of
> it, cleaning that, clean also the other side of the confusion by making
> efl_add more rigorous in its behavior.
>
> 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
>


-- 
http://andywilliams.me
http://ajwillia.ms
--
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

Re: [E-devel] efl_add causing confusion

[Carsten Haitzler]

On Wed, 03 Jan 2018 00:41:30 -0500 Cedric Bail  said:

> > The whole efl_del argument just exist because it is kinda poorly
> > named. IMO, del means: get this object to an "empty" state.
> > Just like close to files and hide and unparent to UI objects. efl_del
> > should not steal references under people who owns it, the object
> > would get deleted at a later time when everybody using the object
> > stops doing so, we could even return errors from efl_del'eted
> > objects for methods that do not make sense anymore, causing
> > most actions to, possibly, halt earlier rather than later.
> 
> So, if efl_del does not still a references under people who owns it, how do
> we fix it ? Should it still magically reset its parent to NULL when there is
> one and just efl_unref in the other case ? Should it be symetric to
> efl_add_ref and always reset the parent to NULL along with unref ? Or should
> it do none of this at all and you have to manually do the parent set and the
> unref ?
> 
> Trying to figure out what behavior would make it work for binding, I would
> guess it would be best to just make it symetric to efl_add_ref. This will
> give a predictable outcome I think, but I am not sure it is enough. What do
> you think ?

my take on efl_del is this:

1. is is allowed to do things like "hide objects", "close fd's" etc. like
felipe said, BUt it will not unparent UNLESS this is the LAST ref, then it
unparents (which is equivalent to an unref in addition to removal from parent
as any removal from the parent removes a ref).

> >   IMO, the whole problem with efl_add/efl_add_ref is that
> > "parents" are treated specially, which they should not. parent_set
> > should increment efl_ref and parent_unset should decrement it.
> 
> Agreed and surprised it is not the case.
> 
> > For C, OTH, where we do expect some "automatism" on resource
> > handling, efl_unref'ing may be too much of a hassle when a
> > parent is already going to handle the lifetime of the object. So,
> > it would make sense, IMO, for efl_add_scope. It could even be
> > that efl_add_scope is named efl_add, no problem, as long as
> > there's a efl_add that keeps this semantics for binding
> > development. Which also means that parent_set/unset must
> > be fixed.
> 
> I think that once efl_del behavior is clearly defined, the existence of an
> efl_add_scope/efl_add will also be clearer to everyone.
> 
> >   Also, @own tags must not relate to parent_set, because that
> > has no useful information for tags or users, if needed we can
> > add a @reparent tag, but that's not really special information
> > such as real owernship information.
> 
> I am still wondering what the @own really mean. Does that mean that the
> object own at least one reference of it ? But in that case, doesn't that mean
> that the user need to always ref it, if it plans to keep it around.
> 
> As for @reparent, I am not sure we have case yet where we return an object
> that can not be reparented, do we have such a case ?

i think we will have the case for:

1. objects must have a parent that is in the main loop tree (as they are bound
to the main loop and main loop only).
2. objects that must have NO parent (and no references to objects outside
of their child tree) (if we ever support transfer of objects from thread to
thread).


> 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

Re: [E-devel] efl_add causing confusion

[Carsten Haitzler]

On Wed, 03 Jan 2018 09:43:16 + Andrew Williams  said:

> Hi Cedric,
> 
> I think I agree, if we can tidy the definitions then everything should get
> clearer.
> 
> efl_add_ref being needed for bindings implies that our definition of
> efl_add was not clean enough in the first place.

we were very clear on that. for bindings it's needed (or if you want to write
code in that style) but it's highly inconvenient and we've been over that.

> If efl_del is "hide and unparent" then maybe what we really need is
> efl_gfx_hide and efl_parent_unset - but I don't see why we need to unparent
> anyhow...

well if it's the last ref... the unparenting cleanly removes from the parent
AND removes the last ref as a result.

BUT i kind of agree with you here. the unparenting is proving to be a major
pain in the rear. by that i mean the unparenting happens and then parent is
NULL and THEN as a result of this reference goes to 0 and destructors get
called. when the destructors are called, the parent is already NULL and this
has proven an "odd" case i've been dealing with the efl loop work... because on
destruction the object doesn't know what loop it belonged to anymore... yes -
you have to handle parent_set to NULL then but it means this has to do some
kind of destruction of loop bound resources... :(

IMHO when the destructors are called the object should still have a parent and
be removed from the child list as a very last "after last destructor is called"
step, not "before destructors are called".

also we need to do better parent policing.

1. objects that can never have a NULL parent need to be marked as such
1.2 objects that MUST have a loop parent at the top of their parent tree or
somewhere in it (provider_find for a loop class must fund a non-null loop
object).
1.3 objects MUST have a loop object and it MUST be the main loop object and
only that one.
2. some objects are intended to be toplevels (with NULL as the parent) and
should be marked as such (e.g. loop objects).

> If we are delegating to a parent to manage the lifecycle of the object then
> we should step away from the reference and forget it - that is the most
> "convenient" behaviour I guess.
> 
> if:
> efl_add *always* returned an owned reference and took no parent
> efl_add_child *never* returned an owned reference and required a parent
> 
> then:
> efl_add_ref* would no longer be required right? (if the binding requires a
> ref after efl_add_child we have efl_ref that it could wrap up)
> efl_del would take a reference and dec (probably not needed as we have
> efl_unref?)
> efl_del_child seems unlikely to be needed as all that is left is hiding
> which is a graphics call, not an eo lifecycle one.
> 
> The more I look at it the more I think we have too much UI related thinking
> in our object lifecycle.

that's because 90% of our objects have been UI related in the past sand pretty
much still are. for us, i think this is the right thing.

> Andy
> 
> On Wed, 3 Jan 2018 at 05:41 Cedric Bail  wrote:
> 
> > > The whole efl_del argument just exist because it is kinda poorly
> > > named. IMO, del means: get this object to an "empty" state.
> > > Just like close to files and hide and unparent to UI objects. efl_del
> > > should not steal references under people who owns it, the object
> > > would get deleted at a later time when everybody using the object
> > > stops doing so, we could even return errors from efl_del'eted
> > > objects for methods that do not make sense anymore, causing
> > > most actions to, possibly, halt earlier rather than later.
> >
> > So, if efl_del does not still a references under people who owns it, how
> > do we fix it ? Should it still magically reset its parent to NULL when
> > there is one and just efl_unref in the other case ? Should it be symetric
> > to efl_add_ref and always reset the parent to NULL along with unref ? Or
> > should it do none of this at all and you have to manually do the parent set
> > and the unref ?
> >
> > Trying to figure out what behavior would make it work for binding, I would
> > guess it would be best to just make it symetric to efl_add_ref. This will
> > give a predictable outcome I think, but I am not sure it is enough. What do
> > you think ?
> >
> > >   IMO, the whole problem with efl_add/efl_add_ref is that
> > > "parents" are treated specially, which they should not. parent_set
> > > should increment efl_ref and parent_unset should decrement it.
> >
> > Agreed and surprised it is not the case.
> >
> > > For C, OTH, where we do expect some "automatism" on resource
> > > handling, efl_unref'ing may be too much of a hassle when a
> > > parent is already going to handle the lifetime of the object. So,
> > > it would make sense, IMO, for efl_add_scope. It could even be
> > > that efl_add_scope is named efl_add, no problem, as long as
> > > there's a efl_add that keeps this semantics for binding
> > > development. Which also means that parent_set/unset must
> 

Re: [E-devel] efl_add causing confusion

[Carsten Haitzler]

On Wed, 03 Jan 2018 19:03:39 -0500 Cedric Bail <ced...@ddlm.me> said:

> >  Original Message 
> > Subject: Re: [E-devel] efl_add causing confusion
> > Local Time: January 3, 2018 1:43 AM
> > UTC Time: January 3, 2018 9:43 AM
> > From: a...@andywilliams.me
> > To: Enlightenment developer list <enlightenment-devel@lists.sourceforge.net>
> >
> > Hi Cedric,
> >
> > I think I agree, if we can tidy the definitions then everything should get
> > clearer.
> >
> > efl_add_ref being needed for bindings implies that our definition of
> > efl_add was not clean enough in the first place.
> > If efl_del is "hide and unparent" then maybe what we really need is
> > efl_gfx_hide and efl_parent_unset - but I don't see why we need to unparent
> > anyhow...
> 
> After some discussion with Felipe we came to the conclusion that efl_del is
> not properly design and the core problem is indeed that it does to many
> things. Our idea is to split it in two part. efl_release that will have
> different meaning on object, could close a socket if it an network object for
> example or hide and remove from scenegraph if it is a canvas object. This
> will not affect the object lifecycle. It will make it emit a new event once
> it is released (This would be a first step, but later on we would likely want
> to block some function from working when an object is released and the
> ability to unrelease an object).
> 
> The second function would be what efl_del kind of does, but moved to an
> inline function that call efl_release. This way binding can use release (it
> doesn't randomly mess with the object life cycle) and provide their own logic
> for del. Now in C, it is still not obvious what efl_del should do in term of
> unparenting and unref. Right now, it does a wonderful if there is a parent,
> then unset the parent, if there is no parent, then just unref. But in the
> following example :
> 
> obj = efl_add(CLASS, NULL);
> efl_parent_set(obj, some_parent);
> efl_del(obj);
> 
> obj will still have one reference, but no parent left, while in the example
> below :

are you sure about the above?

 if (!prev_parent && pd->parent_sunk) efl_ref(obj);

so if there was no previous parent (this case...) only if parent_sunk is set
to we add the ref (rather than transfer it). so basically parent_sunk will be
set if you did an efl_add with a non-null parent, or you used efl_add_ref().

> obj = efl_add(CLASS, some_parent);
> efl_del(obj);
> 
> The object will have been properly destroyed. This kind of problem happen
> easily in our API when we actually return a new Eo object. Depending on how
> we create it, its reference count might change. My personal take is that
> efl_del should not do anything on the parent. If we give an object to be
> managed by the parent, del should not be the one touching it. The parent
> should. So I would argue that efl_del should just be efl_release + efl_unref.

See my reply to andy about this right above and how having a NULL parent during
destruction is a major pain... :)

> > If we are delegating to a parent to manage the lifecycle of the object then
> > we should step away from the reference and forget it - that is the most
> > "convenient" behaviour I guess.
> >
> > if:
> > efl_add always returned an owned reference and took no parent
> > efl_add_child never returned an owned reference and required a parent
> 
> This seems to match the change we think are necessary to efl_del.

i really dislike the idea of efl_add_child(). see my other replies to andy.
this will be 90-99% of all adds. adding as a child to something. don't make it
longer. but the change to this code will be a lot of work and painful. and i
doubt it's really any different to parent being just NULL or non-NULL (and
leads to 4 add funcs instead of our current 2).

> > then:
> > efl_add_ref* would no longer be required right? (if the binding requires a
> > ref after efl_add_child we have efl_ref that it could wrap up)
> 
> We could still provide an efl_add_ref helper in C too, but otherwise yes to
> your point.
> 
> > efl_del would take a reference and dec (probably not needed as we have
> > efl_unref?)
> > efl_del_child seems unlikely to be needed as all that is left is hiding
> > which is a graphics call, not an eo lifecycle one.
> 
> I am not a fan of the idea of an efl_del_child, but as it would be an inline
> helper, it can be done later if we feel it is useful.
> 
> This proposal would work for binding. Right now, they rely on efl_unref and
> can't call efl_del due to the unpredictable lifecycle that result of it,
> cleaning that, clean also the other sid

Re: [E-devel] efl_add causing confusion

[Carsten Haitzler]

On Wed, 03 Jan 2018 09:36:34 + Andrew Williams  said:

> Wow, I guess I walked into that with my "... whatever ..." so let's go 1
> step simpler still:
> 
> void some_api(Eo *parent) {
>Eo *var = efl_add(SOME_CLASS, parent);
> 
>printf("I got an Eo %p, %s\n", var, efl_name_get(var));
> 
>// TODO figure whether or not I should release var
>efl_unref(var); // ???
> }
> 
> This is still not clear as we don't know what parent was used. Compare to:
> 
> void some_api(void) {
>char *ptr = malloc(10);
> 
>printf("Got pointer %p\n", ptr);
> 
>free(ptr);
> }
> 
> Here it is clear. malloc always returns a memory handle that must be freed.
> 
> Are you seeing the confusion yet? I don't think I have it in me to explain
> any further.

the point i made about using parent var i made before stands. the code screams
to me "i must have a parent". otherwise NULL would be used. the majority
(probably vast majority) of objects will probably need a parent, thus what we
have is optimized to be short, simple and sweet for the majority of use cases.
if we added a "no parent" version which would make the most sense
(efl_noparent_add) since it would make the less common case more wordy, then
we'd need to add variants also for the _ref case (efl_noparent_add_ref) so we'd
be adding 2, not 1. is having "rarely used constructors" added to what people
need to learn and remember a good thing just to avoid passing NULL as a parent?
(because accompanying this change would make NULL parents an invalid param and
need to cause efl_add to fail).

i have doubts if this is a good thing. it raises the learning curve. but then
there is all the work needed to retrofit the code we have to match this core
change. let's just assume the change os 50/50 good/bad ... which i think it's
not even there, but let's say it's even. there is a lot of work needed to
change efl's codebase to match and this (1571 instances from my count right
now). you have to get all those 1500+ changes correct. likely 80% will be able
to be gotten right. some of these need fixing too like test_efl_anim_*.c which
use NULL parents... without a loop to run these tests... how will they sensibly
work? yup. they rely on ecore_animators internally... but this is wrong for
efl's design direction... these should have. in fact must have parent objects
(that trace back to a loop - the main loop in this case).

> What I really want to avoid is having people confused by our new API and
> having to review this again in retrospect a few years down the line.

I get what you are saying. I think that a NULL parent is going to be a rare
case. it's where you have created a floating object (or very rare toplevel tree
objects like loops but these will be created for you like the mainloop one
is, as a part of creating a thread object which WILL be a child of your
current loop object IMHO). and as a floating object... i point to the
discussion surrounding gtk and prior votes/threads etc. :) i disagree that this
is a "the sky is falling" issue. i think it's one of those "it's not perfect
but the alternative is just as bad if not a little worse" situations.

> Andy
> 
> On Wed, 3 Jan 2018 at 03:59 Carsten Haitzler  wrote:
> 
> > On Tue, 02 Jan 2018 19:19:37 + Andrew Williams 
> > said:
> >
> > > Hi,
> > >
> > > I am obviously struggling to explain this with words, so will try with
> > code.
> > > Take the following code fragment:
> > >
> > > void some_api(Eo *parent) {
> > >Eo *var = efl_add(SOME_CLASS, parent);
> > >
> > >... do some stuff with var (possible, acknowledged, race condition)
> > ...
> > >
> > >// TODO figure whether or not I should release var
> > >efl_unref(var); // ???
> > > }
> > >
> > > In that code how can I know the correct behaviour without inspecting the
> > > content of parent?
> >
> > the same as:
> >
> > void some_api(...) {
> >char *ptr = malloc(10);
> >...
> >free(ptr); // ???
> > }
> >
> > you do NOT know if you are to free or not ... it depends what you do with
> > ptr
> > between malloc and end of scope. do you pass the ptr to another func to
> > consume it or not?stick it in a list or array to store it? this is life in
> > C. C
> > is not java.
> >
> > parent here is an explicit store like a list or array. store this object in
> > this parent (if not null). it's explicit.
> >
> > > If I am a user of this API I would expect to be able to read code and
> > > understand if the correct objects have been released.
> > > With the last line this may be released too soon, without it we may have
> > a
> > > memory leak...
> >
> > reading your sample code says to me "this object REQUIRES a parent"
> > otherwise
> > why would you pass in a parent var at all? you'd use NULL if it doesn't.
> >
> > > Does that help?
> > > Andy
> > >
> > > On Wed, 27 Dec 2017 at 07:15 Carsten Haitzler 
> > wrote:
> > >
> > > > On Tue, 26 Dec 2017 

Re: [E-devel] efl_add causing confusion

[Cedric Bail]

>  Original Message 
> Subject: Re: [E-devel] efl_add causing confusion
> Local Time: January 3, 2018 1:43 AM
> UTC Time: January 3, 2018 9:43 AM
> From: a...@andywilliams.me
> To: Enlightenment developer list <enlightenment-devel@lists.sourceforge.net>
>
> Hi Cedric,
>
> I think I agree, if we can tidy the definitions then everything should get
> clearer.
>
> efl_add_ref being needed for bindings implies that our definition of
> efl_add was not clean enough in the first place.
> If efl_del is "hide and unparent" then maybe what we really need is
> efl_gfx_hide and efl_parent_unset - but I don't see why we need to unparent
> anyhow...

After some discussion with Felipe we came to the conclusion that efl_del is not 
properly design and the core problem is indeed that it does to many things. Our 
idea is to split it in two part. efl_release that will have different meaning 
on object, could close a socket if it an network object for example or hide and 
remove from scenegraph if it is a canvas object. This will not affect the 
object lifecycle. It will make it emit a new event once it is released (This 
would be a first step, but later on we would likely want to block some function 
from working when an object is released and the ability to unrelease an object).

The second function would be what efl_del kind of does, but moved to an inline 
function that call efl_release. This way binding can use release (it doesn't 
randomly mess with the object life cycle) and provide their own logic for del. 
Now in C, it is still not obvious what efl_del should do in term of unparenting 
and unref. Right now, it does a wonderful if there is a parent, then unset the 
parent, if there is no parent, then just unref. But in the following example :

obj = efl_add(CLASS, NULL);
efl_parent_set(obj, some_parent);
efl_del(obj);

obj will still have one reference, but no parent left, while in the example 
below :

obj = efl_add(CLASS, some_parent);
efl_del(obj);

The object will have been properly destroyed. This kind of problem happen 
easily in our API when we actually return a new Eo object. Depending on how we 
create it, its reference count might change. My personal take is that efl_del 
should not do anything on the parent. If we give an object to be managed by the 
parent, del should not be the one touching it. The parent should. So I would 
argue that efl_del should just be efl_release + efl_unref.

> If we are delegating to a parent to manage the lifecycle of the object then
> we should step away from the reference and forget it - that is the most
> "convenient" behaviour I guess.
>
> if:
> efl_add always returned an owned reference and took no parent
> efl_add_child never returned an owned reference and required a parent

This seems to match the change we think are necessary to efl_del.

> then:
> efl_add_ref* would no longer be required right? (if the binding requires a
> ref after efl_add_child we have efl_ref that it could wrap up)

We could still provide an efl_add_ref helper in C too, but otherwise yes to 
your point.

> efl_del would take a reference and dec (probably not needed as we have
> efl_unref?)
> efl_del_child seems unlikely to be needed as all that is left is hiding
> which is a graphics call, not an eo lifecycle one.

I am not a fan of the idea of an efl_del_child, but as it would be an inline 
helper, it can be done later if we feel it is useful.

This proposal would work for binding. Right now, they rely on efl_unref and 
can't call efl_del due to the unpredictable lifecycle that result of it, 
cleaning that, clean also the other side of the confusion by making efl_add 
more rigorous in its behavior.

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

Re: [E-devel] efl_add causing confusion

[Andrew Williams]

Hi Cedric,

I think I agree, if we can tidy the definitions then everything should get
clearer.

efl_add_ref being needed for bindings implies that our definition of
efl_add was not clean enough in the first place.
If efl_del is "hide and unparent" then maybe what we really need is
efl_gfx_hide and efl_parent_unset - but I don't see why we need to unparent
anyhow...

If we are delegating to a parent to manage the lifecycle of the object then
we should step away from the reference and forget it - that is the most
"convenient" behaviour I guess.

if:
efl_add *always* returned an owned reference and took no parent
efl_add_child *never* returned an owned reference and required a parent

then:
efl_add_ref* would no longer be required right? (if the binding requires a
ref after efl_add_child we have efl_ref that it could wrap up)
efl_del would take a reference and dec (probably not needed as we have
efl_unref?)
efl_del_child seems unlikely to be needed as all that is left is hiding
which is a graphics call, not an eo lifecycle one.

The more I look at it the more I think we have too much UI related thinking
in our object lifecycle.

Andy

On Wed, 3 Jan 2018 at 05:41 Cedric Bail  wrote:

> > The whole efl_del argument just exist because it is kinda poorly
> > named. IMO, del means: get this object to an "empty" state.
> > Just like close to files and hide and unparent to UI objects. efl_del
> > should not steal references under people who owns it, the object
> > would get deleted at a later time when everybody using the object
> > stops doing so, we could even return errors from efl_del'eted
> > objects for methods that do not make sense anymore, causing
> > most actions to, possibly, halt earlier rather than later.
>
> So, if efl_del does not still a references under people who owns it, how
> do we fix it ? Should it still magically reset its parent to NULL when
> there is one and just efl_unref in the other case ? Should it be symetric
> to efl_add_ref and always reset the parent to NULL along with unref ? Or
> should it do none of this at all and you have to manually do the parent set
> and the unref ?
>
> Trying to figure out what behavior would make it work for binding, I would
> guess it would be best to just make it symetric to efl_add_ref. This will
> give a predictable outcome I think, but I am not sure it is enough. What do
> you think ?
>
> >   IMO, the whole problem with efl_add/efl_add_ref is that
> > "parents" are treated specially, which they should not. parent_set
> > should increment efl_ref and parent_unset should decrement it.
>
> Agreed and surprised it is not the case.
>
> > For C, OTH, where we do expect some "automatism" on resource
> > handling, efl_unref'ing may be too much of a hassle when a
> > parent is already going to handle the lifetime of the object. So,
> > it would make sense, IMO, for efl_add_scope. It could even be
> > that efl_add_scope is named efl_add, no problem, as long as
> > there's a efl_add that keeps this semantics for binding
> > development. Which also means that parent_set/unset must
> > be fixed.
>
> I think that once efl_del behavior is clearly defined, the existence of an
> efl_add_scope/efl_add will also be clearer to everyone.
>
> >   Also, @own tags must not relate to parent_set, because that
> > has no useful information for tags or users, if needed we can
> > add a @reparent tag, but that's not really special information
> > such as real owernship information.
>
> I am still wondering what the @own really mean. Does that mean that the
> object own at least one reference of it ? But in that case, doesn't that
> mean that the user need to always ref it, if it plans to keep it around.
>
> As for @reparent, I am not sure we have case yet where we return an object
> that can not be reparented, do we have such a case ?
>
> 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
>


-- 
http://andywilliams.me
http://ajwillia.ms
--
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

Re: [E-devel] efl_add causing confusion

[Andrew Williams]

Wow, I guess I walked into that with my "... whatever ..." so let's go 1
step simpler still:

void some_api(Eo *parent) {
   Eo *var = efl_add(SOME_CLASS, parent);

   printf("I got an Eo %p, %s\n", var, efl_name_get(var));

   // TODO figure whether or not I should release var
   efl_unref(var); // ???
}

This is still not clear as we don't know what parent was used. Compare to:

void some_api(void) {
   char *ptr = malloc(10);

   printf("Got pointer %p\n", ptr);

   free(ptr);
}

Here it is clear. malloc always returns a memory handle that must be freed.

Are you seeing the confusion yet? I don't think I have it in me to explain
any further.
What I really want to avoid is having people confused by our new API and
having to review this again in retrospect a few years down the line.

Andy

On Wed, 3 Jan 2018 at 03:59 Carsten Haitzler  wrote:

> On Tue, 02 Jan 2018 19:19:37 + Andrew Williams 
> said:
>
> > Hi,
> >
> > I am obviously struggling to explain this with words, so will try with
> code.
> > Take the following code fragment:
> >
> > void some_api(Eo *parent) {
> >Eo *var = efl_add(SOME_CLASS, parent);
> >
> >... do some stuff with var (possible, acknowledged, race condition)
> ...
> >
> >// TODO figure whether or not I should release var
> >efl_unref(var); // ???
> > }
> >
> > In that code how can I know the correct behaviour without inspecting the
> > content of parent?
>
> the same as:
>
> void some_api(...) {
>char *ptr = malloc(10);
>...
>free(ptr); // ???
> }
>
> you do NOT know if you are to free or not ... it depends what you do with
> ptr
> between malloc and end of scope. do you pass the ptr to another func to
> consume it or not?stick it in a list or array to store it? this is life in
> C. C
> is not java.
>
> parent here is an explicit store like a list or array. store this object in
> this parent (if not null). it's explicit.
>
> > If I am a user of this API I would expect to be able to read code and
> > understand if the correct objects have been released.
> > With the last line this may be released too soon, without it we may have
> a
> > memory leak...
>
> reading your sample code says to me "this object REQUIRES a parent"
> otherwise
> why would you pass in a parent var at all? you'd use NULL if it doesn't.
>
> > Does that help?
> > Andy
> >
> > On Wed, 27 Dec 2017 at 07:15 Carsten Haitzler 
> wrote:
> >
> > > On Tue, 26 Dec 2017 17:46:50 -0200 Felipe Magno de Almeida
> > >  said:
> > >
> > > > JP, Cedric, me and TAsn have been having this argument for a while.
> > > >
> > > > IMO, the whole problem is that we're thinking of ownership in terms
> > > > of parentship, which is wrong with reference counting. Ownership
> > > > is shared between all reference owners and that's it. That's also
> > > > the only sane way for bindings to work without having references
> > > > being pull under their feet.
> > > >
> > > > The whole efl_del argument just exist because it is kinda poorly
> > > > named. IMO, del means: get this object to an "empty" state.
> > > > Just like close to files and hide and unparent to UI objects. efl_del
> > > > should not steal references under people who owns it, the object
> > > > would get deleted at a later time when everybody using the object
> > > > stops doing so, we could even return errors from efl_del'eted
> > > > objects for methods that do not make sense anymore, causing
> > > > most actions to, possibly, halt earlier rather than later.
> > > >
> > > > IMO, the whole problem with efl_add/efl_add_ref is that
> > > > "parents" are treated specially, which they should not. parent_set
> > > > should increment efl_ref and parent_unset should decrement it.
> > >
> > > that's actually what happens, why efl_add_ref ends up with a refcount
> of
> > > 2, and
> > > efl_add has a refcount of 1 (if parent is non-null). efl_add if parent
> is
> > > not
> > > null is doing a parent_set during add. it's taking the "convenience"
> that
> > > the
> > > parent is transferred from scope to parent just so you dont have to
> unref
> > > at
> > > end of scope manually - in c that is. we're really just talking c here.
> > >
> > > > For C, OTH, where we do expect some "automatism" on resource
> > > > handling, efl_unref'ing may be too much of a hassle when a
> > > > parent is already going to handle the lifetime of the object. So,
> > >
> > > that was precisely the vote and discussion back in 2014. so while what
> we
> > > have
> > > in c is not "strictly correct" (so to speak) it's far more convenient
> than
> > > forcing people to manually unref at end of scope.
> > >
> > > > it would make sense, IMO, for efl_add_scope. It could even be
> > > > that efl_add_scope is named efl_add, no problem, as long as
> > > > there's a efl_add that keeps this semantics for binding
> > > > development. Which also means that parent_set/unset must
> > > > be fixed.
> > >

Re: [E-devel] efl_add causing confusion

[Cedric Bail]

> The whole efl_del argument just exist because it is kinda poorly
> named. IMO, del means: get this object to an "empty" state.
> Just like close to files and hide and unparent to UI objects. efl_del
> should not steal references under people who owns it, the object
> would get deleted at a later time when everybody using the object
> stops doing so, we could even return errors from efl_del'eted
> objects for methods that do not make sense anymore, causing
> most actions to, possibly, halt earlier rather than later.

So, if efl_del does not still a references under people who owns it, how do we 
fix it ? Should it still magically reset its parent to NULL when there is one 
and just efl_unref in the other case ? Should it be symetric to efl_add_ref and 
always reset the parent to NULL along with unref ? Or should it do none of this 
at all and you have to manually do the parent set and the unref ?

Trying to figure out what behavior would make it work for binding, I would 
guess it would be best to just make it symetric to efl_add_ref. This will give 
a predictable outcome I think, but I am not sure it is enough. What do you 
think ?

>   IMO, the whole problem with efl_add/efl_add_ref is that
> "parents" are treated specially, which they should not. parent_set
> should increment efl_ref and parent_unset should decrement it.

Agreed and surprised it is not the case.

> For C, OTH, where we do expect some "automatism" on resource
> handling, efl_unref'ing may be too much of a hassle when a
> parent is already going to handle the lifetime of the object. So,
> it would make sense, IMO, for efl_add_scope. It could even be
> that efl_add_scope is named efl_add, no problem, as long as
> there's a efl_add that keeps this semantics for binding
> development. Which also means that parent_set/unset must
> be fixed.

I think that once efl_del behavior is clearly defined, the existence of an 
efl_add_scope/efl_add will also be clearer to everyone.

>   Also, @own tags must not relate to parent_set, because that
> has no useful information for tags or users, if needed we can
> add a @reparent tag, but that's not really special information
> such as real owernship information.

I am still wondering what the @own really mean. Does that mean that the object 
own at least one reference of it ? But in that case, doesn't that mean that the 
user need to always ref it, if it plans to keep it around.

As for @reparent, I am not sure we have case yet where we return an object that 
can not be reparented, do we have such a case ?

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

Re: [E-devel] efl_add causing confusion

[Carsten Haitzler]

On Tue, 02 Jan 2018 19:19:37 + Andrew Williams  said:

> Hi,
> 
> I am obviously struggling to explain this with words, so will try with code.
> Take the following code fragment:
> 
> void some_api(Eo *parent) {
>Eo *var = efl_add(SOME_CLASS, parent);
> 
>... do some stuff with var (possible, acknowledged, race condition) ...
> 
>// TODO figure whether or not I should release var
>efl_unref(var); // ???
> }
> 
> In that code how can I know the correct behaviour without inspecting the
> content of parent?

the same as:

void some_api(...) {
   char *ptr = malloc(10);
   ...
   free(ptr); // ???
}

you do NOT know if you are to free or not ... it depends what you do with ptr
between malloc and end of scope. do you pass the ptr to another func to
consume it or not?stick it in a list or array to store it? this is life in C. C
is not java.

parent here is an explicit store like a list or array. store this object in
this parent (if not null). it's explicit.

> If I am a user of this API I would expect to be able to read code and
> understand if the correct objects have been released.
> With the last line this may be released too soon, without it we may have a
> memory leak...

reading your sample code says to me "this object REQUIRES a parent" otherwise
why would you pass in a parent var at all? you'd use NULL if it doesn't.

> Does that help?
> Andy
> 
> On Wed, 27 Dec 2017 at 07:15 Carsten Haitzler  wrote:
> 
> > On Tue, 26 Dec 2017 17:46:50 -0200 Felipe Magno de Almeida
> >  said:
> >
> > > JP, Cedric, me and TAsn have been having this argument for a while.
> > >
> > > IMO, the whole problem is that we're thinking of ownership in terms
> > > of parentship, which is wrong with reference counting. Ownership
> > > is shared between all reference owners and that's it. That's also
> > > the only sane way for bindings to work without having references
> > > being pull under their feet.
> > >
> > > The whole efl_del argument just exist because it is kinda poorly
> > > named. IMO, del means: get this object to an "empty" state.
> > > Just like close to files and hide and unparent to UI objects. efl_del
> > > should not steal references under people who owns it, the object
> > > would get deleted at a later time when everybody using the object
> > > stops doing so, we could even return errors from efl_del'eted
> > > objects for methods that do not make sense anymore, causing
> > > most actions to, possibly, halt earlier rather than later.
> > >
> > > IMO, the whole problem with efl_add/efl_add_ref is that
> > > "parents" are treated specially, which they should not. parent_set
> > > should increment efl_ref and parent_unset should decrement it.
> >
> > that's actually what happens, why efl_add_ref ends up with a refcount of
> > 2, and
> > efl_add has a refcount of 1 (if parent is non-null). efl_add if parent is
> > not
> > null is doing a parent_set during add. it's taking the "convenience" that
> > the
> > parent is transferred from scope to parent just so you dont have to unref
> > at
> > end of scope manually - in c that is. we're really just talking c here.
> >
> > > For C, OTH, where we do expect some "automatism" on resource
> > > handling, efl_unref'ing may be too much of a hassle when a
> > > parent is already going to handle the lifetime of the object. So,
> >
> > that was precisely the vote and discussion back in 2014. so while what we
> > have
> > in c is not "strictly correct" (so to speak) it's far more convenient than
> > forcing people to manually unref at end of scope.
> >
> > > it would make sense, IMO, for efl_add_scope. It could even be
> > > that efl_add_scope is named efl_add, no problem, as long as
> > > there's a efl_add that keeps this semantics for binding
> > > development. Which also means that parent_set/unset must
> > > be fixed.
> >
> > that'd be efl_add_ref() for bindings where scope is auto-managed by the
> > language (as above), and efl_add for c. if you want to write c in the
> > "strict
> > scope references" way like these other languages, then efl_add_ref is
> > there for
> > that. it's going to be inconvenient and requires you to handle scope
> > cleaning
> > correctly. with some gcc extensions this can actually be automated, but the
> > problem is we have to have our api work without such extensions.
> >
> > you can use __attribute__ ((__cleanup__(efl_unref))) on such vars in
> > gcc... but
> > it's non-standard. if this was standard across all major compilers then
> > this
> > whole argument would be moot. we're have efl_add behave like efl_add_ref
> > all
> > the time and require all objects handles to be unique in scope and have
> > some
> > macro to declare the object handle with the above attributes and you have
> > to
> > compile with -fexceptions to handle c++ exception unwinding if it goes
> > through
> > some c code along the way but as i said. it's non-standard. we can't
> > 

Re: [E-devel] efl_add causing confusion

[Andrew Williams]

Hi,

I am obviously struggling to explain this with words, so will try with code.
Take the following code fragment:

void some_api(Eo *parent) {
   Eo *var = efl_add(SOME_CLASS, parent);

   ... do some stuff with var (possible, acknowledged, race condition) ...

   // TODO figure whether or not I should release var
   efl_unref(var); // ???
}

In that code how can I know the correct behaviour without inspecting the
content of parent?

If I am a user of this API I would expect to be able to read code and
understand if the correct objects have been released.
With the last line this may be released too soon, without it we may have a
memory leak...

Does that help?
Andy

On Wed, 27 Dec 2017 at 07:15 Carsten Haitzler  wrote:

> On Tue, 26 Dec 2017 17:46:50 -0200 Felipe Magno de Almeida
>  said:
>
> > JP, Cedric, me and TAsn have been having this argument for a while.
> >
> > IMO, the whole problem is that we're thinking of ownership in terms
> > of parentship, which is wrong with reference counting. Ownership
> > is shared between all reference owners and that's it. That's also
> > the only sane way for bindings to work without having references
> > being pull under their feet.
> >
> > The whole efl_del argument just exist because it is kinda poorly
> > named. IMO, del means: get this object to an "empty" state.
> > Just like close to files and hide and unparent to UI objects. efl_del
> > should not steal references under people who owns it, the object
> > would get deleted at a later time when everybody using the object
> > stops doing so, we could even return errors from efl_del'eted
> > objects for methods that do not make sense anymore, causing
> > most actions to, possibly, halt earlier rather than later.
> >
> > IMO, the whole problem with efl_add/efl_add_ref is that
> > "parents" are treated specially, which they should not. parent_set
> > should increment efl_ref and parent_unset should decrement it.
>
> that's actually what happens, why efl_add_ref ends up with a refcount of
> 2, and
> efl_add has a refcount of 1 (if parent is non-null). efl_add if parent is
> not
> null is doing a parent_set during add. it's taking the "convenience" that
> the
> parent is transferred from scope to parent just so you dont have to unref
> at
> end of scope manually - in c that is. we're really just talking c here.
>
> > For C, OTH, where we do expect some "automatism" on resource
> > handling, efl_unref'ing may be too much of a hassle when a
> > parent is already going to handle the lifetime of the object. So,
>
> that was precisely the vote and discussion back in 2014. so while what we
> have
> in c is not "strictly correct" (so to speak) it's far more convenient than
> forcing people to manually unref at end of scope.
>
> > it would make sense, IMO, for efl_add_scope. It could even be
> > that efl_add_scope is named efl_add, no problem, as long as
> > there's a efl_add that keeps this semantics for binding
> > development. Which also means that parent_set/unset must
> > be fixed.
>
> that'd be efl_add_ref() for bindings where scope is auto-managed by the
> language (as above), and efl_add for c. if you want to write c in the
> "strict
> scope references" way like these other languages, then efl_add_ref is
> there for
> that. it's going to be inconvenient and requires you to handle scope
> cleaning
> correctly. with some gcc extensions this can actually be automated, but the
> problem is we have to have our api work without such extensions.
>
> you can use __attribute__ ((__cleanup__(efl_unref))) on such vars in
> gcc... but
> it's non-standard. if this was standard across all major compilers then
> this
> whole argument would be moot. we're have efl_add behave like efl_add_ref
> all
> the time and require all objects handles to be unique in scope and have
> some
> macro to declare the object handle with the above attributes and you have
> to
> compile with -fexceptions to handle c++ exception unwinding if it goes
> through
> some c code along the way but as i said. it's non-standard. we can't
> rely
> on it.
>
> > Also, @own tags must _not_ relate to parent_set, because that
> > has no useful information for tags or users, if needed we can
> > add a @reparent tag, but that's not really special information
> > such as real owernship information.
> >
> > So, #4 on the original OP should be treated as a bug if we
> > consider efl_add as efl_add_scope, but we also need
> > to fix parent_set/parent_unset. And code must not unref
> > more than they ref.
>
> that was my take too. it should be unrefed only once. it smells of a bug
> to me.
> i have very rarely played (i think just once or twice) with efl_add_ref().
> i've
> just use efl_ref() and efl_unref().
>
> > Regards,
> >
> >
> > On Tue, Dec 26, 2017 at 10:21 AM, Andrew Williams 
> > wrote:
> > > In the books I have read they consider null being a special case of
> > > behaviour within a 

Re: [E-devel] efl_add causing confusion

[Carsten Haitzler]

On Tue, 26 Dec 2017 17:46:50 -0200 Felipe Magno de Almeida
 said:

> JP, Cedric, me and TAsn have been having this argument for a while.
> 
> IMO, the whole problem is that we're thinking of ownership in terms
> of parentship, which is wrong with reference counting. Ownership
> is shared between all reference owners and that's it. That's also
> the only sane way for bindings to work without having references
> being pull under their feet.
> 
> The whole efl_del argument just exist because it is kinda poorly
> named. IMO, del means: get this object to an "empty" state.
> Just like close to files and hide and unparent to UI objects. efl_del
> should not steal references under people who owns it, the object
> would get deleted at a later time when everybody using the object
> stops doing so, we could even return errors from efl_del'eted
> objects for methods that do not make sense anymore, causing
> most actions to, possibly, halt earlier rather than later.
> 
> IMO, the whole problem with efl_add/efl_add_ref is that
> "parents" are treated specially, which they should not. parent_set
> should increment efl_ref and parent_unset should decrement it.

that's actually what happens, why efl_add_ref ends up with a refcount of 2, and
efl_add has a refcount of 1 (if parent is non-null). efl_add if parent is not
null is doing a parent_set during add. it's taking the "convenience" that the
parent is transferred from scope to parent just so you dont have to unref at
end of scope manually - in c that is. we're really just talking c here.

> For C, OTH, where we do expect some "automatism" on resource
> handling, efl_unref'ing may be too much of a hassle when a
> parent is already going to handle the lifetime of the object. So,

that was precisely the vote and discussion back in 2014. so while what we have
in c is not "strictly correct" (so to speak) it's far more convenient than
forcing people to manually unref at end of scope.

> it would make sense, IMO, for efl_add_scope. It could even be
> that efl_add_scope is named efl_add, no problem, as long as
> there's a efl_add that keeps this semantics for binding
> development. Which also means that parent_set/unset must
> be fixed.

that'd be efl_add_ref() for bindings where scope is auto-managed by the
language (as above), and efl_add for c. if you want to write c in the "strict
scope references" way like these other languages, then efl_add_ref is there for
that. it's going to be inconvenient and requires you to handle scope cleaning
correctly. with some gcc extensions this can actually be automated, but the
problem is we have to have our api work without such extensions.

you can use __attribute__ ((__cleanup__(efl_unref))) on such vars in gcc... but
it's non-standard. if this was standard across all major compilers then this
whole argument would be moot. we're have efl_add behave like efl_add_ref all
the time and require all objects handles to be unique in scope and have some
macro to declare the object handle with the above attributes and you have to
compile with -fexceptions to handle c++ exception unwinding if it goes through
some c code along the way but as i said. it's non-standard. we can't rely
on it.

> Also, @own tags must _not_ relate to parent_set, because that
> has no useful information for tags or users, if needed we can
> add a @reparent tag, but that's not really special information
> such as real owernship information.
> 
> So, #4 on the original OP should be treated as a bug if we
> consider efl_add as efl_add_scope, but we also need
> to fix parent_set/parent_unset. And code must not unref
> more than they ref.

that was my take too. it should be unrefed only once. it smells of a bug to me.
i have very rarely played (i think just once or twice) with efl_add_ref(). i've
just use efl_ref() and efl_unref().

> Regards,
> 
> 
> On Tue, Dec 26, 2017 at 10:21 AM, Andrew Williams 
> wrote:
> > In the books I have read they consider null being a special case of
> > behaviour within a method just as confusing as passing a bool like in your
> > illustration.
> >
> > Andy
> >
> > On Tue, 26 Dec 2017 at 12:19, Andrew Williams  wrote:
> >
> >> Hi,
> >>
> >> With the proposal of efl_add and efl_add_child we remove the need for
> >> efl_add_ref* as the result of the former becomes consistent in its return
> >> of owned or not owned references.
> >> Hopefully Cedric can confirm this as I don’t know the spec.
> >> Right now we have a second one because the first is not consistently
> >> returning pointers that either do or do not need to be unrefd.
> >>
> >> Andy
> >> On Tue, 26 Dec 2017 at 04:25, Carsten Haitzler 
> >> wrote:
> >>
> >>> On Sun, 24 Dec 2017 09:16:08 + Andrew Williams 
> >>> said:
> >>>
> >>> > Are you trolling me now?
> >>>
> >>> no. you said its inconsistent. it's consistent. it has a simple rule.
> >>>
> >>> 

Re: [E-devel] efl_add causing confusion

[Carsten Haitzler]

On Tue, 26 Dec 2017 12:19:30 + Andrew Williams  said:

> Hi,
> 
> With the proposal of efl_add and efl_add_child we remove the need for
> efl_add_ref* as the result of the former becomes consistent in its return
> of owned or not owned references.

no. it's needed for bindings (c++, js, lua etc.) because in c++ with raii an
unref is called on exiting scope, and lua.js will do so with a gc that will
discovered the orphaned scope and unref.

> Hopefully Cedric can confirm this as I don’t know the spec.
> Right now we have a second one because the first is not consistently
> returning pointers that either do or do not need to be unrefd.

then efl_add_child is not consistent with efl_add. this is just as confusing as
parent being NULL or not. but now we need 4 "add" constructors because we still
need efl_add_ref as above.

> Andy
> On Tue, 26 Dec 2017 at 04:25, Carsten Haitzler  wrote:
> 
> > On Sun, 24 Dec 2017 09:16:08 + Andrew Williams 
> > said:
> >
> > > Are you trolling me now?
> >
> > no. you said its inconsistent. it's consistent. it has a simple rule.
> >
> > http://www.dictionary.com/browse/consistent
> >
> > it consistently adheres to the same principles. i described them.
> >
> > > A method that does two different things depending on some magic value /
> >
> > it's not a MAGIC value. it's a parent object handle. it's far from magic.
> > it's
> > "put this object into THIS box here, or into NO box and give it to me"
> > based on
> > that option.
> >
> > > null flag is a code smell (see Clean Coders if this is not familiar).
> > > Consider this method:
> > >
> > > ptr get_mem(string poolname, long bytes) {
> > > If (poolname == NULL)
> > > return malloc(bytes); // MUST be freed
> > > else
> > > return get_pool(poolname).borrow(bytes); // must NOT be freed
> > > }
> > >
> > > Do you think that is consistent? The user is not sure without inspecting
> > > the parameter contents whether or not the should free(). This is
> > > conceptually what we are setting up.
> >
> > #define efl_add_noparent(klass, ...) efl_add(klass, NULL, ## __VA_ARGS__)
> >
> > happy? you can have a macro to hide the parent if NULL. but it'll be used
> > fairly rarely.
> >
> > > Back to our efl_add - what would be consistent is this:
> > >
> > > Eo* efl_add(klass, ... constructors ...); // must be unrefd (no parent)
> > >
> > > Eo* efl_add_child(klass, parent, ... constructors ... ); // parent must
> > not
> > > be null, should not be unrefd
> > >
> > > That is consistent. It is also compliant with the V7 vote. It still has
> > the
> > > race condition but is much easier to read. I know from the method names
> > > what is expected.
> >
> > your proposal was to have efl_add return void. the above is better for
> > sure. i
> > see we were walking down similar paths.
> >
> > i still dislike the above because it just makes the api more verbose for
> > the
> > sake of special-casing "parent == NULL". i dislike it. this isn't a magic
> > "bool" that turns on or off behaviour. that is actually not great. you
> > read code
> > like
> >
> > func_do_something(obj, EINA_TRUE, EINA_FALSE, EINA_TRUE);
> >
> > ... and what are the 3 bools? it's not clear or obvious. but with a
> > constructor
> > like efl_add firstly it's incredibly common so it's something that will be
> > learned very quickly, and the typing already tells you it's an object
> > handle.
> > and you should have learned already that it's the parent object (or no
> > parent
> > at all if NULL).
> >
> > why do i dislike it? we now go from 2 constructors (efl_add and
> > efl_add_ref) to
> > 4 (efl_add, efl_child_add, efl_add_ref, efl_child_add_ref). i dislike this
> > "explosion" just to hide the parent arg being NULL.
> >
> > > Thoughts?
> > > On Sun, 24 Dec 2017 at 03:33, Carsten Haitzler 
> > wrote:
> > >
> > > > On Sat, 23 Dec 2017 11:30:58 + Andrew Williams <
> > a...@andywilliams.me>
> > > > said:
> > > >
> > > > > Hi,
> > > > >
> > > > > As this thread seems to be descending into word games and (insert
> > > > > appropriate word) contests I will reiterate my concern:
> > > > >
> > > > > efl_add is inconsistent and that should be addressed.
> > > >
> > > > do it's not. i explained already that it is not. i'll repeat again.
> > it's
> > > > consistent:
> > > >
> > > > if parent == valid object, then ref is owned by parent
> > > > else ref is owned by caller/scope.
> > > >
> > > > that is consistent.
> > > >
> > > > > I hope that is clear enough
> > > > > Andy
> > > > >
> > > > >
> > > > > On Thu, 21 Dec 2017 at 13:15, Andrew Williams 
> > > > wrote:
> > > > >
> > > > > > Hi,
> > > > > >
> > > > > > This is now well documented (
> > > > > > https://www.enlightenment.org/develop/tutorials/c/eo-refcount.md)
> > but
> > > > the
> > > > > > more I use efl_add the more I feel it is confusing especially to
> > new
> > > > > > developers.
> > > > > >
> > > > > > In 

Re: [E-devel] efl_add causing confusion

[Felipe Magno de Almeida]

JP, Cedric, me and TAsn have been having this argument for a while.

IMO, the whole problem is that we're thinking of ownership in terms
of parentship, which is wrong with reference counting. Ownership
is shared between all reference owners and that's it. That's also
the only sane way for bindings to work without having references
being pull under their feet.

The whole efl_del argument just exist because it is kinda poorly
named. IMO, del means: get this object to an "empty" state.
Just like close to files and hide and unparent to UI objects. efl_del
should not steal references under people who owns it, the object
would get deleted at a later time when everybody using the object
stops doing so, we could even return errors from efl_del'eted
objects for methods that do not make sense anymore, causing
most actions to, possibly, halt earlier rather than later.

IMO, the whole problem with efl_add/efl_add_ref is that
"parents" are treated specially, which they should not. parent_set
should increment efl_ref and parent_unset should decrement it.

For C, OTH, where we do expect some "automatism" on resource
handling, efl_unref'ing may be too much of a hassle when a
parent is already going to handle the lifetime of the object. So,
it would make sense, IMO, for efl_add_scope. It could even be
that efl_add_scope is named efl_add, no problem, as long as
there's a efl_add that keeps this semantics for binding
development. Which also means that parent_set/unset must
be fixed.

Also, @own tags must _not_ relate to parent_set, because that
has no useful information for tags or users, if needed we can
add a @reparent tag, but that's not really special information
such as real owernship information.

So, #4 on the original OP should be treated as a bug if we
consider efl_add as efl_add_scope, but we also need
to fix parent_set/parent_unset. And code must not unref
more than they ref.

Regards,


On Tue, Dec 26, 2017 at 10:21 AM, Andrew Williams  wrote:
> In the books I have read they consider null being a special case of
> behaviour within a method just as confusing as passing a bool like in your
> illustration.
>
> Andy
>
> On Tue, 26 Dec 2017 at 12:19, Andrew Williams  wrote:
>
>> Hi,
>>
>> With the proposal of efl_add and efl_add_child we remove the need for
>> efl_add_ref* as the result of the former becomes consistent in its return
>> of owned or not owned references.
>> Hopefully Cedric can confirm this as I don’t know the spec.
>> Right now we have a second one because the first is not consistently
>> returning pointers that either do or do not need to be unrefd.
>>
>> Andy
>> On Tue, 26 Dec 2017 at 04:25, Carsten Haitzler 
>> wrote:
>>
>>> On Sun, 24 Dec 2017 09:16:08 + Andrew Williams 
>>> said:
>>>
>>> > Are you trolling me now?
>>>
>>> no. you said its inconsistent. it's consistent. it has a simple rule.
>>>
>>> http://www.dictionary.com/browse/consistent
>>>
>>> it consistently adheres to the same principles. i described them.
>>>
>>> > A method that does two different things depending on some magic value /
>>>
>>> it's not a MAGIC value. it's a parent object handle. it's far from magic.
>>> it's
>>> "put this object into THIS box here, or into NO box and give it to me"
>>> based on
>>> that option.
>>>
>>> > null flag is a code smell (see Clean Coders if this is not familiar).
>>> > Consider this method:
>>> >
>>> > ptr get_mem(string poolname, long bytes) {
>>> > If (poolname == NULL)
>>> > return malloc(bytes); // MUST be freed
>>> > else
>>> > return get_pool(poolname).borrow(bytes); // must NOT be freed
>>> > }
>>> >
>>> > Do you think that is consistent? The user is not sure without inspecting
>>> > the parameter contents whether or not the should free(). This is
>>> > conceptually what we are setting up.
>>>
>>> #define efl_add_noparent(klass, ...) efl_add(klass, NULL, ## __VA_ARGS__)
>>>
>>> happy? you can have a macro to hide the parent if NULL. but it'll be used
>>> fairly rarely.
>>>
>>> > Back to our efl_add - what would be consistent is this:
>>> >
>>> > Eo* efl_add(klass, ... constructors ...); // must be unrefd (no parent)
>>> >
>>> > Eo* efl_add_child(klass, parent, ... constructors ... ); // parent must
>>> not
>>> > be null, should not be unrefd
>>> >
>>> > That is consistent. It is also compliant with the V7 vote. It still has
>>> the
>>> > race condition but is much easier to read. I know from the method names
>>> > what is expected.
>>>
>>> your proposal was to have efl_add return void. the above is better for
>>> sure. i
>>> see we were walking down similar paths.
>>>
>>> i still dislike the above because it just makes the api more verbose for
>>> the
>>> sake of special-casing "parent == NULL". i dislike it. this isn't a magic
>>> "bool" that turns on or off behaviour. that is actually not great. you
>>> read code
>>> like
>>>
>>> func_do_something(obj, EINA_TRUE, EINA_FALSE, EINA_TRUE);
>>>
>>> 

Re: [E-devel] efl_add causing confusion

[Andrew Williams]

In the books I have read they consider null being a special case of
behaviour within a method just as confusing as passing a bool like in your
illustration.

Andy

On Tue, 26 Dec 2017 at 12:19, Andrew Williams  wrote:

> Hi,
>
> With the proposal of efl_add and efl_add_child we remove the need for
> efl_add_ref* as the result of the former becomes consistent in its return
> of owned or not owned references.
> Hopefully Cedric can confirm this as I don’t know the spec.
> Right now we have a second one because the first is not consistently
> returning pointers that either do or do not need to be unrefd.
>
> Andy
> On Tue, 26 Dec 2017 at 04:25, Carsten Haitzler 
> wrote:
>
>> On Sun, 24 Dec 2017 09:16:08 + Andrew Williams 
>> said:
>>
>> > Are you trolling me now?
>>
>> no. you said its inconsistent. it's consistent. it has a simple rule.
>>
>> http://www.dictionary.com/browse/consistent
>>
>> it consistently adheres to the same principles. i described them.
>>
>> > A method that does two different things depending on some magic value /
>>
>> it's not a MAGIC value. it's a parent object handle. it's far from magic.
>> it's
>> "put this object into THIS box here, or into NO box and give it to me"
>> based on
>> that option.
>>
>> > null flag is a code smell (see Clean Coders if this is not familiar).
>> > Consider this method:
>> >
>> > ptr get_mem(string poolname, long bytes) {
>> > If (poolname == NULL)
>> > return malloc(bytes); // MUST be freed
>> > else
>> > return get_pool(poolname).borrow(bytes); // must NOT be freed
>> > }
>> >
>> > Do you think that is consistent? The user is not sure without inspecting
>> > the parameter contents whether or not the should free(). This is
>> > conceptually what we are setting up.
>>
>> #define efl_add_noparent(klass, ...) efl_add(klass, NULL, ## __VA_ARGS__)
>>
>> happy? you can have a macro to hide the parent if NULL. but it'll be used
>> fairly rarely.
>>
>> > Back to our efl_add - what would be consistent is this:
>> >
>> > Eo* efl_add(klass, ... constructors ...); // must be unrefd (no parent)
>> >
>> > Eo* efl_add_child(klass, parent, ... constructors ... ); // parent must
>> not
>> > be null, should not be unrefd
>> >
>> > That is consistent. It is also compliant with the V7 vote. It still has
>> the
>> > race condition but is much easier to read. I know from the method names
>> > what is expected.
>>
>> your proposal was to have efl_add return void. the above is better for
>> sure. i
>> see we were walking down similar paths.
>>
>> i still dislike the above because it just makes the api more verbose for
>> the
>> sake of special-casing "parent == NULL". i dislike it. this isn't a magic
>> "bool" that turns on or off behaviour. that is actually not great. you
>> read code
>> like
>>
>> func_do_something(obj, EINA_TRUE, EINA_FALSE, EINA_TRUE);
>>
>> ... and what are the 3 bools? it's not clear or obvious. but with a
>> constructor
>> like efl_add firstly it's incredibly common so it's something that will be
>> learned very quickly, and the typing already tells you it's an object
>> handle.
>> and you should have learned already that it's the parent object (or no
>> parent
>> at all if NULL).
>>
>> why do i dislike it? we now go from 2 constructors (efl_add and
>> efl_add_ref) to
>> 4 (efl_add, efl_child_add, efl_add_ref, efl_child_add_ref). i dislike this
>> "explosion" just to hide the parent arg being NULL.
>>
>> > Thoughts?
>> > On Sun, 24 Dec 2017 at 03:33, Carsten Haitzler 
>> wrote:
>> >
>> > > On Sat, 23 Dec 2017 11:30:58 + Andrew Williams <
>> a...@andywilliams.me>
>> > > said:
>> > >
>> > > > Hi,
>> > > >
>> > > > As this thread seems to be descending into word games and (insert
>> > > > appropriate word) contests I will reiterate my concern:
>> > > >
>> > > > efl_add is inconsistent and that should be addressed.
>> > >
>> > > do it's not. i explained already that it is not. i'll repeat again.
>> it's
>> > > consistent:
>> > >
>> > > if parent == valid object, then ref is owned by parent
>> > > else ref is owned by caller/scope.
>> > >
>> > > that is consistent.
>> > >
>> > > > I hope that is clear enough
>> > > > Andy
>> > > >
>> > > >
>> > > > On Thu, 21 Dec 2017 at 13:15, Andrew Williams > >
>> > > wrote:
>> > > >
>> > > > > Hi,
>> > > > >
>> > > > > This is now well documented (
>> > > > > https://www.enlightenment.org/develop/tutorials/c/eo-refcount.md)
>> but
>> > > the
>> > > > > more I use efl_add the more I feel it is confusing especially to
>> new
>> > > > > developers.
>> > > > >
>> > > > > In the current model (if I understand it correctly)
>> > > > > 1) child = efl_add(klass, parent) means the child must NOT be
>> > > unfeferenced
>> > > > > 2) child = efl_add(klass, NULL) means the child should be
>> unreferenced
>> > > > > 3) child = efl_add_ref(klass, parent) means the child must be
>> > > unreferenced
>> > > > > 4) 

Re: [E-devel] efl_add causing confusion

[Andrew Williams]

Hi,

With the proposal of efl_add and efl_add_child we remove the need for
efl_add_ref* as the result of the former becomes consistent in its return
of owned or not owned references.
Hopefully Cedric can confirm this as I don’t know the spec.
Right now we have a second one because the first is not consistently
returning pointers that either do or do not need to be unrefd.

Andy
On Tue, 26 Dec 2017 at 04:25, Carsten Haitzler  wrote:

> On Sun, 24 Dec 2017 09:16:08 + Andrew Williams 
> said:
>
> > Are you trolling me now?
>
> no. you said its inconsistent. it's consistent. it has a simple rule.
>
> http://www.dictionary.com/browse/consistent
>
> it consistently adheres to the same principles. i described them.
>
> > A method that does two different things depending on some magic value /
>
> it's not a MAGIC value. it's a parent object handle. it's far from magic.
> it's
> "put this object into THIS box here, or into NO box and give it to me"
> based on
> that option.
>
> > null flag is a code smell (see Clean Coders if this is not familiar).
> > Consider this method:
> >
> > ptr get_mem(string poolname, long bytes) {
> > If (poolname == NULL)
> > return malloc(bytes); // MUST be freed
> > else
> > return get_pool(poolname).borrow(bytes); // must NOT be freed
> > }
> >
> > Do you think that is consistent? The user is not sure without inspecting
> > the parameter contents whether or not the should free(). This is
> > conceptually what we are setting up.
>
> #define efl_add_noparent(klass, ...) efl_add(klass, NULL, ## __VA_ARGS__)
>
> happy? you can have a macro to hide the parent if NULL. but it'll be used
> fairly rarely.
>
> > Back to our efl_add - what would be consistent is this:
> >
> > Eo* efl_add(klass, ... constructors ...); // must be unrefd (no parent)
> >
> > Eo* efl_add_child(klass, parent, ... constructors ... ); // parent must
> not
> > be null, should not be unrefd
> >
> > That is consistent. It is also compliant with the V7 vote. It still has
> the
> > race condition but is much easier to read. I know from the method names
> > what is expected.
>
> your proposal was to have efl_add return void. the above is better for
> sure. i
> see we were walking down similar paths.
>
> i still dislike the above because it just makes the api more verbose for
> the
> sake of special-casing "parent == NULL". i dislike it. this isn't a magic
> "bool" that turns on or off behaviour. that is actually not great. you
> read code
> like
>
> func_do_something(obj, EINA_TRUE, EINA_FALSE, EINA_TRUE);
>
> ... and what are the 3 bools? it's not clear or obvious. but with a
> constructor
> like efl_add firstly it's incredibly common so it's something that will be
> learned very quickly, and the typing already tells you it's an object
> handle.
> and you should have learned already that it's the parent object (or no
> parent
> at all if NULL).
>
> why do i dislike it? we now go from 2 constructors (efl_add and
> efl_add_ref) to
> 4 (efl_add, efl_child_add, efl_add_ref, efl_child_add_ref). i dislike this
> "explosion" just to hide the parent arg being NULL.
>
> > Thoughts?
> > On Sun, 24 Dec 2017 at 03:33, Carsten Haitzler 
> wrote:
> >
> > > On Sat, 23 Dec 2017 11:30:58 + Andrew Williams <
> a...@andywilliams.me>
> > > said:
> > >
> > > > Hi,
> > > >
> > > > As this thread seems to be descending into word games and (insert
> > > > appropriate word) contests I will reiterate my concern:
> > > >
> > > > efl_add is inconsistent and that should be addressed.
> > >
> > > do it's not. i explained already that it is not. i'll repeat again.
> it's
> > > consistent:
> > >
> > > if parent == valid object, then ref is owned by parent
> > > else ref is owned by caller/scope.
> > >
> > > that is consistent.
> > >
> > > > I hope that is clear enough
> > > > Andy
> > > >
> > > >
> > > > On Thu, 21 Dec 2017 at 13:15, Andrew Williams 
> > > wrote:
> > > >
> > > > > Hi,
> > > > >
> > > > > This is now well documented (
> > > > > https://www.enlightenment.org/develop/tutorials/c/eo-refcount.md)
> but
> > > the
> > > > > more I use efl_add the more I feel it is confusing especially to
> new
> > > > > developers.
> > > > >
> > > > > In the current model (if I understand it correctly)
> > > > > 1) child = efl_add(klass, parent) means the child must NOT be
> > > unfeferenced
> > > > > 2) child = efl_add(klass, NULL) means the child should be
> unreferenced
> > > > > 3) child = efl_add_ref(klass, parent) means the child must be
> > > unreferenced
> > > > > 4) child = efl_add_ref(klass, NULL) somehow means that the child
> > > should be
> > > > > unreferenced twice
> > > > >
> > > > > In my opinion 1) and 4) are peculiar and so I provide a proposal
> to fix
> > > > > this:
> > > > >
> > > > > We could change efl_add to return void. It never retains a
> reference.
> > > If
> > > > > the parent is NULL then it should be automatically unref before
> > > 

Re: [E-devel] efl_add causing confusion

[Carsten Haitzler]

On Sun, 24 Dec 2017 09:23:16 + Andrew Williams  said:

> Hi,
> 
> Yes invalid access is easier to debug than a memory lean, in general.
> In this situation we are talking about the possible consistent memory leaks
> vs the occasional (race condition) undefined access. The latter is not
> easier to debug.

you just said it was easier to debug, then you say it isn't... eh? and you
agree with me that leaks will be more common than invalid accesses, yet you
want  to propose a method that has a greater percentage of problems (and i
argue a greater consequence when those issues arise).

> And if we did find the line at error it would look (using the designed
> efl_add behaviour) completely reasonable - what would the fix even be?

the fact a parent has deleted an object "unexpectedly" is the issue. why is it
unexpected? you do this:

obj = efl_add(class, parent);
efl_del(parent);
efl_method_x(obj); // invalid access here

it'll be an invalid object access. well obviously. parent deleted it's child. i
would not class this as unexpected. if it's defined behaviour like

obj = efl_add(class, parent);
efl_do_something_abort(parent);
efl_method_x(obj); // invalid access here

then doing "so_something_abort" on the parent causes children to be deleted.
maybe it was a bad choice to have this kind of behaviour? perhaps it is the
right thing? check the api docs.

but in the C language SCOPE does NOT guarantee a reference to an object because
C has no such scoping rules. just because i have a struct x * in my scope
doesn't mean something might not free it while i am running. it is no js or c#
or java etc. ... it's C. even C++ doesn't guarantee this unless you explicitly
use an RAII pattern. This is the nature of the language. trying to force other
language models on it will lead to more verbose code, with manual need to
remember to unref and thus more likely chances of things going wrong. i'm
repeating myself.

but still. the vast majority of developers disagree with you. your proposal is
to have to unref a handle if you want to have a handle to do something with it
which is exactly what was voted on. if *YOU* wish to do this... it exists.
efl_add_ref(). most developers disagree with you and would advise against. that
still doesn't even address the fact that a change at this stage to what you
propose would be insanely disruptive to the efl tree, development in general
and lead to bugs needing to be cleaned up for the next 6-12 or even more months
as a result of it. but the decision was made with a vastly overwhelming
majority disagreeing with you. your "but they didn't consider null parents" is
bogus. of course null parents are considered because they HAVE to be to create
toplevel objects anyway. your proposal means needing efl_add_ref whenever you
need the obj handle and that was voted on as "bad".

> Andy
> 
> On Sun, 24 Dec 2017 at 03:33, Carsten Haitzler  wrote:
> 
> > On Sat, 23 Dec 2017 11:19:59 + Andrew Williams 
> > said:
> >
> > > As framework developers we should be doing everything in our power to
> > avoid
> > > undefined behaviour. I’ll take accidents memory leaks any day - there
> > are a
> > > plethora of tools to solve that but it’s far harder to debug the “app not
> > > working correctly sometimes on my system” bug reports.
> >
> > actually no. we should be doing everything we can to make things EASY.
> > avoiding
> > undefined behaviour is one way of making things easier. having to write
> > less
> > code and it be HARDER to write things that have bad side-effects is also
> > making
> > things easier.
> >
> > if you don't leak large amounts of objects you will not have tools find it
> > easy
> > to find such leaks. if you leak LOTs (1000's or more objects in one place)
> > then
> > you can probably find it by spotting an excessive allocation point in
> > something
> > like massif, BUT nothing will detect a leaked pointer, because eoid
> > maintains a
> > pointer to the object in the eoid object table. the pointer is actually
> > never
> > lost. it's tracked by eo. so no. leak check tools won't work. maybe "still
> > allocated on shutdown" might unless you have a LOT of that data which is a
> > lot
> > of work to ensure is done. there are lots of allocations on exit from efl,
> > glibc and other places already (that are harmless).
> >
> > invalid access as i explained is directly ind immediately debuggable.
> > leaks -
> > not so much.
> >
> > but regardless of that the votes already were vastly for the thing we have
> > today.
> >
> > > Andy
> > >
> > > On Sat, 23 Dec 2017 at 10:56, Carsten Haitzler 
> > wrote:
> > >
> > > > On Fri, 22 Dec 2017 11:53:32 + Andrew Williams <
> > a...@andywilliams.me>
> > > > said:
> > > >
> > > > > We do have a showstopper design bug.
> > > >
> > > > see my previous mail. https://phab.enlightenment.org/V7 to be
> > explicit.
> > > > discussed. voted 

Re: [E-devel] efl_add causing confusion

[Carsten Haitzler]

On Sun, 24 Dec 2017 09:16:08 + Andrew Williams  said:

> Are you trolling me now?

no. you said its inconsistent. it's consistent. it has a simple rule.

http://www.dictionary.com/browse/consistent

it consistently adheres to the same principles. i described them.

> A method that does two different things depending on some magic value /

it's not a MAGIC value. it's a parent object handle. it's far from magic. it's
"put this object into THIS box here, or into NO box and give it to me" based on
that option.

> null flag is a code smell (see Clean Coders if this is not familiar).
> Consider this method:
> 
> ptr get_mem(string poolname, long bytes) {
> If (poolname == NULL)
> return malloc(bytes); // MUST be freed
> else
> return get_pool(poolname).borrow(bytes); // must NOT be freed
> }
> 
> Do you think that is consistent? The user is not sure without inspecting
> the parameter contents whether or not the should free(). This is
> conceptually what we are setting up.

#define efl_add_noparent(klass, ...) efl_add(klass, NULL, ## __VA_ARGS__)

happy? you can have a macro to hide the parent if NULL. but it'll be used
fairly rarely.

> Back to our efl_add - what would be consistent is this:
> 
> Eo* efl_add(klass, ... constructors ...); // must be unrefd (no parent)
> 
> Eo* efl_add_child(klass, parent, ... constructors ... ); // parent must not
> be null, should not be unrefd
> 
> That is consistent. It is also compliant with the V7 vote. It still has the
> race condition but is much easier to read. I know from the method names
> what is expected.

your proposal was to have efl_add return void. the above is better for sure. i
see we were walking down similar paths.

i still dislike the above because it just makes the api more verbose for the
sake of special-casing "parent == NULL". i dislike it. this isn't a magic
"bool" that turns on or off behaviour. that is actually not great. you read code
like

func_do_something(obj, EINA_TRUE, EINA_FALSE, EINA_TRUE);

... and what are the 3 bools? it's not clear or obvious. but with a constructor
like efl_add firstly it's incredibly common so it's something that will be
learned very quickly, and the typing already tells you it's an object handle.
and you should have learned already that it's the parent object (or no parent
at all if NULL).

why do i dislike it? we now go from 2 constructors (efl_add and efl_add_ref) to
4 (efl_add, efl_child_add, efl_add_ref, efl_child_add_ref). i dislike this
"explosion" just to hide the parent arg being NULL.

> Thoughts?
> On Sun, 24 Dec 2017 at 03:33, Carsten Haitzler  wrote:
> 
> > On Sat, 23 Dec 2017 11:30:58 + Andrew Williams 
> > said:
> >
> > > Hi,
> > >
> > > As this thread seems to be descending into word games and (insert
> > > appropriate word) contests I will reiterate my concern:
> > >
> > > efl_add is inconsistent and that should be addressed.
> >
> > do it's not. i explained already that it is not. i'll repeat again. it's
> > consistent:
> >
> > if parent == valid object, then ref is owned by parent
> > else ref is owned by caller/scope.
> >
> > that is consistent.
> >
> > > I hope that is clear enough
> > > Andy
> > >
> > >
> > > On Thu, 21 Dec 2017 at 13:15, Andrew Williams 
> > wrote:
> > >
> > > > Hi,
> > > >
> > > > This is now well documented (
> > > > https://www.enlightenment.org/develop/tutorials/c/eo-refcount.md) but
> > the
> > > > more I use efl_add the more I feel it is confusing especially to new
> > > > developers.
> > > >
> > > > In the current model (if I understand it correctly)
> > > > 1) child = efl_add(klass, parent) means the child must NOT be
> > unfeferenced
> > > > 2) child = efl_add(klass, NULL) means the child should be unreferenced
> > > > 3) child = efl_add_ref(klass, parent) means the child must be
> > unreferenced
> > > > 4) child = efl_add_ref(klass, NULL) somehow means that the child
> > should be
> > > > unreferenced twice
> > > >
> > > > In my opinion 1) and 4) are peculiar and so I provide a proposal to fix
> > > > this:
> > > >
> > > > We could change efl_add to return void. It never retains a reference.
> > If
> > > > the parent is NULL then it should be automatically unref before
> > returning.
> > > > Then efl_add_ref would be changed to mirror this and always retain
> > exactly
> > > > 1 reference - so if parent is NULL there is no double count returned.
> > > >
> > > > Using this model if an Eo * is returned then I know I have a reference
> > and
> > > > must later unref.
> > > > Any need for using the pointer in an efl_add (which is no longer
> > returned)
> > > > would still be supported through efl_added within the constructor.
> > > >
> > > > What do people think about this? I put the suggestion forward to
> > improve
> > > > the symettry with add and unref which is currently confusing. If my
> > > > assumptions above are incorrect please let me know!
> > > >
> > > > Thanks,
> > > 

Re: [E-devel] efl_add causing confusion

[Carsten Haitzler]

On Sun, 24 Dec 2017 09:20:33 + Andrew Williams  said:

> The proposal has 0 occurrences of “NULL”.

it has api examples with parent provided on creation. obviously some objects
will not provide a parent (e.g. a toplevel of a tree that has no parents).

> Andy
> 
> On Sun, 24 Dec 2017 at 03:33, Carsten Haitzler  wrote:
> 
> > On Sat, 23 Dec 2017 11:26:42 + Andrew Williams 
> > said:
> >
> > > Hi,
> > >
> > > Thanks for posting that poll. Very useful.
> > > If I may I wish to contradict your assertion that this has all been
> > > discussed before - the poll had *0* consideration for what if parent is
> > > NULL.
> >
> > i don't see why it needed to. NULL == no parent, thus floating ref.
> > non-NULL
> > and the ref is immediately handed to the parent object. (assuming non-null
> > is
> > also a valid object).
> >
> > > If you re-read the accepted proposal you will see that is not the current
> > > behaviour at all. There is a (relatively) explicit handing of the
> > reference
> > > which can occur *after* I am finished with the reference.
> > > In short the proposal that was voted for did not have this race condition
> > > by design.
> >
> > the proposal of course considers NULL. how do you create the win? at the
> > time
> > we had no loop object or had yet discussed it, so windows would have been
> > created with a NULL parent.
> >
> > the core is the same. you want efl_add to return nothing (void) which
> > makes it
> > useless for creating a container (win, box, button, etc. etc.) which is a
> > huge
> > use case of objects at construction etc. ... which means we'd need to use
> > efl_add_ref() instead. or something that is the same (and you say this too
> > saying you then need to unref). and the whole idea of having to unref at
> > end of
> > scope during creation was voted against. the same arguments i have been
> > making
> > - it's less error prone to not have to unref. it's commonly seen enough in
> > c
> > (gtk does it for example) etc. etc.
> >
> > > And so it follows that we are currently working with a third option that
> > > was not part of the vote.
> >
> > i don't see it that way. it's the same. if you remove the return from
> > efl_add()
> > (returns void and deletes the object if parent is NULL) then the proposal
> > you
> > have is to use efl_add_Ref for all these cases where you do need the
> > return and
> > that is what was voted on already. even if today > 50% of devs voted for
> > this
> > i'd still say "don't change" due to the sheer cost of change as i've
> > explained.
> > my answer is "use efl_add_ref then if this bothers you and remember to
> > unref,
> > but now you are the one holding the bag. we advised against this by
> > default".
> > the thing you want is there d available for use. i and most developers
> > would
> > advise against it (see vote).
> >
> > > Same with gtk, apologies for misreading initially, they hand the
> > reference
> > > over when they are done.
> >
> > correct. and we hand it over "in one go" in efl_add. it amounts to the
> > same.
> >
> > > One last thought - safety is broader than not crashing. If we have put in
> > > place methods to catch corner cases in our design then perhaps our design
> > > is not solid enough?
> >
> > eoid catches this case not because it was meant to catch the corner case
> > you
> > speak of (parent deletes child while you still have the object handle in
> > scope) or other corner cases, but because of a more general problem of
> > lots of
> > problems with developers unable to understand magic checks with invalid
> > object
> > refs in general (mostly far far far later than scope usage), and simply
> > handing
> > off the bug as "an efl bug" because it crashed inside an efl function (even
> > though the crash was in the magic check which de referenced a pointer
> > pointing
> > out of memory space). it happens to be a very safe mechanism to access our
> > objects and it happens to deal with the case you speak of.
> >
> > > Andy
> > >
> > > On Sat, 23 Dec 2017 at 10:56, Carsten Haitzler 
> > wrote:
> > >
> > > > On Fri, 22 Dec 2017 11:44:32 + Andrew Williams <
> > a...@andywilliams.me>
> > > > said:
> > > >
> > > > > Hi,
> > > > >
> > > > > I think your summary about the Gtk is not what you think - read the
> > docs
> > > > > further
> > > > >
> > https://developer.gnome.org/gtk3/stable/GtkWindow.html#gtk-window-new
> > > > and
> > > > > you see that actually the trivial example leaks the reference - just
> > like
> > > > > an efl_add_ref with no unref or del later.
> > > > > They are *not* returning a pointer where you can expect to lose
> > ownership
> > > > > at some future time. Notice also they do not pass parents to the
> > > > > constructors.
> > > >
> > > > ummm no. that's wrong. because if you destroy the gtk window that you
> > > > added the
> > > > button and box to... the button is deleted and it DOESNT LEAK. the
> 

Re: [E-devel] efl_add causing confusion

[Andrew Williams]

Hi,

Yes invalid access is easier to debug than a memory lean, in general.
In this situation we are talking about the possible consistent memory leaks
vs the occasional (race condition) undefined access. The latter is not
easier to debug.

And if we did find the line at error it would look (using the designed
efl_add behaviour) completely reasonable - what would the fix even be?

Andy

On Sun, 24 Dec 2017 at 03:33, Carsten Haitzler  wrote:

> On Sat, 23 Dec 2017 11:19:59 + Andrew Williams 
> said:
>
> > As framework developers we should be doing everything in our power to
> avoid
> > undefined behaviour. I’ll take accidents memory leaks any day - there
> are a
> > plethora of tools to solve that but it’s far harder to debug the “app not
> > working correctly sometimes on my system” bug reports.
>
> actually no. we should be doing everything we can to make things EASY.
> avoiding
> undefined behaviour is one way of making things easier. having to write
> less
> code and it be HARDER to write things that have bad side-effects is also
> making
> things easier.
>
> if you don't leak large amounts of objects you will not have tools find it
> easy
> to find such leaks. if you leak LOTs (1000's or more objects in one place)
> then
> you can probably find it by spotting an excessive allocation point in
> something
> like massif, BUT nothing will detect a leaked pointer, because eoid
> maintains a
> pointer to the object in the eoid object table. the pointer is actually
> never
> lost. it's tracked by eo. so no. leak check tools won't work. maybe "still
> allocated on shutdown" might unless you have a LOT of that data which is a
> lot
> of work to ensure is done. there are lots of allocations on exit from efl,
> glibc and other places already (that are harmless).
>
> invalid access as i explained is directly ind immediately debuggable.
> leaks -
> not so much.
>
> but regardless of that the votes already were vastly for the thing we have
> today.
>
> > Andy
> >
> > On Sat, 23 Dec 2017 at 10:56, Carsten Haitzler 
> wrote:
> >
> > > On Fri, 22 Dec 2017 11:53:32 + Andrew Williams <
> a...@andywilliams.me>
> > > said:
> > >
> > > > We do have a showstopper design bug.
> > >
> > > see my previous mail. https://phab.enlightenment.org/V7 to be
> explicit.
> > > discussed. voted on. known compromise. lots of code built on top of
> that
> > > design
> > > decision. not a showstopper by any stretch. it's exactly what gtk
> does. the
> > > only difference is we have the "parent set" be part of the add and gtk
> > > does it
> > > as a separate step with container_add()
> > >
> > > > We are managing to work with it because we know how it works
> internally.
> > > > The API must be straight forward and provide consistent, robust
> options.
> > > >
> > > > Any new user would be asking of efl_add:
> > > > *) Do I need to unref the result when I am done?
> > > > *) How long is my returned reference valid for?
> > >
> > > see the vote. see gtk samples. floating reference.
> > >
> > > > Both of those are answered with "it depends" and that is a design
> > > > showstopper.
> > >
> > > disagree. it's clear. if parent is NULL then calling scope gets a
> > > floating reference you have to unref. if not then parent owns that
> > > reference and the rules of that parent cover lifetime of that object.
> > > covered
> > > that in prior mails - the alternative is to force people to manually
> unref
> > > every time they want a handle to something at all to do useful things
> with
> > > ...
> > > like add a child to this parent (child packed into a box parent for
> > > example).
> > > see the gtk/gobject design doc with the idea of floating references.
> see
> > > the
> > > previous discussion thread. see the vote in favor of what is there now
> and
> > > with
> > > everyone fully aware of scoping etc. and that is why efl_add_ref
> exists if
> > > you
> > > choose to use that.
> > >
> > > note for this discussion i am assuming scope is a small local function
> > > scope
> > > not larger. technically it can be the global scope of the entire app.
> > >
> > > i'll put it simply:
> > >
> > > 1: do it your way and people will have to always unref at end of scope
> (if
> > > they
> > > want an object handle to do something with, like pack a button into a
> box
> > > parent which is exceedingly common). the will at times forget to unref
> > > causing
> > > leaks.
> > >
> > > 2: do it the current way and perhaps sometimes in some rare
> circumstances
> > > an
> > > object is deleted by a parent before your scope ends.
> > >
> > > chances of 1: i'll put it at maybe 10-20% that someone forgets a scope
> > > exit case
> > > and forgets to unref (a return in the middle for example).
> > >
> > > chances of 2: i'll put at at maybe 1% on a bad day. likely far far
> less.
> > >
> > > so let's say 15% to 1%. 15x more mistakes likely with your proposal.
> but
> > > let's
> > > cover the 

Re: [E-devel] efl_add causing confusion

[Andrew Williams]

The proposal has 0 occurrences of “NULL”.

Andy

On Sun, 24 Dec 2017 at 03:33, Carsten Haitzler  wrote:

> On Sat, 23 Dec 2017 11:26:42 + Andrew Williams 
> said:
>
> > Hi,
> >
> > Thanks for posting that poll. Very useful.
> > If I may I wish to contradict your assertion that this has all been
> > discussed before - the poll had *0* consideration for what if parent is
> > NULL.
>
> i don't see why it needed to. NULL == no parent, thus floating ref.
> non-NULL
> and the ref is immediately handed to the parent object. (assuming non-null
> is
> also a valid object).
>
> > If you re-read the accepted proposal you will see that is not the current
> > behaviour at all. There is a (relatively) explicit handing of the
> reference
> > which can occur *after* I am finished with the reference.
> > In short the proposal that was voted for did not have this race condition
> > by design.
>
> the proposal of course considers NULL. how do you create the win? at the
> time
> we had no loop object or had yet discussed it, so windows would have been
> created with a NULL parent.
>
> the core is the same. you want efl_add to return nothing (void) which
> makes it
> useless for creating a container (win, box, button, etc. etc.) which is a
> huge
> use case of objects at construction etc. ... which means we'd need to use
> efl_add_ref() instead. or something that is the same (and you say this too
> saying you then need to unref). and the whole idea of having to unref at
> end of
> scope during creation was voted against. the same arguments i have been
> making
> - it's less error prone to not have to unref. it's commonly seen enough in
> c
> (gtk does it for example) etc. etc.
>
> > And so it follows that we are currently working with a third option that
> > was not part of the vote.
>
> i don't see it that way. it's the same. if you remove the return from
> efl_add()
> (returns void and deletes the object if parent is NULL) then the proposal
> you
> have is to use efl_add_Ref for all these cases where you do need the
> return and
> that is what was voted on already. even if today > 50% of devs voted for
> this
> i'd still say "don't change" due to the sheer cost of change as i've
> explained.
> my answer is "use efl_add_ref then if this bothers you and remember to
> unref,
> but now you are the one holding the bag. we advised against this by
> default".
> the thing you want is there d available for use. i and most developers
> would
> advise against it (see vote).
>
> > Same with gtk, apologies for misreading initially, they hand the
> reference
> > over when they are done.
>
> correct. and we hand it over "in one go" in efl_add. it amounts to the
> same.
>
> > One last thought - safety is broader than not crashing. If we have put in
> > place methods to catch corner cases in our design then perhaps our design
> > is not solid enough?
>
> eoid catches this case not because it was meant to catch the corner case
> you
> speak of (parent deletes child while you still have the object handle in
> scope) or other corner cases, but because of a more general problem of
> lots of
> problems with developers unable to understand magic checks with invalid
> object
> refs in general (mostly far far far later than scope usage), and simply
> handing
> off the bug as "an efl bug" because it crashed inside an efl function (even
> though the crash was in the magic check which de referenced a pointer
> pointing
> out of memory space). it happens to be a very safe mechanism to access our
> objects and it happens to deal with the case you speak of.
>
> > Andy
> >
> > On Sat, 23 Dec 2017 at 10:56, Carsten Haitzler 
> wrote:
> >
> > > On Fri, 22 Dec 2017 11:44:32 + Andrew Williams <
> a...@andywilliams.me>
> > > said:
> > >
> > > > Hi,
> > > >
> > > > I think your summary about the Gtk is not what you think - read the
> docs
> > > > further
> > > >
> https://developer.gnome.org/gtk3/stable/GtkWindow.html#gtk-window-new
> > > and
> > > > you see that actually the trivial example leaks the reference - just
> like
> > > > an efl_add_ref with no unref or del later.
> > > > They are *not* returning a pointer where you can expect to lose
> ownership
> > > > at some future time. Notice also they do not pass parents to the
> > > > constructors.
> > >
> > > ummm no. that's wrong. because if you destroy the gtk window that you
> > > added the
> > > button and box to... the button is deleted and it DOESNT LEAK. the
> button
> > > is
> > > destroyed and freed. ownership of the reference is TRANSFERRED to the
> > > parent
> > > (the window) by gtk_container_add. see there are no unrefs there. it's
> not
> > > the
> > > same.  if the window was deleted (implicitly or explicitly) within that
> > > scope
> > > the button object pointer would have become invalid as well.
> > >
> > > efl_add_ref will have the button LEAK and stay around forever until
> you do
> > > another unref on it as the 

Re: [E-devel] efl_add causing confusion

[Andrew Williams]

Are you trolling me now?

A method that does two different things depending on some magic value /
null flag is a code smell (see Clean Coders if this is not familiar).
Consider this method:

ptr get_mem(string poolname, long bytes) {
If (poolname == NULL)
return malloc(bytes); // MUST be freed
else
return get_pool(poolname).borrow(bytes); // must NOT be freed
}

Do you think that is consistent? The user is not sure without inspecting
the parameter contents whether or not the should free(). This is
conceptually what we are setting up.

Back to our efl_add - what would be consistent is this:

Eo* efl_add(klass, ... constructors ...); // must be unrefd (no parent)

Eo* efl_add_child(klass, parent, ... constructors ... ); // parent must not
be null, should not be unrefd

That is consistent. It is also compliant with the V7 vote. It still has the
race condition but is much easier to read. I know from the method names
what is expected.

Thoughts?
On Sun, 24 Dec 2017 at 03:33, Carsten Haitzler  wrote:

> On Sat, 23 Dec 2017 11:30:58 + Andrew Williams 
> said:
>
> > Hi,
> >
> > As this thread seems to be descending into word games and (insert
> > appropriate word) contests I will reiterate my concern:
> >
> > efl_add is inconsistent and that should be addressed.
>
> do it's not. i explained already that it is not. i'll repeat again. it's
> consistent:
>
> if parent == valid object, then ref is owned by parent
> else ref is owned by caller/scope.
>
> that is consistent.
>
> > I hope that is clear enough
> > Andy
> >
> >
> > On Thu, 21 Dec 2017 at 13:15, Andrew Williams 
> wrote:
> >
> > > Hi,
> > >
> > > This is now well documented (
> > > https://www.enlightenment.org/develop/tutorials/c/eo-refcount.md) but
> the
> > > more I use efl_add the more I feel it is confusing especially to new
> > > developers.
> > >
> > > In the current model (if I understand it correctly)
> > > 1) child = efl_add(klass, parent) means the child must NOT be
> unfeferenced
> > > 2) child = efl_add(klass, NULL) means the child should be unreferenced
> > > 3) child = efl_add_ref(klass, parent) means the child must be
> unreferenced
> > > 4) child = efl_add_ref(klass, NULL) somehow means that the child
> should be
> > > unreferenced twice
> > >
> > > In my opinion 1) and 4) are peculiar and so I provide a proposal to fix
> > > this:
> > >
> > > We could change efl_add to return void. It never retains a reference.
> If
> > > the parent is NULL then it should be automatically unref before
> returning.
> > > Then efl_add_ref would be changed to mirror this and always retain
> exactly
> > > 1 reference - so if parent is NULL there is no double count returned.
> > >
> > > Using this model if an Eo * is returned then I know I have a reference
> and
> > > must later unref.
> > > Any need for using the pointer in an efl_add (which is no longer
> returned)
> > > would still be supported through efl_added within the constructor.
> > >
> > > What do people think about this? I put the suggestion forward to
> improve
> > > the symettry with add and unref which is currently confusing. If my
> > > assumptions above are incorrect please let me know!
> > >
> > > Thanks,
> > > Andy
> > > --
> > > http://andywilliams.me
> > > http://ajwillia.ms
> > >
> > --
> > http://andywilliams.me
> > http://ajwillia.ms
> >
> --
> > 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
>
> --
http://andywilliams.me
http://ajwillia.ms
--
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

Re: [E-devel] efl_add causing confusion

[Carsten Haitzler]

On Sat, 23 Dec 2017 11:30:58 + Andrew Williams  said:

> Hi,
> 
> As this thread seems to be descending into word games and (insert
> appropriate word) contests I will reiterate my concern:
> 
> efl_add is inconsistent and that should be addressed.

do it's not. i explained already that it is not. i'll repeat again. it's
consistent:

if parent == valid object, then ref is owned by parent
else ref is owned by caller/scope.

that is consistent.

> I hope that is clear enough
> Andy
> 
> 
> On Thu, 21 Dec 2017 at 13:15, Andrew Williams  wrote:
> 
> > Hi,
> >
> > This is now well documented (
> > https://www.enlightenment.org/develop/tutorials/c/eo-refcount.md) but the
> > more I use efl_add the more I feel it is confusing especially to new
> > developers.
> >
> > In the current model (if I understand it correctly)
> > 1) child = efl_add(klass, parent) means the child must NOT be unfeferenced
> > 2) child = efl_add(klass, NULL) means the child should be unreferenced
> > 3) child = efl_add_ref(klass, parent) means the child must be unreferenced
> > 4) child = efl_add_ref(klass, NULL) somehow means that the child should be
> > unreferenced twice
> >
> > In my opinion 1) and 4) are peculiar and so I provide a proposal to fix
> > this:
> >
> > We could change efl_add to return void. It never retains a reference. If
> > the parent is NULL then it should be automatically unref before returning.
> > Then efl_add_ref would be changed to mirror this and always retain exactly
> > 1 reference - so if parent is NULL there is no double count returned.
> >
> > Using this model if an Eo * is returned then I know I have a reference and
> > must later unref.
> > Any need for using the pointer in an efl_add (which is no longer returned)
> > would still be supported through efl_added within the constructor.
> >
> > What do people think about this? I put the suggestion forward to improve
> > the symettry with add and unref which is currently confusing. If my
> > assumptions above are incorrect please let me know!
> >
> > Thanks,
> > Andy
> > --
> > http://andywilliams.me
> > http://ajwillia.ms
> >
> -- 
> http://andywilliams.me
> http://ajwillia.ms
> --
> 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

Re: [E-devel] efl_add causing confusion

[Carsten Haitzler]

On Sat, 23 Dec 2017 11:26:42 + Andrew Williams  said:

> Hi,
> 
> Thanks for posting that poll. Very useful.
> If I may I wish to contradict your assertion that this has all been
> discussed before - the poll had *0* consideration for what if parent is
> NULL.

i don't see why it needed to. NULL == no parent, thus floating ref. non-NULL
and the ref is immediately handed to the parent object. (assuming non-null is
also a valid object).

> If you re-read the accepted proposal you will see that is not the current
> behaviour at all. There is a (relatively) explicit handing of the reference
> which can occur *after* I am finished with the reference.
> In short the proposal that was voted for did not have this race condition
> by design.

the proposal of course considers NULL. how do you create the win? at the time
we had no loop object or had yet discussed it, so windows would have been
created with a NULL parent.

the core is the same. you want efl_add to return nothing (void) which makes it
useless for creating a container (win, box, button, etc. etc.) which is a huge
use case of objects at construction etc. ... which means we'd need to use
efl_add_ref() instead. or something that is the same (and you say this too
saying you then need to unref). and the whole idea of having to unref at end of
scope during creation was voted against. the same arguments i have been making
- it's less error prone to not have to unref. it's commonly seen enough in c
(gtk does it for example) etc. etc.

> And so it follows that we are currently working with a third option that
> was not part of the vote.

i don't see it that way. it's the same. if you remove the return from efl_add()
(returns void and deletes the object if parent is NULL) then the proposal you
have is to use efl_add_Ref for all these cases where you do need the return and
that is what was voted on already. even if today > 50% of devs voted for this
i'd still say "don't change" due to the sheer cost of change as i've explained.
my answer is "use efl_add_ref then if this bothers you and remember to unref,
but now you are the one holding the bag. we advised against this by default".
the thing you want is there d available for use. i and most developers would
advise against it (see vote).

> Same with gtk, apologies for misreading initially, they hand the reference
> over when they are done.

correct. and we hand it over "in one go" in efl_add. it amounts to the same.

> One last thought - safety is broader than not crashing. If we have put in
> place methods to catch corner cases in our design then perhaps our design
> is not solid enough?

eoid catches this case not because it was meant to catch the corner case you
speak of (parent deletes child while you still have the object handle in
scope) or other corner cases, but because of a more general problem of lots of
problems with developers unable to understand magic checks with invalid object
refs in general (mostly far far far later than scope usage), and simply handing
off the bug as "an efl bug" because it crashed inside an efl function (even
though the crash was in the magic check which de referenced a pointer pointing
out of memory space). it happens to be a very safe mechanism to access our
objects and it happens to deal with the case you speak of.

> Andy
> 
> On Sat, 23 Dec 2017 at 10:56, Carsten Haitzler  wrote:
> 
> > On Fri, 22 Dec 2017 11:44:32 + Andrew Williams 
> > said:
> >
> > > Hi,
> > >
> > > I think your summary about the Gtk is not what you think - read the docs
> > > further
> > > https://developer.gnome.org/gtk3/stable/GtkWindow.html#gtk-window-new
> > and
> > > you see that actually the trivial example leaks the reference - just like
> > > an efl_add_ref with no unref or del later.
> > > They are *not* returning a pointer where you can expect to lose ownership
> > > at some future time. Notice also they do not pass parents to the
> > > constructors.
> >
> > ummm no. that's wrong. because if you destroy the gtk window that you
> > added the
> > button and box to... the button is deleted and it DOESNT LEAK. the button
> > is
> > destroyed and freed. ownership of the reference is TRANSFERRED to the
> > parent
> > (the window) by gtk_container_add. see there are no unrefs there. it's not
> > the
> > same.  if the window was deleted (implicitly or explicitly) within that
> > scope
> > the button object pointer would have become invalid as well.
> >
> > efl_add_ref will have the button LEAK and stay around forever until you do
> > another unref on it as the parent being destroyed (the window or box) will
> > not
> > be enough to remove all references.
> >
> > i did gtk dev for quite a few years... :)
> >
> > > In ObjC the basic construct is
> > >
> > > obj = [klass alloc]
> > > ...
> > > do things with obj
> > > ...
> > > [obj release]
> > >
> > > and they made this less cumbersome with
> > >
> > > obj = [[klass alloc] 

Re: [E-devel] efl_add causing confusion

[Carsten Haitzler]

On Sat, 23 Dec 2017 11:19:59 + Andrew Williams  said:

> As framework developers we should be doing everything in our power to avoid
> undefined behaviour. I’ll take accidents memory leaks any day - there are a
> plethora of tools to solve that but it’s far harder to debug the “app not
> working correctly sometimes on my system” bug reports.

actually no. we should be doing everything we can to make things EASY. avoiding
undefined behaviour is one way of making things easier. having to write less
code and it be HARDER to write things that have bad side-effects is also making
things easier.

if you don't leak large amounts of objects you will not have tools find it easy
to find such leaks. if you leak LOTs (1000's or more objects in one place) then
you can probably find it by spotting an excessive allocation point in something
like massif, BUT nothing will detect a leaked pointer, because eoid maintains a
pointer to the object in the eoid object table. the pointer is actually never
lost. it's tracked by eo. so no. leak check tools won't work. maybe "still
allocated on shutdown" might unless you have a LOT of that data which is a lot
of work to ensure is done. there are lots of allocations on exit from efl,
glibc and other places already (that are harmless).

invalid access as i explained is directly ind immediately debuggable. leaks -
not so much.

but regardless of that the votes already were vastly for the thing we have
today.

> Andy
> 
> On Sat, 23 Dec 2017 at 10:56, Carsten Haitzler  wrote:
> 
> > On Fri, 22 Dec 2017 11:53:32 + Andrew Williams 
> > said:
> >
> > > We do have a showstopper design bug.
> >
> > see my previous mail. https://phab.enlightenment.org/V7 to be explicit.
> > discussed. voted on. known compromise. lots of code built on top of that
> > design
> > decision. not a showstopper by any stretch. it's exactly what gtk does. the
> > only difference is we have the "parent set" be part of the add and gtk
> > does it
> > as a separate step with container_add()
> >
> > > We are managing to work with it because we know how it works internally.
> > > The API must be straight forward and provide consistent, robust options.
> > >
> > > Any new user would be asking of efl_add:
> > > *) Do I need to unref the result when I am done?
> > > *) How long is my returned reference valid for?
> >
> > see the vote. see gtk samples. floating reference.
> >
> > > Both of those are answered with "it depends" and that is a design
> > > showstopper.
> >
> > disagree. it's clear. if parent is NULL then calling scope gets a
> > floating reference you have to unref. if not then parent owns that
> > reference and the rules of that parent cover lifetime of that object.
> > covered
> > that in prior mails - the alternative is to force people to manually unref
> > every time they want a handle to something at all to do useful things with
> > ...
> > like add a child to this parent (child packed into a box parent for
> > example).
> > see the gtk/gobject design doc with the idea of floating references. see
> > the
> > previous discussion thread. see the vote in favor of what is there now and
> > with
> > everyone fully aware of scoping etc. and that is why efl_add_ref exists if
> > you
> > choose to use that.
> >
> > note for this discussion i am assuming scope is a small local function
> > scope
> > not larger. technically it can be the global scope of the entire app.
> >
> > i'll put it simply:
> >
> > 1: do it your way and people will have to always unref at end of scope (if
> > they
> > want an object handle to do something with, like pack a button into a box
> > parent which is exceedingly common). the will at times forget to unref
> > causing
> > leaks.
> >
> > 2: do it the current way and perhaps sometimes in some rare circumstances
> > an
> > object is deleted by a parent before your scope ends.
> >
> > chances of 1: i'll put it at maybe 10-20% that someone forgets a scope
> > exit case
> > and forgets to unref (a return in the middle for example).
> >
> > chances of 2: i'll put at at maybe 1% on a bad day. likely far far less.
> >
> > so let's say 15% to 1%. 15x more mistakes likely with your proposal. but
> > let's
> > cover the CONSEQUENCES of a mistake:
> >
> > consequences of 1: leak memory (possibly huge amounts, if for example this
> > is
> > inside a model where you create objects and return them to the model to
> > display but now they return with an extra ref and never get deleted as a
> > result). leaking enough memory will result in severe system degradation
> > (hitting swap or OOM killer) not to mention be relatively hard to pin down
> > where the extra ref came from. a lot of fun debugging this ans isolating
> > it.
> >
> > consequences of 2: a complaint to stderr with the object id, a backtrace to
> > where it's invalid (instantly identifying the point of invalid access and
> > very
> > quickly leading to finding the 

Re: [E-devel] efl_add causing confusion

[Andrew Williams]

Hi,

As this thread seems to be descending into word games and (insert
appropriate word) contests I will reiterate my concern:

efl_add is inconsistent and that should be addressed.

I hope that is clear enough
Andy


On Thu, 21 Dec 2017 at 13:15, Andrew Williams  wrote:

> Hi,
>
> This is now well documented (
> https://www.enlightenment.org/develop/tutorials/c/eo-refcount.md) but the
> more I use efl_add the more I feel it is confusing especially to new
> developers.
>
> In the current model (if I understand it correctly)
> 1) child = efl_add(klass, parent) means the child must NOT be unfeferenced
> 2) child = efl_add(klass, NULL) means the child should be unreferenced
> 3) child = efl_add_ref(klass, parent) means the child must be unreferenced
> 4) child = efl_add_ref(klass, NULL) somehow means that the child should be
> unreferenced twice
>
> In my opinion 1) and 4) are peculiar and so I provide a proposal to fix
> this:
>
> We could change efl_add to return void. It never retains a reference. If
> the parent is NULL then it should be automatically unref before returning.
> Then efl_add_ref would be changed to mirror this and always retain exactly
> 1 reference - so if parent is NULL there is no double count returned.
>
> Using this model if an Eo * is returned then I know I have a reference and
> must later unref.
> Any need for using the pointer in an efl_add (which is no longer returned)
> would still be supported through efl_added within the constructor.
>
> What do people think about this? I put the suggestion forward to improve
> the symettry with add and unref which is currently confusing. If my
> assumptions above are incorrect please let me know!
>
> Thanks,
> Andy
> --
> http://andywilliams.me
> http://ajwillia.ms
>
-- 
http://andywilliams.me
http://ajwillia.ms
--
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

Re: [E-devel] efl_add causing confusion

[Andrew Williams]

Hi,

Thanks for posting that poll. Very useful.
If I may I wish to contradict your assertion that this has all been
discussed before - the poll had *0* consideration for what if parent is
NULL.
If you re-read the accepted proposal you will see that is not the current
behaviour at all. There is a (relatively) explicit handing of the reference
which can occur *after* I am finished with the reference.
In short the proposal that was voted for did not have this race condition
by design.

And so it follows that we are currently working with a third option that
was not part of the vote.

Same with gtk, apologies for misreading initially, they hand the reference
over when they are done.

One last thought - safety is broader than not crashing. If we have put in
place methods to catch corner cases in our design then perhaps our design
is not solid enough?

Andy

On Sat, 23 Dec 2017 at 10:56, Carsten Haitzler  wrote:

> On Fri, 22 Dec 2017 11:44:32 + Andrew Williams 
> said:
>
> > Hi,
> >
> > I think your summary about the Gtk is not what you think - read the docs
> > further
> > https://developer.gnome.org/gtk3/stable/GtkWindow.html#gtk-window-new
> and
> > you see that actually the trivial example leaks the reference - just like
> > an efl_add_ref with no unref or del later.
> > They are *not* returning a pointer where you can expect to lose ownership
> > at some future time. Notice also they do not pass parents to the
> > constructors.
>
> ummm no. that's wrong. because if you destroy the gtk window that you
> added the
> button and box to... the button is deleted and it DOESNT LEAK. the button
> is
> destroyed and freed. ownership of the reference is TRANSFERRED to the
> parent
> (the window) by gtk_container_add. see there are no unrefs there. it's not
> the
> same.  if the window was deleted (implicitly or explicitly) within that
> scope
> the button object pointer would have become invalid as well.
>
> efl_add_ref will have the button LEAK and stay around forever until you do
> another unref on it as the parent being destroyed (the window or box) will
> not
> be enough to remove all references.
>
> i did gtk dev for quite a few years... :)
>
> > In ObjC the basic construct is
> >
> > obj = [klass alloc]
> > ...
> > do things with obj
> > ...
> > [obj release]
> >
> > and they made this less cumbersome with
> >
> > obj = [[klass alloc] autorelease]
> > ...
> > do things with obj
> >
> > This provides a guarantee that the obj reference is alive for the current
> > scope.
> > I can find no examples where an object reference may or may not stay live
> > and may or may not need released based on a variable passed to the
> > constructor call...
>
> because the LANGUAGE will unref when scope exits. C will not. you must do
> it
> manually. and that is a vector for bugs, leaks and complaints about how
> hard it
> is and how much simpler gtk is etc.
>
> what's worse? a 20% chance that someone will forget to unref when exiting
> scope (and there can be multiple scope exit points), or that "the object
> will
> be deleted from under you". the 2nd will result in a complaint that the
> eoid is
> invalid. the first will result in a memory leak. possibly until all memory
> is
> exhausted and oom killers kick in. one has a very tiny chance of happening
> the
> other has a much higher chance (forgetting to manually unref is going to
> be a
> far higher chance). the one with the higher chance will lead to memory
> being
> gobbled up until perhaps a system falls over (possibly many processes and
> daemons fall over), and the other results in a complaint of an invalid
> object.
>
> and i've already mentioned that magical deletion in these cases is
> probably a
> bug (probably, not certainly) in the lifecycle design of something.
>
> > Wanting safety in our applications is not pedantry - anything less than
> > scope guarantees is asking for trouble and we are just patching bad
> > decisions.
>
> didn't i just say it was safe? it's safe. eoid ensures that.
>
> > I have been told on so many occasions that none of this API is final -
> but
> > now there is a chance to make things better but I am too late?
> > Please make up your mind.
>
> these arguments were had already a long time ago before now and now we're
> going
> back after a lot of code has been built on top. you are asking for a change
> that will lead to a lot of instability for a long time and a huge delay in
> work. it'd better be a very very very very good reason. what is the value
> of
> that change and then how much work is needed to implement it... not even
> considering who will do it. is it worth it?
>
> IMHO it's not. it's just dredging up an old argument that was already
> resolved.
> i really don't want to trawl through multiple years of mailing lists etc.
> to go
> find it, but i did:
>
> https://phab.enlightenment.org/V7
>
> what we have is the result of that discussion and decision. from mid 2014.
> i do
> 

Re: [E-devel] efl_add causing confusion

[Andrew Williams]

As framework developers we should be doing everything in our power to avoid
undefined behaviour. I’ll take accidents memory leaks any day - there are a
plethora of tools to solve that but it’s far harder to debug the “app not
working correctly sometimes on my system” bug reports.

Andy

On Sat, 23 Dec 2017 at 10:56, Carsten Haitzler  wrote:

> On Fri, 22 Dec 2017 11:53:32 + Andrew Williams 
> said:
>
> > We do have a showstopper design bug.
>
> see my previous mail. https://phab.enlightenment.org/V7 to be explicit.
> discussed. voted on. known compromise. lots of code built on top of that
> design
> decision. not a showstopper by any stretch. it's exactly what gtk does. the
> only difference is we have the "parent set" be part of the add and gtk
> does it
> as a separate step with container_add()
>
> > We are managing to work with it because we know how it works internally.
> > The API must be straight forward and provide consistent, robust options.
> >
> > Any new user would be asking of efl_add:
> > *) Do I need to unref the result when I am done?
> > *) How long is my returned reference valid for?
>
> see the vote. see gtk samples. floating reference.
>
> > Both of those are answered with "it depends" and that is a design
> > showstopper.
>
> disagree. it's clear. if parent is NULL then calling scope gets a
> floating reference you have to unref. if not then parent owns that
> reference and the rules of that parent cover lifetime of that object.
> covered
> that in prior mails - the alternative is to force people to manually unref
> every time they want a handle to something at all to do useful things with
> ...
> like add a child to this parent (child packed into a box parent for
> example).
> see the gtk/gobject design doc with the idea of floating references. see
> the
> previous discussion thread. see the vote in favor of what is there now and
> with
> everyone fully aware of scoping etc. and that is why efl_add_ref exists if
> you
> choose to use that.
>
> note for this discussion i am assuming scope is a small local function
> scope
> not larger. technically it can be the global scope of the entire app.
>
> i'll put it simply:
>
> 1: do it your way and people will have to always unref at end of scope (if
> they
> want an object handle to do something with, like pack a button into a box
> parent which is exceedingly common). the will at times forget to unref
> causing
> leaks.
>
> 2: do it the current way and perhaps sometimes in some rare circumstances
> an
> object is deleted by a parent before your scope ends.
>
> chances of 1: i'll put it at maybe 10-20% that someone forgets a scope
> exit case
> and forgets to unref (a return in the middle for example).
>
> chances of 2: i'll put at at maybe 1% on a bad day. likely far far less.
>
> so let's say 15% to 1%. 15x more mistakes likely with your proposal. but
> let's
> cover the CONSEQUENCES of a mistake:
>
> consequences of 1: leak memory (possibly huge amounts, if for example this
> is
> inside a model where you create objects and return them to the model to
> display but now they return with an extra ref and never get deleted as a
> result). leaking enough memory will result in severe system degradation
> (hitting swap or OOM killer) not to mention be relatively hard to pin down
> where the extra ref came from. a lot of fun debugging this ans isolating
> it.
>
> consequences of 2: a complaint to stderr with the object id, a backtrace to
> where it's invalid (instantly identifying the point of invalid access and
> very
> quickly leading to finding the source). with the identification of the
> invalid
> source point, finding the deletion point is very easy by adding a del event
> callback and tracing that. We could find ways of making this easier with
> env
> vars or a convenience call like efl_debug_del_track(obj, EINA_TRUE) which
> might print a debug log with bt on deletion of this object. you already
> know
> the exact object that is creating this issue from the first bt... so this
> will
> be a 2nd step to isolate it exactly. much easier to fix than #1 AND the
> conseqeunces are far less as it's just a "error print".
>
> so how do we rate the conequences? well #2 - let's call it a 10, and let's
> say
> somethnig vey hard to debug and that could lead to oom killers and swap
> storms... i'll call that a 1000. so 15*1000 vs 1*10. 1500 TIMES worse. yes.
> numbers pulled out of thin air. i'm making a point. your proposal is worse.
>
> in addition moving from #2 to #1 would be a huge effort to do, lead to a
> lot of
> problems in the efl tree for a long time until cleaned up (leaks found and
> fixed, all the instances done right etc.) thus adding pain, delays etc.
> etc.
> and ... who will do it? the majority (~87% of devs) disagree with it
> option #2.
> see poll. so most devs will not be keen to do something the vast majority
> disagree with.
>
> is something voted against by 87% of devs and 1500 

Re: [E-devel] efl_add causing confusion

[Carsten Haitzler]

On Fri, 22 Dec 2017 11:44:32 + Andrew Williams  said:

> Hi,
> 
> I think your summary about the Gtk is not what you think - read the docs
> further
> https://developer.gnome.org/gtk3/stable/GtkWindow.html#gtk-window-new and
> you see that actually the trivial example leaks the reference - just like
> an efl_add_ref with no unref or del later.
> They are *not* returning a pointer where you can expect to lose ownership
> at some future time. Notice also they do not pass parents to the
> constructors.

ummm no. that's wrong. because if you destroy the gtk window that you added the
button and box to... the button is deleted and it DOESNT LEAK. the button is
destroyed and freed. ownership of the reference is TRANSFERRED to the parent
(the window) by gtk_container_add. see there are no unrefs there. it's not the
same.  if the window was deleted (implicitly or explicitly) within that scope
the button object pointer would have become invalid as well.

efl_add_ref will have the button LEAK and stay around forever until you do
another unref on it as the parent being destroyed (the window or box) will not
be enough to remove all references.

i did gtk dev for quite a few years... :)

> In ObjC the basic construct is
> 
> obj = [klass alloc]
> ...
> do things with obj
> ...
> [obj release]
> 
> and they made this less cumbersome with
> 
> obj = [[klass alloc] autorelease]
> ...
> do things with obj
> 
> This provides a guarantee that the obj reference is alive for the current
> scope.
> I can find no examples where an object reference may or may not stay live
> and may or may not need released based on a variable passed to the
> constructor call...

because the LANGUAGE will unref when scope exits. C will not. you must do it
manually. and that is a vector for bugs, leaks and complaints about how hard it
is and how much simpler gtk is etc.

what's worse? a 20% chance that someone will forget to unref when exiting
scope (and there can be multiple scope exit points), or that "the object will
be deleted from under you". the 2nd will result in a complaint that the eoid is
invalid. the first will result in a memory leak. possibly until all memory is
exhausted and oom killers kick in. one has a very tiny chance of happening the
other has a much higher chance (forgetting to manually unref is going to be a
far higher chance). the one with the higher chance will lead to memory being
gobbled up until perhaps a system falls over (possibly many processes and
daemons fall over), and the other results in a complaint of an invalid object.

and i've already mentioned that magical deletion in these cases is probably a
bug (probably, not certainly) in the lifecycle design of something.

> Wanting safety in our applications is not pedantry - anything less than
> scope guarantees is asking for trouble and we are just patching bad
> decisions.

didn't i just say it was safe? it's safe. eoid ensures that.

> I have been told on so many occasions that none of this API is final - but
> now there is a chance to make things better but I am too late?
> Please make up your mind.

these arguments were had already a long time ago before now and now we're going
back after a lot of code has been built on top. you are asking for a change
that will lead to a lot of instability for a long time and a huge delay in
work. it'd better be a very very very very good reason. what is the value of
that change and then how much work is needed to implement it... not even
considering who will do it. is it worth it?

IMHO it's not. it's just dredging up an old argument that was already resolved.
i really don't want to trawl through multiple years of mailing lists etc. to go
find it, but i did:

https://phab.enlightenment.org/V7

what we have is the result of that discussion and decision. from mid 2014. i do
remember this having all come up before though and it being resolved early on
in eo's life before a lot was built on top.

so it's not "final" but the cost of change is immense. this has been discussed
and voted on with an overwhelming vote in favor of what we have now. the side
effects of a change are immense. just another few votes for a change are imho
not enough to create that change.

> Andrew
> 
> On Fri, 22 Dec 2017 at 10:56 Carsten Haitzler  wrote:
> 
> > On Fri, 22 Dec 2017 10:10:48 + Andrew Williams 
> > said:
> >
> > > On Fri, 22 Dec 2017 at 09:59 Carsten Haitzler 
> > wrote:
> > >
> > > > On Fri, 22 Dec 2017 09:43:20 + Andrew Williams <
> > a...@andywilliams.me>
> > > > said:
> > > >
> > > > > Hi,
> > > > >
> > > > > I think that maybe I could have explained this a little better, let
> > me
> > > > step
> > > > > back from the implementation details for a minute.
> > > > >
> > > > > As a user of this API all I want to know is do I have to unref a
> > > > reference
> > > > > that I have been handed. From this point of view we should be
> > consistent.

Re: [E-devel] efl_add causing confusion

[Carsten Haitzler]

On Fri, 22 Dec 2017 11:53:32 + Andrew Williams  said:

> We do have a showstopper design bug.

see my previous mail. https://phab.enlightenment.org/V7 to be explicit.
discussed. voted on. known compromise. lots of code built on top of that design
decision. not a showstopper by any stretch. it's exactly what gtk does. the
only difference is we have the "parent set" be part of the add and gtk does it
as a separate step with container_add()

> We are managing to work with it because we know how it works internally.
> The API must be straight forward and provide consistent, robust options.
> 
> Any new user would be asking of efl_add:
> *) Do I need to unref the result when I am done?
> *) How long is my returned reference valid for?

see the vote. see gtk samples. floating reference.

> Both of those are answered with "it depends" and that is a design
> showstopper.

disagree. it's clear. if parent is NULL then calling scope gets a
floating reference you have to unref. if not then parent owns that
reference and the rules of that parent cover lifetime of that object. covered
that in prior mails - the alternative is to force people to manually unref
every time they want a handle to something at all to do useful things with ...
like add a child to this parent (child packed into a box parent for example).
see the gtk/gobject design doc with the idea of floating references. see the
previous discussion thread. see the vote in favor of what is there now and with
everyone fully aware of scoping etc. and that is why efl_add_ref exists if you
choose to use that.

note for this discussion i am assuming scope is a small local function scope
not larger. technically it can be the global scope of the entire app.

i'll put it simply:

1: do it your way and people will have to always unref at end of scope (if they
want an object handle to do something with, like pack a button into a box
parent which is exceedingly common). the will at times forget to unref causing
leaks.

2: do it the current way and perhaps sometimes in some rare circumstances an
object is deleted by a parent before your scope ends.

chances of 1: i'll put it at maybe 10-20% that someone forgets a scope exit case
and forgets to unref (a return in the middle for example).

chances of 2: i'll put at at maybe 1% on a bad day. likely far far less.

so let's say 15% to 1%. 15x more mistakes likely with your proposal. but let's
cover the CONSEQUENCES of a mistake:

consequences of 1: leak memory (possibly huge amounts, if for example this is
inside a model where you create objects and return them to the model to
display but now they return with an extra ref and never get deleted as a
result). leaking enough memory will result in severe system degradation
(hitting swap or OOM killer) not to mention be relatively hard to pin down
where the extra ref came from. a lot of fun debugging this ans isolating it.

consequences of 2: a complaint to stderr with the object id, a backtrace to
where it's invalid (instantly identifying the point of invalid access and very
quickly leading to finding the source). with the identification of the invalid
source point, finding the deletion point is very easy by adding a del event
callback and tracing that. We could find ways of making this easier with env
vars or a convenience call like efl_debug_del_track(obj, EINA_TRUE) which
might print a debug log with bt on deletion of this object. you already know
the exact object that is creating this issue from the first bt... so this will
be a 2nd step to isolate it exactly. much easier to fix than #1 AND the
conseqeunces are far less as it's just a "error print".

so how do we rate the conequences? well #2 - let's call it a 10, and let's say
somethnig vey hard to debug and that could lead to oom killers and swap
storms... i'll call that a 1000. so 15*1000 vs 1*10. 1500 TIMES worse. yes.
numbers pulled out of thin air. i'm making a point. your proposal is worse.

in addition moving from #2 to #1 would be a huge effort to do, lead to a lot of
problems in the efl tree for a long time until cleaned up (leaks found and
fixed, all the instances done right etc.) thus adding pain, delays etc. etc.
and ... who will do it? the majority (~87% of devs) disagree with it option #2.
see poll. so most devs will not be keen to do something the vast majority
disagree with.

is something voted against by 87% of devs and 1500 times worse for the end
user worth spending a lot of effort on and adding delays and such pain?

> Andrew
> 
> On Fri, 22 Dec 2017 at 10:56 Carsten Haitzler  wrote:
> 
> > [snip]
> >
> > this is the kind of change that would only really justify being done if we
> > had
> > a showstopper design bug. we do not. not IMHO. there is clear behaviour and
> > it's logical and simple. if you don't LIKE it... efl_add_ref is there for
> > you
> > to use and then to remember to unref on all scope exits by hand. i
> > certainly
> > would not choose to use 

Re: [E-devel] efl_add causing confusion

[Carsten Haitzler]

On Fri, 22 Dec 2017 11:19:40 -0500 Cedric Bail <ced...@ddlm.me> said:

> >  Original Message 
> > Subject: Re: [E-devel] efl_add causing confusion
> > Local Time: December 21, 2017 7:18 PM
> > UTC Time: December 22, 2017 3:18 AM
> > From: ras...@rasterman.com
> > To: Enlightenment developer list <enlightenment-devel@lists.sourceforge.net>
> > Cedric Bail <ced...@ddlm.me>
> 
> 
> 
> >> you have an object and you do evas_object_del(). you can keep the object
> >> alive with more refs, but del SHOULD hide the object. refs should not
> >> affect VISIBILITY of the object, thus a del is different to an unref.
> >>
> >> del == "remove a reference from this object and otherwise act as if this
> >> object should now be deleted".
> >>
> >> unref == "just remove my reference to this object. this MAY result in a del
> >> behavior at some point, but i'm simply releasing a reference and not
> >> actually explicitly ending the lifetime of this object".
> >>
> >> semantically they are quite different things.
> 
> You missed my point, bindings can not use del due to the semantic of it as it
> might unparent or might remove a reference, they can't control its behavior.
> Sure it is nice to have a del that you can override in a class as oposed to
> unref, but if just C can use it, there is no point in it being in a Eo object.

of course bindings can expose and use it.

obj.del();

unlike c there may be another SCOPE reference at the top in the target language
so it's be equivalent to:

begin scope -> {
efl_ref(obj);
...
efl_del(obj);
...
efl_unref(obj);
} <- end scope

> Now if we want to continue to provide efl_del facility a proposal solution
> need to be tested with binding. I would expect that if it did always set the
> parent to NULL and always unref, it would be more usable by binding, but it
> still require testing that and all the change proposed by Andrew.

it does always set parent to null... and that results in an unref... or it just
unrefs. either way a del results in an unref - always (unless intercepted for
special reasons like caching etc. but from the outside it should look like it
was an unref).

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

Re: [E-devel] efl_add causing confusion

[Cedric Bail]

>  Original Message 
> Subject: Re: [E-devel] efl_add causing confusion
> Local Time: December 21, 2017 7:18 PM
> UTC Time: December 22, 2017 3:18 AM
> From: ras...@rasterman.com
> To: Enlightenment developer list <enlightenment-devel@lists.sourceforge.net>
> Cedric Bail <ced...@ddlm.me>



>> you have an object and you do evas_object_del(). you can keep the object 
>> alive
>> with more refs, but del SHOULD hide the object. refs should not affect
>> VISIBILITY of the object, thus a del is different to an unref.
>>
>> del == "remove a reference from this object and otherwise act as if this 
>> object
>> should now be deleted".
>>
>> unref == "just remove my reference to this object. this MAY result in a del
>> behavior at some point, but i'm simply releasing a reference and not actually
>> explicitly ending the lifetime of this object".
>>
>> semantically they are quite different things.

You missed my point, bindings can not use del due to the semantic of it as it 
might unparent or might remove a reference, they can't control its behavior. 
Sure it is nice to have a del that you can override in a class as oposed to 
unref, but if just C can use it, there is no point in it being in a Eo object.

Now if we want to continue to provide efl_del facility a proposal solution need 
to be tested with binding. I would expect that if it did always set the parent 
to NULL and always unref, it would be more usable by binding, but it still 
require testing that and all the change proposed by Andrew.

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

Re: [E-devel] efl_add causing confusion

[Andrew Williams]

We do have a showstopper design bug.
We are managing to work with it because we know how it works internally.
The API must be straight forward and provide consistent, robust options.

Any new user would be asking of efl_add:
*) Do I need to unref the result when I am done?
*) How long is my returned reference valid for?

Both of those are answered with "it depends" and that is a design
showstopper.

Andrew

On Fri, 22 Dec 2017 at 10:56 Carsten Haitzler  wrote:

> [snip]
>
> this is the kind of change that would only really justify being done if we
> had
> a showstopper design bug. we do not. not IMHO. there is clear behaviour and
> it's logical and simple. if you don't LIKE it... efl_add_ref is there for
> you
> to use and then to remember to unref on all scope exits by hand. i
> certainly
> would not choose to use this.
>
> [snip]
>
> --
> - Codito, ergo sum - "I code, therefore I am" --
> Carsten Haitzler - ras...@rasterman.com
>
> --
http://andywilliams.me
http://ajwillia.ms
--
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

Re: [E-devel] efl_add causing confusion

[Andrew Williams]

Hi,

I think your summary about the Gtk is not what you think - read the docs
further
https://developer.gnome.org/gtk3/stable/GtkWindow.html#gtk-window-new and
you see that actually the trivial example leaks the reference - just like
an efl_add_ref with no unref or del later.
They are *not* returning a pointer where you can expect to lose ownership
at some future time. Notice also they do not pass parents to the
constructors.

In ObjC the basic construct is

obj = [klass alloc]
...
do things with obj
...
[obj release]

and they made this less cumbersome with

obj = [[klass alloc] autorelease]
...
do things with obj

This provides a guarantee that the obj reference is alive for the current
scope.
I can find no examples where an object reference may or may not stay live
and may or may not need released based on a variable passed to the
constructor call...

Wanting safety in our applications is not pedantry - anything less than
scope guarantees is asking for trouble and we are just patching bad
decisions.
I have been told on so many occasions that none of this API is final - but
now there is a chance to make things better but I am too late?
Please make up your mind.

Andrew

On Fri, 22 Dec 2017 at 10:56 Carsten Haitzler  wrote:

> On Fri, 22 Dec 2017 10:10:48 + Andrew Williams 
> said:
>
> > On Fri, 22 Dec 2017 at 09:59 Carsten Haitzler 
> wrote:
> >
> > > On Fri, 22 Dec 2017 09:43:20 + Andrew Williams <
> a...@andywilliams.me>
> > > said:
> > >
> > > > Hi,
> > > >
> > > > I think that maybe I could have explained this a little better, let
> me
> > > step
> > > > back from the implementation details for a minute.
> > > >
> > > > As a user of this API all I want to know is do I have to unref a
> > > reference
> > > > that I have been handed. From this point of view we should be
> consistent.
> > > > My proposal was (intended) to mean that efl_add should never require
> the
> > > > user to unreference whereas efl_add_ref should always require this
> (but
> > > > only once - that may have been a bug).
> > > >
> > > > Having to know the value of the second parameter to the method to
> > > ascertain
> > > > the correct behaviour is what I think is confusing and should be
> > > > eliminated. Therefore the main purpose was so that the docs could
> ready
> > > > simply "this is a borrowed reference which you should not unref" vs
> "this
> > > > is an owned reference and you must unref when done".
> > >
> > > yes. i got that. but your proposal is no better than what is there. in
> fact
> > > think it's worse. unless i didn't understand it?
> > >
> >
> > "is no better"? it ensures that you know to unref efl_add_ref and that
> you
> > cannot accidentally use a risky efl_add. Not ideal which is why I
> suggested
> > efl_add_scope in a follow up which is something you don't have to unref
> but
> > also guarantees liveness for the scope.
>
> forcing all this code that builds widgets (see below about leaks) to
> remember
> to unref is asking for disaster as people will juyst end up using
> efl_Add_ref
> all the time because not doing so it incredibly inconvenient. it's also
> going to
> be a major complaint in usage.
>
> reality is ... people seem fine with how we do it in c. why? this is how
> gtk
> works... same thing. it's also how efl has worked for a long time. hello
> world
> for gtk:
>
> https://developer.gnome.org/gtk-tutorial/2.90/c39.html
> https://developer.gnome.org/gtk3/stable/gtk-getting-started.html
>
> the construction return an object ptr, you do things with it, hand it to a
> parent. no unreffing. in gtk the parent could delete the children any time
> it
> liked like in efl. this is a common design pattern already in c for this
> kind
> of stuff.
>
> you can be pedantic, or you can just be practical. this is one of these
> "we're
> being practical" things rather than pedantic. if you want to write code
> pedantically then efl_add_ref() is there - remember to unref when going
> out of
> scope or you'll have leaks.
>
> > > i get your point... as i said. but your proposal doesn't sound as if
> it's
> > > better.
> > >
> > > what MIGHT be better is to class objects into "can have a null parent"
> and
> > > "must have a non-null parent". the 1st case doesn't change. from now.
> the
> > > 2nd
> > > case results in efl_add returning NULL (construction fails) if the
> parent
> > > is
> > > null. that might be an improvement.
> > >
> >
> > Why do I, as the user, have to care what the content of the parent
> variable
> > is? It was suggested to me that efl_add could require a parent and then
> > would safely return the reference, but you still have the race condition
> so
> > I'm not convinced.
>
> see above. gtk and friends are full of this condition yet people are fine
> with
> it. people have been fine with this in efl for a long time. it's not a race
> condition really. it's a lifecycle behavior definition. once you hand a
> child
> to a 

Re: [E-devel] efl_add causing confusion

[Carsten Haitzler]

On Fri, 22 Dec 2017 09:47:34 + Andrew Williams <a...@andywilliams.me> said:

> Hi,
> 
> Surely hide and unref are separate? Why are we trying to do both with a
> single call to _del? If we wish to continue supporting "hide and unref" in
> a single call then we can - but as a convenience. It could be documented as
> such: "del will remove your reference to the object and, if the object is
> visible, hide it. Any other references will remain valid but will no longer
> visibly affect the object" nice and clear.

that was my point. del is different to unref. it may also do these other things
like unparent, hide, etc. etc.

> But I think cedric is still correct - unref is all that is "needed" - any
> user could _hide && _unref if they wanted - it would then be exactly the
> same as the _del semantic you describe.
> 
> If we have to put a big OR in a simple method documentation then we are
> setting up confusion.

i think it's pretty simple - use ref and unref for REFERENCES, use add and del
for begnning and end of life. actual end of life may be delayed due to
references still being there.

> Andy
> 
> On Fri, 22 Dec 2017 at 03:26 Carsten Haitzler <ras...@rasterman.com> wrote:
> 
> > On Thu, 21 Dec 2017 13:29:05 -0500 Cedric Bail <ced...@ddlm.me> said:
> >
> > > Hi,
> > >
> > > >  Original Message 
> > > > Subject: Re: [E-devel] efl_add causing confusion
> > > > Local Time: December 21, 2017 9:02 AM
> > > > UTC Time: December 21, 2017 5:02 PM
> > > > From: a...@andywilliams.me
> > > > To: Enlightenment developer list <
> > enlightenment-devel@lists.sourceforge.net>
> > > >
> > > > P.S. if the loss of temporary handles with efl_add is not met with
> > > > efl_added it would be possible to add a new macro along the lines of:
> > > >
> > > > efl_add_scope(klass, parent) { ... statements ... } whereby the scope
> > of
> > > > the variable is valid only within that block.
> > >
> > > This is an interesting idea that give an explicit lifecycle to the newly
> > > created reference and when you safely own it.
> > >
> > > > On Thu, 21 Dec 2017 at 13:15 Andrew Williams a...@andywilliams.me
> > wrote:
> > > >
> > > >> Hi,
> > > >> This is now well documented (
> > > >> https://www.enlightenment.org/develop/tutorials/c/eo-refcount.md)
> > but the
> > > >> more I use efl_add the more I feel it is confusing especially to new
> > > >> developers.
> > > >> In the current model (if I understand it correctly)
> > > >>
> > > >> - child = efl_add(klass, parent) means the child must NOT be
> > unfeferenced
> > > >> - child = efl_add(klass, NULL) means the child should be unreferenced
> > > >> - child = efl_add_ref(klass, parent) means the child must be
> > unreferenced
> > > >> - child = efl_add_ref(klass, NULL) somehow means that the child
> > should be
> > > >> unreferenced twice
> > > >>
> > > >> In my opinion 1) and 4) are peculiar and so I provide a proposal to
> > fix
> > > >> this:
> > > >> We could change efl_add to return void. It never retains a reference.
> > If
> > > >> the parent is NULL then it should be automatically unref before
> > returning.
> > > >> Then efl_add_ref would be changed to mirror this and always retain
> > exactly
> > > >> 1 reference - so if parent is NULL there is no double count returned.
> > > >> Using this model if an Eo * is returned then I know I have a
> > reference and
> > > >> must later unref.
> > > >> Any need for using the pointer in an efl_add (which is no longer
> > returned)
> > > >> would still be supported through efl_added within the constructor.
> > > >> What do people think about this? I put the suggestion forward to
> > improve
> > > >> the symettry with add and unref which is currently confusing. If my
> > > >> assumptions above are incorrect please let me know!
> > >
> > > We have been trying to fix the semantic problem of efl_add since pretty
> > much
> > > it was created. This proposal seems to build on top of efl_add_ref that
> > was
> > > added to fix the problem created for binding. I do like this proposal at
> > it
> > > start fixing the problem of inconsistency when creating an

Re: [E-devel] efl_add causing confusion

[Carsten Haitzler]

On Fri, 22 Dec 2017 10:10:48 + Andrew Williams  said:

> On Fri, 22 Dec 2017 at 09:59 Carsten Haitzler  wrote:
> 
> > On Fri, 22 Dec 2017 09:43:20 + Andrew Williams 
> > said:
> >
> > > Hi,
> > >
> > > I think that maybe I could have explained this a little better, let me
> > step
> > > back from the implementation details for a minute.
> > >
> > > As a user of this API all I want to know is do I have to unref a
> > reference
> > > that I have been handed. From this point of view we should be consistent.
> > > My proposal was (intended) to mean that efl_add should never require the
> > > user to unreference whereas efl_add_ref should always require this (but
> > > only once - that may have been a bug).
> > >
> > > Having to know the value of the second parameter to the method to
> > ascertain
> > > the correct behaviour is what I think is confusing and should be
> > > eliminated. Therefore the main purpose was so that the docs could ready
> > > simply "this is a borrowed reference which you should not unref" vs "this
> > > is an owned reference and you must unref when done".
> >
> > yes. i got that. but your proposal is no better than what is there. in fact
> > think it's worse. unless i didn't understand it?
> >
> 
> "is no better"? it ensures that you know to unref efl_add_ref and that you
> cannot accidentally use a risky efl_add. Not ideal which is why I suggested
> efl_add_scope in a follow up which is something you don't have to unref but
> also guarantees liveness for the scope.

forcing all this code that builds widgets (see below about leaks) to remember
to unref is asking for disaster as people will juyst end up using efl_Add_ref
all the time because not doing so it incredibly inconvenient. it's also going to
be a major complaint in usage.

reality is ... people seem fine with how we do it in c. why? this is how gtk
works... same thing. it's also how efl has worked for a long time. hello world
for gtk:

https://developer.gnome.org/gtk-tutorial/2.90/c39.html
https://developer.gnome.org/gtk3/stable/gtk-getting-started.html

the construction return an object ptr, you do things with it, hand it to a
parent. no unreffing. in gtk the parent could delete the children any time it
liked like in efl. this is a common design pattern already in c for this kind
of stuff.

you can be pedantic, or you can just be practical. this is one of these "we're
being practical" things rather than pedantic. if you want to write code
pedantically then efl_add_ref() is there - remember to unref when going out of
scope or you'll have leaks.

> > i get your point... as i said. but your proposal doesn't sound as if it's
> > better.
> >
> > what MIGHT be better is to class objects into "can have a null parent" and
> > "must have a non-null parent". the 1st case doesn't change. from now. the
> > 2nd
> > case results in efl_add returning NULL (construction fails) if the parent
> > is
> > null. that might be an improvement.
> >
> 
> Why do I, as the user, have to care what the content of the parent variable
> is? It was suggested to me that efl_add could require a parent and then
> would safely return the reference, but you still have the race condition so
> I'm not convinced.

see above. gtk and friends are full of this condition yet people are fine with
it. people have been fine with this in efl for a long time. it's not a race
condition really. it's a lifecycle behavior definition. once you hand a child
to a parent, that parent can and will control lifecycle and thus it MAY delete
it at some point... and that point MAY be immediately in some rare cases. we
have a single efl_add() constructor do a multi-step "create, set
properties/callbcks and set parent etc. etc.". so yes - knowing what parent is
matters because of this.

> > > The added complication is the race condition with the "parent" usage in
> > > efl_add. When I pass a parent reference to efl_add then the returned
> > handle
> > > is something I can use. Yes this is very helpful. But this is dangerous.
> > We
> > > (as a framework) are providing 0 guarantee that the object will live as
> > > long as the scope - that is entirely up to the parent, not the user and
> > not
> > > the framework. This is also something that I think we should address.
> >
> > we can't guarantee it if the parent decides some time within the scope to
> > delete the object you added. well we can;'t unless you use efl_add_ref()
> > then
> > you create an object with 2 references (parent one and your scope/app one)
> > and
> > you will have to remember to unref at end of scope. this leads to verbose
> > code,
> > and having to remember to do this... which is more than likely going to
> > lead to
> > huge amounts of memory leaking.
> 
> Only if not coded correctly. We currently return references which are not
> clear if they should be unref or not so there is not a huge difference.

it is clear. if parent is NULL - YOU own 

Re: [E-devel] efl_add causing confusion

[Andrew Williams]

On Fri, 22 Dec 2017 at 09:59 Carsten Haitzler  wrote:

> On Fri, 22 Dec 2017 09:43:20 + Andrew Williams 
> said:
>
> > Hi,
> >
> > I think that maybe I could have explained this a little better, let me
> step
> > back from the implementation details for a minute.
> >
> > As a user of this API all I want to know is do I have to unref a
> reference
> > that I have been handed. From this point of view we should be consistent.
> > My proposal was (intended) to mean that efl_add should never require the
> > user to unreference whereas efl_add_ref should always require this (but
> > only once - that may have been a bug).
> >
> > Having to know the value of the second parameter to the method to
> ascertain
> > the correct behaviour is what I think is confusing and should be
> > eliminated. Therefore the main purpose was so that the docs could ready
> > simply "this is a borrowed reference which you should not unref" vs "this
> > is an owned reference and you must unref when done".
>
> yes. i got that. but your proposal is no better than what is there. in fact
> think it's worse. unless i didn't understand it?
>

"is no better"? it ensures that you know to unref efl_add_ref and that you
cannot accidentally use a risky efl_add. Not ideal which is why I suggested
efl_add_scope in a follow up which is something you don't have to unref but
also guarantees liveness for the scope.


> i get your point... as i said. but your proposal doesn't sound as if it's
> better.
>
> what MIGHT be better is to class objects into "can have a null parent" and
> "must have a non-null parent". the 1st case doesn't change. from now. the
> 2nd
> case results in efl_add returning NULL (construction fails) if the parent
> is
> null. that might be an improvement.
>

Why do I, as the user, have to care what the content of the parent variable
is? It was suggested to me that efl_add could require a parent and then
would safely return the reference, but you still have the race condition so
I'm not convinced.


> > The added complication is the race condition with the "parent" usage in
> > efl_add. When I pass a parent reference to efl_add then the returned
> handle
> > is something I can use. Yes this is very helpful. But this is dangerous.
> We
> > (as a framework) are providing 0 guarantee that the object will live as
> > long as the scope - that is entirely up to the parent, not the user and
> not
> > the framework. This is also something that I think we should address.
>
> we can't guarantee it if the parent decides some time within the scope to
> delete the object you added. well we can;'t unless you use efl_add_ref()
> then
> you create an object with 2 references (parent one and your scope/app one)
> and
> you will have to remember to unref at end of scope. this leads to verbose
> code,
> and having to remember to do this... which is more than likely going to
> lead to
> huge amounts of memory leaking.
>

Only if not coded correctly. We currently return references which are not
clear if they should be unref or not so there is not a huge difference.


> so which is worse? sometimes a parent decides to delete an object on you
> while
> you are still holding and using its handle, or that you always have to
> unref
> every obj you added when you exit scope? i'll take the first one any day
> of the
> week. :)
>

I'm going to say safety. 100% safety is more important than removing a line
of code. This is our opportunity to do things right. Introducing a race
condition at the root of our API seems an unfortunate choice.


> > So there you go - the background behind my proposal. Hopefully if I did
> not
> > explain the details well you can now see why I felt it is important.
>
> yup. i know the background. i'm speaking of the details of the proposal.
>
> > Thanks,
> > Andy
> >
> > On Fri, 22 Dec 2017 at 03:25 Carsten Haitzler 
> wrote:
> >
> > > On Thu, 21 Dec 2017 13:15:26 + Andrew Williams <
> a...@andywilliams.me>
> > > said:
> > >
> > > > Hi,
> > > >
> > > > This is now well documented (
> > > > https://www.enlightenment.org/develop/tutorials/c/eo-refcount.md)
> but
> > > the
> > > > more I use efl_add the more I feel it is confusing especially to new
> > > > developers.
> > > >
> > > > In the current model (if I understand it correctly)
> > > > 1) child = efl_add(klass, parent) means the child must NOT be
> > > unfeferenced
> > > > 2) child = efl_add(klass, NULL) means the child should be
> unreferenced
> > > > 3) child = efl_add_ref(klass, parent) means the child must be
> > > unreferenced
> > > > 4) child = efl_add_ref(klass, NULL) somehow means that the child
> should
> > > be
> > > > unreferenced twice
> > >
> > > #4 smells like a bug... 1 is intended.
> > >
> > > > In my opinion 1) and 4) are peculiar and so I provide a proposal to
> fix
> > > > this:
> > > >
> > > > We could change efl_add to return void. It never retains a
> reference. If
> > > > the parent is 

Re: [E-devel] efl_add causing confusion

[Carsten Haitzler]

On Fri, 22 Dec 2017 09:43:20 + Andrew Williams  said:

> Hi,
> 
> I think that maybe I could have explained this a little better, let me step
> back from the implementation details for a minute.
> 
> As a user of this API all I want to know is do I have to unref a reference
> that I have been handed. From this point of view we should be consistent.
> My proposal was (intended) to mean that efl_add should never require the
> user to unreference whereas efl_add_ref should always require this (but
> only once - that may have been a bug).
> 
> Having to know the value of the second parameter to the method to ascertain
> the correct behaviour is what I think is confusing and should be
> eliminated. Therefore the main purpose was so that the docs could ready
> simply "this is a borrowed reference which you should not unref" vs "this
> is an owned reference and you must unref when done".

yes. i got that. but your proposal is no better than what is there. in fact
think it's worse. unless i didn't understand it?

i get your point... as i said. but your proposal doesn't sound as if it's
better.

what MIGHT be better is to class objects into "can have a null parent" and
"must have a non-null parent". the 1st case doesn't change. from now. the 2nd
case results in efl_add returning NULL (construction fails) if the parent is
null. that might be an improvement.

> The added complication is the race condition with the "parent" usage in
> efl_add. When I pass a parent reference to efl_add then the returned handle
> is something I can use. Yes this is very helpful. But this is dangerous. We
> (as a framework) are providing 0 guarantee that the object will live as
> long as the scope - that is entirely up to the parent, not the user and not
> the framework. This is also something that I think we should address.

we can't guarantee it if the parent decides some time within the scope to
delete the object you added. well we can;'t unless you use efl_add_ref() then
you create an object with 2 references (parent one and your scope/app one) and
you will have to remember to unref at end of scope. this leads to verbose code,
and having to remember to do this... which is more than likely going to lead to
huge amounts of memory leaking.

so which is worse? sometimes a parent decides to delete an object on you while
you are still holding and using its handle, or that you always have to unref
every obj you added when you exit scope? i'll take the first one any day of the
week. :)

> So there you go - the background behind my proposal. Hopefully if I did not
> explain the details well you can now see why I felt it is important.

yup. i know the background. i'm speaking of the details of the proposal.

> Thanks,
> Andy
> 
> On Fri, 22 Dec 2017 at 03:25 Carsten Haitzler  wrote:
> 
> > On Thu, 21 Dec 2017 13:15:26 + Andrew Williams 
> > said:
> >
> > > Hi,
> > >
> > > This is now well documented (
> > > https://www.enlightenment.org/develop/tutorials/c/eo-refcount.md) but
> > the
> > > more I use efl_add the more I feel it is confusing especially to new
> > > developers.
> > >
> > > In the current model (if I understand it correctly)
> > > 1) child = efl_add(klass, parent) means the child must NOT be
> > unfeferenced
> > > 2) child = efl_add(klass, NULL) means the child should be unreferenced
> > > 3) child = efl_add_ref(klass, parent) means the child must be
> > unreferenced
> > > 4) child = efl_add_ref(klass, NULL) somehow means that the child should
> > be
> > > unreferenced twice
> >
> > #4 smells like a bug... 1 is intended.
> >
> > > In my opinion 1) and 4) are peculiar and so I provide a proposal to fix
> > > this:
> > >
> > > We could change efl_add to return void. It never retains a reference. If
> > > the parent is NULL then it should be automatically unref before
> > returning.
> > > Then efl_add_ref would be changed to mirror this and always retain
> > exactly
> > > 1 reference - so if parent is NULL there is no double count returned.
> >
> > umm... then you are saying efl_add_ref() is exactly the same as efl_add()
> > today. what does this help? and the shorter efl_add now returns nothing so
> > i
> > can't use the return to usefully access the things i created later on like
> > add
> > callbacks to it, change the label of a button, delete a window, or use it
> > as a
> > parent for further adds which is like THE most common use case around when
> > building a ui for example (create box, then create a button and pack button
> > into box. i need the box to be able to do that). unless i use efl_add_ref
> > which
> > si the same thing as efl_add now.
> >
> > > Using this model if an Eo * is returned then I know I have a reference
> > and
> > > must later unref.
> > > Any need for using the pointer in an efl_add (which is no longer
> > returned)
> > > would still be supported through efl_added within the constructor.
> >
> > if efl_add_ref returns an obj 

Re: [E-devel] efl_add causing confusion

[Andrew Williams]

Hi,

Surely hide and unref are separate? Why are we trying to do both with a
single call to _del? If we wish to continue supporting "hide and unref" in
a single call then we can - but as a convenience. It could be documented as
such: "del will remove your reference to the object and, if the object is
visible, hide it. Any other references will remain valid but will no longer
visibly affect the object" nice and clear.

But I think cedric is still correct - unref is all that is "needed" - any
user could _hide && _unref if they wanted - it would then be exactly the
same as the _del semantic you describe.

If we have to put a big OR in a simple method documentation then we are
setting up confusion.

Andy

On Fri, 22 Dec 2017 at 03:26 Carsten Haitzler <ras...@rasterman.com> wrote:

> On Thu, 21 Dec 2017 13:29:05 -0500 Cedric Bail <ced...@ddlm.me> said:
>
> > Hi,
> >
> > > ---- Original Message 
> > > Subject: Re: [E-devel] efl_add causing confusion
> > > Local Time: December 21, 2017 9:02 AM
> > > UTC Time: December 21, 2017 5:02 PM
> > > From: a...@andywilliams.me
> > > To: Enlightenment developer list <
> enlightenment-devel@lists.sourceforge.net>
> > >
> > > P.S. if the loss of temporary handles with efl_add is not met with
> > > efl_added it would be possible to add a new macro along the lines of:
> > >
> > > efl_add_scope(klass, parent) { ... statements ... } whereby the scope
> of
> > > the variable is valid only within that block.
> >
> > This is an interesting idea that give an explicit lifecycle to the newly
> > created reference and when you safely own it.
> >
> > > On Thu, 21 Dec 2017 at 13:15 Andrew Williams a...@andywilliams.me
> wrote:
> > >
> > >> Hi,
> > >> This is now well documented (
> > >> https://www.enlightenment.org/develop/tutorials/c/eo-refcount.md)
> but the
> > >> more I use efl_add the more I feel it is confusing especially to new
> > >> developers.
> > >> In the current model (if I understand it correctly)
> > >>
> > >> - child = efl_add(klass, parent) means the child must NOT be
> unfeferenced
> > >> - child = efl_add(klass, NULL) means the child should be unreferenced
> > >> - child = efl_add_ref(klass, parent) means the child must be
> unreferenced
> > >> - child = efl_add_ref(klass, NULL) somehow means that the child
> should be
> > >> unreferenced twice
> > >>
> > >> In my opinion 1) and 4) are peculiar and so I provide a proposal to
> fix
> > >> this:
> > >> We could change efl_add to return void. It never retains a reference.
> If
> > >> the parent is NULL then it should be automatically unref before
> returning.
> > >> Then efl_add_ref would be changed to mirror this and always retain
> exactly
> > >> 1 reference - so if parent is NULL there is no double count returned.
> > >> Using this model if an Eo * is returned then I know I have a
> reference and
> > >> must later unref.
> > >> Any need for using the pointer in an efl_add (which is no longer
> returned)
> > >> would still be supported through efl_added within the constructor.
> > >> What do people think about this? I put the suggestion forward to
> improve
> > >> the symettry with add and unref which is currently confusing. If my
> > >> assumptions above are incorrect please let me know!
> >
> > We have been trying to fix the semantic problem of efl_add since pretty
> much
> > it was created. This proposal seems to build on top of efl_add_ref that
> was
> > added to fix the problem created for binding. I do like this proposal at
> it
> > start fixing the problem of inconsistency when creating an object. An
> > additional fix is that we would not need any more an efl_del as efl_unref
> > will be the only thing needed (efl_del is a weird thing that does unset
> the
> > parent and unref somehow, but can't also be used safely by binding). This
> > would make for a cleaner semantic and less surprise in refcounting.
>
> i disagree that unref is all that is needed. here is a case we hit in
> legacy
> api:
>
> you have an object and you do evas_object_del(). you can keep the object
> alive
> with more refs, but del SHOULD hide the object. refs should not affect
> VISIBILITY of the object, thus a del is different to an unref.
>
> del == "remove a reference from this object and otherwise act as if this
> object
> should now be delet

Re: [E-devel] efl_add causing confusion

[Andrew Williams]

Hi,

I think that maybe I could have explained this a little better, let me step
back from the implementation details for a minute.

As a user of this API all I want to know is do I have to unref a reference
that I have been handed. From this point of view we should be consistent.
My proposal was (intended) to mean that efl_add should never require the
user to unreference whereas efl_add_ref should always require this (but
only once - that may have been a bug).

Having to know the value of the second parameter to the method to ascertain
the correct behaviour is what I think is confusing and should be
eliminated. Therefore the main purpose was so that the docs could ready
simply "this is a borrowed reference which you should not unref" vs "this
is an owned reference and you must unref when done".

The added complication is the race condition with the "parent" usage in
efl_add. When I pass a parent reference to efl_add then the returned handle
is something I can use. Yes this is very helpful. But this is dangerous. We
(as a framework) are providing 0 guarantee that the object will live as
long as the scope - that is entirely up to the parent, not the user and not
the framework. This is also something that I think we should address.

So there you go - the background behind my proposal. Hopefully if I did not
explain the details well you can now see why I felt it is important.

Thanks,
Andy

On Fri, 22 Dec 2017 at 03:25 Carsten Haitzler  wrote:

> On Thu, 21 Dec 2017 13:15:26 + Andrew Williams 
> said:
>
> > Hi,
> >
> > This is now well documented (
> > https://www.enlightenment.org/develop/tutorials/c/eo-refcount.md) but
> the
> > more I use efl_add the more I feel it is confusing especially to new
> > developers.
> >
> > In the current model (if I understand it correctly)
> > 1) child = efl_add(klass, parent) means the child must NOT be
> unfeferenced
> > 2) child = efl_add(klass, NULL) means the child should be unreferenced
> > 3) child = efl_add_ref(klass, parent) means the child must be
> unreferenced
> > 4) child = efl_add_ref(klass, NULL) somehow means that the child should
> be
> > unreferenced twice
>
> #4 smells like a bug... 1 is intended.
>
> > In my opinion 1) and 4) are peculiar and so I provide a proposal to fix
> > this:
> >
> > We could change efl_add to return void. It never retains a reference. If
> > the parent is NULL then it should be automatically unref before
> returning.
> > Then efl_add_ref would be changed to mirror this and always retain
> exactly
> > 1 reference - so if parent is NULL there is no double count returned.
>
> umm... then you are saying efl_add_ref() is exactly the same as efl_add()
> today. what does this help? and the shorter efl_add now returns nothing so
> i
> can't use the return to usefully access the things i created later on like
> add
> callbacks to it, change the label of a button, delete a window, or use it
> as a
> parent for further adds which is like THE most common use case around when
> building a ui for example (create box, then create a button and pack button
> into box. i need the box to be able to do that). unless i use efl_add_ref
> which
> si the same thing as efl_add now.
>
> > Using this model if an Eo * is returned then I know I have a reference
> and
> > must later unref.
> > Any need for using the pointer in an efl_add (which is no longer
> returned)
> > would still be supported through efl_added within the constructor.
>
> if efl_add_ref returns an obj with only 1 reference, then this is wrong
> above.
> if parent is NULL, yes you'd have to unref/del. if parent is not null then
> there is still only 1 ref and it belongs to the parent object. so
>
> > What do people think about this? I put the suggestion forward to improve
> > the symettry with add and unref which is currently confusing. If my
> > assumptions above are incorrect please let me know!
> >
> > Thanks,
> > Andy
> > --
> > http://andywilliams.me
> > http://ajwillia.ms
> >
> --
> > 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
>
> --
http://andywilliams.me
http://ajwillia.ms
--
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

Re: [E-devel] efl_add causing confusion

[Carsten Haitzler]

On Thu, 21 Dec 2017 13:15:26 + Andrew Williams  said:

> Hi,
> 
> This is now well documented (
> https://www.enlightenment.org/develop/tutorials/c/eo-refcount.md) but the
> more I use efl_add the more I feel it is confusing especially to new
> developers.
> 
> In the current model (if I understand it correctly)
> 1) child = efl_add(klass, parent) means the child must NOT be unfeferenced
> 2) child = efl_add(klass, NULL) means the child should be unreferenced
> 3) child = efl_add_ref(klass, parent) means the child must be unreferenced
> 4) child = efl_add_ref(klass, NULL) somehow means that the child should be
> unreferenced twice

#4 smells like a bug... 1 is intended.

> In my opinion 1) and 4) are peculiar and so I provide a proposal to fix
> this:
> 
> We could change efl_add to return void. It never retains a reference. If
> the parent is NULL then it should be automatically unref before returning.
> Then efl_add_ref would be changed to mirror this and always retain exactly
> 1 reference - so if parent is NULL there is no double count returned.

umm... then you are saying efl_add_ref() is exactly the same as efl_add()
today. what does this help? and the shorter efl_add now returns nothing so i
can't use the return to usefully access the things i created later on like add
callbacks to it, change the label of a button, delete a window, or use it as a
parent for further adds which is like THE most common use case around when
building a ui for example (create box, then create a button and pack button
into box. i need the box to be able to do that). unless i use efl_add_ref which
si the same thing as efl_add now.

> Using this model if an Eo * is returned then I know I have a reference and
> must later unref.
> Any need for using the pointer in an efl_add (which is no longer returned)
> would still be supported through efl_added within the constructor.

if efl_add_ref returns an obj with only 1 reference, then this is wrong above.
if parent is NULL, yes you'd have to unref/del. if parent is not null then
there is still only 1 ref and it belongs to the parent object. so

> What do people think about this? I put the suggestion forward to improve
> the symettry with add and unref which is currently confusing. If my
> assumptions above are incorrect please let me know!
> 
> Thanks,
> Andy
> -- 
> http://andywilliams.me
> http://ajwillia.ms
> --
> 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

Re: [E-devel] efl_add causing confusion

[Carsten Haitzler]

On Thu, 21 Dec 2017 13:29:05 -0500 Cedric Bail <ced...@ddlm.me> said:

> Hi,
> 
> >  Original Message ----
> > Subject: Re: [E-devel] efl_add causing confusion
> > Local Time: December 21, 2017 9:02 AM
> > UTC Time: December 21, 2017 5:02 PM
> > From: a...@andywilliams.me
> > To: Enlightenment developer list <enlightenment-devel@lists.sourceforge.net>
> >
> > P.S. if the loss of temporary handles with efl_add is not met with
> > efl_added it would be possible to add a new macro along the lines of:
> >
> > efl_add_scope(klass, parent) { ... statements ... } whereby the scope of
> > the variable is valid only within that block.
> 
> This is an interesting idea that give an explicit lifecycle to the newly
> created reference and when you safely own it.
> 
> > On Thu, 21 Dec 2017 at 13:15 Andrew Williams a...@andywilliams.me wrote:
> >
> >> Hi,
> >> This is now well documented (
> >> https://www.enlightenment.org/develop/tutorials/c/eo-refcount.md) but the
> >> more I use efl_add the more I feel it is confusing especially to new
> >> developers.
> >> In the current model (if I understand it correctly)
> >>
> >> - child = efl_add(klass, parent) means the child must NOT be unfeferenced
> >> - child = efl_add(klass, NULL) means the child should be unreferenced
> >> - child = efl_add_ref(klass, parent) means the child must be unreferenced
> >> - child = efl_add_ref(klass, NULL) somehow means that the child should be
> >> unreferenced twice
> >>
> >> In my opinion 1) and 4) are peculiar and so I provide a proposal to fix
> >> this:
> >> We could change efl_add to return void. It never retains a reference. If
> >> the parent is NULL then it should be automatically unref before returning.
> >> Then efl_add_ref would be changed to mirror this and always retain exactly
> >> 1 reference - so if parent is NULL there is no double count returned.
> >> Using this model if an Eo * is returned then I know I have a reference and
> >> must later unref.
> >> Any need for using the pointer in an efl_add (which is no longer returned)
> >> would still be supported through efl_added within the constructor.
> >> What do people think about this? I put the suggestion forward to improve
> >> the symettry with add and unref which is currently confusing. If my
> >> assumptions above are incorrect please let me know!
> 
> We have been trying to fix the semantic problem of efl_add since pretty much
> it was created. This proposal seems to build on top of efl_add_ref that was
> added to fix the problem created for binding. I do like this proposal at it
> start fixing the problem of inconsistency when creating an object. An
> additional fix is that we would not need any more an efl_del as efl_unref
> will be the only thing needed (efl_del is a weird thing that does unset the
> parent and unref somehow, but can't also be used safely by binding). This
> would make for a cleaner semantic and less surprise in refcounting.

i disagree that unref is all that is needed. here is a case we hit in legacy
api:

you have an object and you do evas_object_del(). you can keep the object alive
with more refs, but del SHOULD hide the object. refs should not affect
VISIBILITY of the object, thus a del is different to an unref.

del == "remove a reference from this object and otherwise act as if this object
should now be deleted".

unref == "just remove my reference to this object. this MAY result in a del
behavior at some point, but i'm simply releasing a reference and not actually
explicitly ending the lifetime of this object".

semantically they are quite different things.
-- 
- 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

Re: [E-devel] efl_add causing confusion

[Cedric Bail]

Hi,

>  Original Message 
> Subject: Re: [E-devel] efl_add causing confusion
> Local Time: December 21, 2017 9:02 AM
> UTC Time: December 21, 2017 5:02 PM
> From: a...@andywilliams.me
> To: Enlightenment developer list <enlightenment-devel@lists.sourceforge.net>
>
> P.S. if the loss of temporary handles with efl_add is not met with
> efl_added it would be possible to add a new macro along the lines of:
>
> efl_add_scope(klass, parent) { ... statements ... } whereby the scope of
> the variable is valid only within that block.

This is an interesting idea that give an explicit lifecycle to the newly 
created reference and when you safely own it.

> On Thu, 21 Dec 2017 at 13:15 Andrew Williams a...@andywilliams.me wrote:
>
>> Hi,
>> This is now well documented (
>> https://www.enlightenment.org/develop/tutorials/c/eo-refcount.md) but the
>> more I use efl_add the more I feel it is confusing especially to new
>> developers.
>> In the current model (if I understand it correctly)
>>
>> - child = efl_add(klass, parent) means the child must NOT be unfeferenced
>> - child = efl_add(klass, NULL) means the child should be unreferenced
>> - child = efl_add_ref(klass, parent) means the child must be unreferenced
>> - child = efl_add_ref(klass, NULL) somehow means that the child should be
>> unreferenced twice
>>
>> In my opinion 1) and 4) are peculiar and so I provide a proposal to fix
>> this:
>> We could change efl_add to return void. It never retains a reference. If
>> the parent is NULL then it should be automatically unref before returning.
>> Then efl_add_ref would be changed to mirror this and always retain exactly
>> 1 reference - so if parent is NULL there is no double count returned.
>> Using this model if an Eo * is returned then I know I have a reference and
>> must later unref.
>> Any need for using the pointer in an efl_add (which is no longer returned)
>> would still be supported through efl_added within the constructor.
>> What do people think about this? I put the suggestion forward to improve
>> the symettry with add and unref which is currently confusing. If my
>> assumptions above are incorrect please let me know!

We have been trying to fix the semantic problem of efl_add since pretty much it 
was created. This proposal seems to build on top of efl_add_ref that was added 
to fix the problem created for binding. I do like this proposal at it start 
fixing the problem of inconsistency when creating an object. An additional fix 
is that we would not need any more an efl_del as efl_unref will be the only 
thing needed (efl_del is a weird thing that does unset the parent and unref 
somehow, but can't also be used safely by binding). This would make for a 
cleaner semantic and less surprise in refcounting.

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

Re: [E-devel] efl_add causing confusion

[Andrew Williams]

P.S. if the loss of temporary handles with efl_add is not met with
efl_added it would be possible to add a new macro along the lines of:

efl_add_scope(klass, parent) { ... statements ... } whereby the scope of
the variable is valid only within that block.

On Thu, 21 Dec 2017 at 13:15 Andrew Williams  wrote:

> Hi,
>
> This is now well documented (
> https://www.enlightenment.org/develop/tutorials/c/eo-refcount.md) but the
> more I use efl_add the more I feel it is confusing especially to new
> developers.
>
> In the current model (if I understand it correctly)
> 1) child = efl_add(klass, parent) means the child must NOT be unfeferenced
> 2) child = efl_add(klass, NULL) means the child should be unreferenced
> 3) child = efl_add_ref(klass, parent) means the child must be unreferenced
> 4) child = efl_add_ref(klass, NULL) somehow means that the child should be
> unreferenced twice
>
> In my opinion 1) and 4) are peculiar and so I provide a proposal to fix
> this:
>
> We could change efl_add to return void. It never retains a reference. If
> the parent is NULL then it should be automatically unref before returning.
> Then efl_add_ref would be changed to mirror this and always retain exactly
> 1 reference - so if parent is NULL there is no double count returned.
>
> Using this model if an Eo * is returned then I know I have a reference and
> must later unref.
> Any need for using the pointer in an efl_add (which is no longer returned)
> would still be supported through efl_added within the constructor.
>
> What do people think about this? I put the suggestion forward to improve
> the symettry with add and unref which is currently confusing. If my
> assumptions above are incorrect please let me know!
>
> Thanks,
> Andy
> --
> http://andywilliams.me
> http://ajwillia.ms
>
-- 
http://andywilliams.me
http://ajwillia.ms
--
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