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 <ras...@rasterman.com> wrote: > On Fri, 22 Dec 2017 11:44:32 +0000 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 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 <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 > > > > > -- > ------------- 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
