The proposal has 0 occurrences of “NULL”. Andy
On Sun, 24 Dec 2017 at 03:33, Carsten Haitzler <ras...@rasterman.com> wrote: > On Sat, 23 Dec 2017 11:26:42 +0000 Andrew Williams <a...@andywilliams.me> > 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 <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 > > > -- > ------------- 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
