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 <ras...@rasterman.com> wrote: > On Fri, 22 Dec 2017 10:10:48 +0000 Andrew Williams <a...@andywilliams.me> > said: > > > On Fri, 22 Dec 2017 at 09:59 Carsten Haitzler <ras...@rasterman.com> > wrote: > > > > > On Fri, 22 Dec 2017 09:43:20 +0000 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 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 (as the caller) the reference. > unref/del as appropriate. if parent is NOT NULL then parent owns the ref > and > the object lifecycle will be managed by the parent. what is not clear about > this? how is "well if you write it correctly and remember to add all the > unref's when exiting scope" better? it's MORE code and MORE places for > things > to go wrong. it's also not a common pattern in C. see the gtk samples and > legacy efl api too. > > > > 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. > > then you are free to use efl_add_ref(). reality is that 99.9% of the time > objects will not be deleted within scope where they are created and set up > because having this kind of behaviour is kind of nasty and it should be > avoided. we have safety thanks to eoid anyway. so you won't get a crash.. > you'll get an error print and things will recover just fine. because there > is > no automatic unreffing when exiting scope in C, requiring people to do > this by > hand is going to lead a world of leaks where people continue to forget > doing > this as this is not a rare thing but a common thing to do, and it's going > to > lead to more verbose code... in return for what? safety isn't improved. > eoid > took care of that already. > > this whole discussion was already had like 2+ years ago when the core > design > for eo was being hashed out. tom was like you wanting something > pedantically > correct requiring unrefs at end of scope etc. yes i know. the option of an > add > that doesn't return anything. that's kind of a very half-baked and not > useful > api. he ended up agreeing that what we have now is not perfect, but it's > the > best option for sanity. now we're having this discussion again and TBH i > think > it's a bit late. there is a mountain of code that now depends on this > design. > even if we were to choose to change - it'd be a nightmare to do and have > efl in > a horribly unstable state for weeks if not months. ever use where the > return is > used has to become an efl_add_ref, and those that done become efl_add so > every > instance needs inspection etc. and someone has to put the efl_unrefs in > all of > the scope exit points AND get it all right... > > 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. > > > > > 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 <ras...@rasterman.com> > > > wrote: > > > > > > > > > On Thu, 21 Dec 2017 13:15:26 +0000 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 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 > > > > > > > > > -- > > > ------------- Codito, ergo sum - "I code, therefore I am" > -------------- > > > Carsten Haitzler - ras...@rasterman.com > > > > > > -- > > http://andywilliams.me > > http://ajwillia.ms > > > -- > ------------- 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
