On Thu, Jan 11, 2018 at 5:54 AM, Carsten Haitzler <ras...@rasterman.com> 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 always start with a parent (as is central to this discussion) > then worrying about the other code path is kind of pointless. :) and this is confusing! >> 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. > > i like it because it is easy. in c. because you have no automatic cleanup on > exit from scope. > >> 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)); > > why would there need to be an explicit adopt? adding a new object with a > parent > "container" already says exactly that. the container_pack is only needed as an > explicit "i want you to pack in the parent THIS way (at the end/start of a box > or at specific x/y/col/rows in a table etc.). in this example (it's just PART of the rationale to the overall thing), you're transferring YOUR reference to the container. That's why. >> 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. > > but parent obj already does this... it just makes everything so pointlessly > verbose. the add with a parent is telling the parent to take ownership of the > object. the handle you are returned is for usability so you can call methods > on > it (outside of the add scope). just what you need for code like: > > o = efl_add(CLASS, parent); > if (x) efl_whatever_x(o); > else efl_thing_y(o); > > yes. technically it can be put inside the add() but eventually you'll want > something complex enough where it just doesn't look right there. > >> others may claim efl_adopt_ref() is always the case, in that case >> keeping efl_invalidate() UNRELATED to efl_unref() is also helpful: > > i think they are unrelated except that if invalidation is a standard part of > the destruction cycle then it must be called when refs go to 0 even if that's > thanks to an unref. > >> --> 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 >> 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 >> >> >> as a last note to this long email, shall we opt to use the "parent >> keeps a ref" method, since we use the eoid abstraction, I'd highly >> recommend that efl_ref() creates ANOTHER eoid for the same object, >> with associated WARN_UNUSED_RESULT/MALLOC and efl_unref() behaves like >> free(), considering the handle to be destroyed. This should help >> finding bugs... it's worth since efl_ref() will be rarely used. > > oh no no. efl_ref() is lightweight and slim. creating more eoid's is going to > be super heavy in comparison. point is: you shouldn't need efl_ref() or efl_unref() if the ownership is clear. Currently it isn't, thus we need to safeguard with things such as efl_ref() and efl_unref() when calling back the user from non-eo methods (ie: callback from other object), as suddenly the object may go away. At least in cases where I'm dealing with children objects, I'd own reference to them automatically, they can never be deleted (or unparented) letting me get a deleted object when I return to my context. also, this would help with your quest to spot incorrect usages, you couldn't unref more than once the same id, it would complain... and would be easy to track leaks, since you could associate efl_ref() with the calling backtrace. -- 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
