GNOME Bugzilla – Bug 447967
Improve reference counting
Last modified: 2011-02-04 16:11:04 UTC
The reference counting in GtkBuilder is not ideal the way it's currently implemented. Especially GObjects needs to be unreferenced if nobody is claiming their ownership. I discussed this with James, here are some notes from the conversation: Builder should have two hash tables: 1: name -> object 2: object ->name. There should be a weak ref handler on each object which does: 1. lookup object name in second hash 2. lookup object in first hash 3. If objects are the same, remove entry from 1st hash 4. remove entry from 2nd hash which frees the name In the builders finalize, do the following: 1. g_hash_table_foreach_remove on 2nd hash table * remove reak ref from object * return True to remove entry from hash table 2. unref 1st and 2nd hash table For GObjects where we own the reference return by g_object_new, add to a list when building is finished unref each object in list. should work for size groups, tree models, text buffers, but don't do this for GtkWindow, which is floating.
I don't think that GtkWindow can be floating. Child GtkWidgets can be floating. The test of this is whether you can document what should be done with the result of gtk_builder_get_object(). I suggest that you keep it consistent with GladeXml, with an additional rule for non-widget GObjects: For widgets: 1. For child widgets, such as GtkButton, they belong to their parent containers and will be destroyed by them when the containers are destroyed, so you don't need to call g_object_unref() or gtk_widget_destroy(). This is like doing gtk_button_new() followed by gtk_container_add() and then continuing to use the GtkButton*. 2. For top-level windows, such as GtkWindow, they should be destroyed with gtk_widget_destroy() when you are finished with them. This is like doing gtk_window_new(). But for objects: 3. For objects, such as GtkTreeModel, g_object_unref() should be called. This would require that a reference is given when glade_xml_get_widget() is called a second time, as well as the first time. This would be inconsistent with other GTK+ API, but it does not seem sensible to expect the application code to know when the result has just been instantiated rather than simply returned again.
Some comments: Re James' proposal: - the second table is not necessary, since the buildable already stores its name itself, no ? - windows are not floating, GTK+ holds a reference to them, which is why they need to be explicitly destroyed Re Murray's comments: I don't think the get_object() function should return a new reference; GTK+ doesn't have reffing getters, and glade_xml_get_widget() does not return a new reference either. The documentation should state that the caller needs to take a reference if he wants to keep the object beyond the lifecycle of the builder. That leaves us with a slight problems for toplevel windows - it is not clear how dispose of toplevel windows that have not been "taken over" by the application. This is a point where partial tree building will be externally visible, since it makes a difference if you get a pointer to a button that sits inside a window that you never use or if the button is constructed on its own without a parent - in the latter case it will be floating and have no parent, in the former case it is not floating and has a parent.
I'm not sure this is really a bug. I think its simpler to say "whatever the builder created, was created on your behalf and will be created with the expected refcounts depending on whether it was GInitiallyUnowned parented or whatever". If we add explicit references from the builder to the built objects then complexity and margin of error will increase, just because we dont think that orphaned built objects should be leeked ? What if I want to optionally add a debug object that periodically dumps stats to a file if its included in the builder file ?
Except from discussion during the #gtk-devel meeting earlier today: <jdahlin> strictly speaking, it won't require API changes, but it's important to get it right before people starts using it <mclasen> jdahlin: so, what exactly is the current proposal for how things should work ? <jdahlin> mclasen: I posted a little bit of my thoughts to the mailing list earlier today <jdahlin> I'd like to make it possible to unref the builder and make all objects not fetched (or references by a fetched object) to be freed as soon as you unref the builder itself <jdahlin> it might be more than we actually need though, the memory used by the builder is much less than the memory used by libglade <mclasen> jdahlin: that should work fine for objects, but for toplevel windows, it doesn't quite work, I think <jdahlin> so it might not matter too much to have the builder lying around as long as the process/interface live * tristan just commented on that bug <tristan> sorry for being late in catching up <tristan> so, we want orphaned objects to be cleaned up... why ? <tbf> as outlined already today: keeping the builder around can make things quite convinient <jdahlin> mclasen: so what should be done for top-levels? <mclasen> jdahlin: I think if they are not fetched and destroyed by the app, they are leaked <jdahlin> mclasen: right, that's what we need to decide <mclasen> the builder could destroy them, but it is a bit hard to know for the builder which ones to destroy and which ones not <jdahlin> should the gtkbuilder claim ownership of all objects, or is it up to the application? <tristan> I think everything should be leeked * tristan thinks the latter completely <jdahlin> rambokid: do you have an opinion? <tbf> objects passed to the outside by the builder could have their ref-count raised <tristan> an object that was built by the builder but never seen by the application code, might still be doing something usefull too <mclasen> tbf: no, we don't have reffing getters, the app needs to ref itself <tbf> so everything attached would stay alive, even even if the builder gets destroyed <tbf> mclasen: so the methods cannot have an "_get_" in their name <jdahlin> gtk_builder_steal_object() ? :) <pbor> the builder is a sort of factory so returning a reffed object may make sense... <tbf> pbor: +1 <mclasen> if we leak everything (ie don't unref when the builder goes away), then you don't have to return a new reference <mclasen> the application can just use the initial one <rambokid> jdahlin: i'm not quite sure what the problem is. isn't it ok if the builder returns objects like they are created? i.e. non-toplevels are floating, and toplevels are being refernced by the toplevel-list anyways... <tristan> mclasen, nod, which is classic libglade behaviour <rambokid> i.e. make it behave exactly like g_object_new <mclasen> I think that is fine <jdahlin> rambokid: the problem, in my opinion is that the application developer needs to know about all the objects created by a userinterface (models, ui manager, sizegroups etc) and unref them when not using them any longer <tristan> jdahlin, if they put unused objects in the interface, they are to blame for not using them no ? <tristan> its like calling g_object_new() and then forgetting the return value <mclasen> the problematic aspect I see is that the builder returns a bunch of objects in different states, ownership-wise <mclasen> some are toplevels that need to be destroyed, some are floating, <jdahlin> tristan: it's more like that the application developer might not be aware that a treemodel or sizegroup is created when a certain operation is done in the user interface designer <mclasen> some are straight objects <tristan> jdahlin, thats another can of worms though <tristan> and more importantly - an object never seen by the application code might still be doing something important. <rambokid> urm, no. the application developer should *only* deal with toplevel handles. that is not a problem for non-toplevel widgets that are children, since those are referenced by their containers. so the only questionable thing are non-widget objects, right? in general, keeping such widgets around only makes sense in the context of a window. so the best thing to do here is to g_object_set_qdata_full(window, "key-for-this-object-kind", sinke_ref(o <rambokid> bject), g_object_unref). for some types, e.g. size groups, you need a list though. <tko> I'd think everything should be owned by the builder until you ask for it (gtk_builder_ref_object ?) <tbf> what about a destructor for gtk builder similiar to g_string_free(str, free)? <mclasen> earlier there was talk of allowing non-toplevel widgets as root objects in builder files <tristan> mclasen, I believe thats already allowed <tristan> and rambokid, usually you dont care about dealing with the window, you just want a handle on some button so you can set it insensitive later on <jdahlin> yes, but if we introduce the concept rambokid just mentioned we'd have to treat toplevels which are widgets but not windows differently <rambokid> tristan: you always care aout a window, because a window is the only thing that makes sense showing and hiding and every app has to decide that individually for every window. <tristan> and all that complexity scares the crap out of me <tristan> rambokid, sure, unless its initially visible ok. <jdahlin> tristan: that's mostly a bad idea anyway <rambokid> jdahlin: not really, you could do the same for those, i.e. attach references as QData. the only difference is that you hand them out floating, i.e. still you behave like g_object_new <jdahlin> rambokid: okay, that sounds reasonable <tristan> I think that if I asked the builder to build something and it dissapears after unreffing the builder, thats a little unfair <pbor> rambokid: not at all, in the app I just care about a window, but enforcing to create the whole window from builder is wrong... I may just want to build some part of the ui and stick it in a sidepane of my main app window <tristan> but also, if toplevels, non-widget GObjects and GTK_WIDGET_TOPLEVEL objects all get cased differently... only chaos can ensue :-S <rambokid> tristan: also, windows are the single object class that the developers have to conciously destroy, so there's no way of getting rid of dealing with a window handle for a developer. but from that, everytihng else can automatically follow via qdata and ref_sink/unref <tristan> rambokid, what about actions ? <rambokid> tristan: absolutely, especially since ref/unref is a memory managment issue, and widget appears/dosappears is a user visible effect, i.e. part of the user experience/interface. memory management issues should *never* have user visible effects (i.e. have widgets disappear because a gc is run ;) <tristan> they have to be explicitly destroyed I would think <pbor> anyway, it seems that builder refcounting still needs some thought... maybe this discussion should be carried on in bugzilla/mailinglist and we should move on? <tristan> pbor++ <jdahlin> true <rambokid> pbor: right, i'm not saying the builder should be constrained to toplevel creation. the point is just to keep all non-widget references from the "toplevel" object created by the builder and handed out to the user. and that could still be a floating non-window object (i.e. mimicking g_object_new which you can also use for windows and widgets)
Created attachment 90727 [details] [review] patch This patch implements the scheme suggested by Tim; * GtkWindows needs to be destroyed manually, it's the responsibility is the application developers to do so * GObjects which are referenced by a widget are ref_sinked when parsing is finished, that reference is removed when the toplevel window is destroyed * Floating toplevel objects are ref_sinked as soon as they are constructed, not sure this is correct, but it makes non-GtkWindow toplevels work valgrind tell me that buildertest is leak free
> * GObjects which are referenced by a widget are ref_sinked when parsing is > finished, that reference is removed when the toplevel window is destroyed So, does this meant that I have to take a reference to, for instance, a GtkTreeModel that I get from gtk_builder_get_object(), both the first time and subsequent times. That's OK. I think the documentation should be in terms of what the application coder should do, rather than the difficult ref_sink concept. > Floating toplevel objects are ref_sinked as soon as they are constructed, not > sure this is correct, but it makes non-GtkWindow toplevels work. So they won't leak even if they are not added to a container and not explicitly unrefed or destroyed?
(In reply to comment #6) > > * GObjects which are referenced by a widget are ref_sinked when parsing is > > finished, that reference is removed when the toplevel window is destroyed > > So, does this meant that I have to take a reference to, for instance, a > GtkTreeModel that I get from gtk_builder_get_object(), both the first time and > subsequent times. That's OK. I think the documentation should be in terms of > what the application coder should do, rather than the difficult ref_sink > concept. It depends what you want to do with it. If you want to use it outside of the interface constructed by builder yes, then you have to manually ref it if you want it to survive after you destroyed the toplevel of the widget referencing the model. You'd also have to ref the model if there are no objects referencing the object inside the xml. > > Floating toplevel objects are ref_sinked as soon as they are constructed, not > > sure this is correct, but it makes non-GtkWindow toplevels work. > > So they won't leak even if they are not added to a container and not explicitly > unrefed or destroyed? The idea was to make them similar to GtkWindow, so they should be explicitly unreffed.
> The idea was to make them similar to GtkWindow, so they should be explicitly unreffed. So, every call will give me a reference, or just the first call? I'm assuming that a second call will not instantiate a second instance.
(In reply to comment #8) > > The idea was to make them similar to GtkWindow, so they should be explicitly > unreffed. > > So, every call will give me a reference, or just the first call? I'm assuming > that a second call will not instantiate a second instance. Each call to gtk_builder_get_object is just representing a hash table lookup, it will not instantiate or increase the reference count. What will remove the floating reference is the fact that it is a non-GtkWindow toplevel object in the xml and has the floating flag set.
(In reply to comment #5) > Created an attachment (id=90727) [edit] > patch > > This patch implements the scheme suggested by Tim; > * GtkWindows needs to be destroyed manually, it's the responsibility is the > application developers to do so > * GObjects which are referenced by a widget are ref_sinked when parsing is > finished, that reference is removed when the toplevel window is destroyed Whats the toplevel window ? (i.e. who knows that the object that referenced this GObject is part of a widget tree ?). What happens for GObjects that are parented by other GObjects ? Does the child GObject get unreffed when its parent's toplevel is destroyed ? what if the parenting object was removed from the hierarchy before the toplevel was destroyed ? > * Floating toplevel objects are ref_sinked as soon as they are constructed, not > sure this is correct, but it makes non-GtkWindow toplevels work Does this also involve any destroying/unreffing of the constructed object hierarchy when the builder is finalized ?
can I propose to sort out some terminology here ? toplevel = GtkWindow, toplevel in the widget hierarchy root = root element in the xml hierarchy
Created attachment 90752 [details] [review] patch v2: use root instead of toplevel (In reply to comment #11) > can I propose to sort out some terminology here ? Sure, new patch attached.
(In reply to comment #10) > (In reply to comment #5) > > Created an attachment (id=90727) [edit] > > patch > > > > This patch implements the scheme suggested by Tim; > > * GtkWindows needs to be destroyed manually, it's the responsibility is the > > application developers to do so > > * GObjects which are referenced by a widget are ref_sinked when parsing is > > finished, that reference is removed when the toplevel window is destroyed > > Whats the toplevel window ? (i.e. who knows that the object that referenced > this GObject is part of a widget tree ?). Sorry for the confusing terminology, I meant a GtkWindow object constructed by a builder. > What happens for GObjects that are parented by other GObjects ? What do you mean by "parented"? > Does the child GObject get unreffed when its parent's toplevel is > destroyed ? what if the parenting object was removed from the hierarchy > before the toplevel was destroyed ? Yes. If you remove something from the hierarchy you're responsible for it. > > * Floating toplevel objects are ref_sinked as soon as they are constructed, not > > sure this is correct, but it makes non-GtkWindow toplevels work > > Does this also involve any destroying/unreffing of the constructed > object hierarchy when the builder is finalized ? No, the application developer is responsible for destroying windows.
[...] > > > * GObjects which are referenced by a widget are ref_sinked when parsing is > > > finished, that reference is removed when the toplevel window is destroyed > > > > Whats the toplevel window ? (i.e. who knows that the object that referenced > > this GObject is part of a widget tree ?). > > Sorry for the confusing terminology, I meant a GtkWindow object constructed by > a builder. So the question still stands - just because a GObject was build and refferenced by another GObject doesnt mean that that other GObject was part of a hierarchy that has a GtkWindow at the toplevel. > > What happens for GObjects that are parented by other GObjects ? > > What do you mean by "parented"? > I mean: <object class="MyFooObject"> <child> <object class="MyBarChild"/> </child> </object> i.e. parenting is not an anomaly of GtkContainer, any object can parent any other object and its wrong for the builder to make assumptions along these lines. > > Does the child GObject get unreffed when its parent's toplevel is > > destroyed ? what if the parenting object was removed from the hierarchy > > before the toplevel was destroyed ? > > Yes. > If you remove something from the hierarchy you're responsible for it. [...] > > Does this also involve any destroying/unreffing of the constructed > > object hierarchy when the builder is finalized ? > > No, the application developer is responsible for destroying windows. Windows and any other objects that are toplevel in the ui file correct ? I know I dont want the builder to go randomly destroying my toplevel GObjects just because they are not owned/parented by other objects...
(In reply to comment #0) > The reference counting in GtkBuilder is not ideal the way it's currently > implemented. Especially GObjects needs to be unreferenced if nobody is claiming > their ownership. Ok I've been looking at this backwards for a time, this bug points to a lack of convenience somewhere, somebody thinks they shouldnt be responsable for explicitly destroying objects that were implicitly built by the builder; that makes sence to a certain extent. I think that first its important to avoid making assumptions about the hierarchy from the builder standpoint; an object that parents another object owns that object, an object that references another object does not own it - the builder has no idea what kind of relationship stands between those objects. So here's a proposal - add some kind of attribute to the "object" tag that tells the builder to "own" that object. If you mark your toplevel window to be "owned" by the builder then it will be destroyed when the builder is destroyed (as a result of a simple g_object_unref()). If you mark your GObject to be "owned" by the builder then it will be unreffed when the builder is destroyed (this might mean the object's destruction, unless that object is parented by another object in the hierarchy), this would count for the GtkWidget flavour of GObject and work exactly the same way. For all objects marked as "owned": All GInitiallyUnowned objects would be ref_sink'ed by the builder at construction time and unreffed at builder destruction time, all GObjects that are not GInitiallyUnowned would simply be unreffed at destruction time All objects that are not explicitly marked as "owned" in the glade file would be delivered to the user, along with full ownership of that object.
(In reply to comment #15) [...] > All objects that are not explicitly marked as "owned" in the glade file > would be delivered to the user, along with full ownership of that object. Ok mistype here, I shouldn't say "along with full ownership of that object", instead I should say it would be delivered to the user in its expected state with no strings attached (as if it were delivered directly from the gtk+ api).
<object class="MyFooObject"> <child> <object class="MyBarChild"/> </child> </object> Tristan, we all knwo that you really really want GContainer, but as things stand right now, there is no concept of containment for non-widget objects. There are just references, and of course the life-cycles of objects may be tied together in specific contexts.
(In reply to comment #17) [...] > Tristan, > > we all knwo that you really really want GContainer, but as things stand > right now, there is no concept of containment for non-widget objects. > There are just references, and of course the life-cycles of objects may > be tied together in specific contexts. > libglade allowed any object to build its own children, gtkbuilder allows it except it makes life hard by forcing you to write weird GMarkup parsing stuff, there only happen to be no widgets in gtk+ itself that do that. Yes, I've been using GObject delagate for +2 years now in conjunction with libglade and will most probably continue to do so. I am not demanding that gtk+ recognize GContainer atm, I am only demanding that a proper framework exist that allows me to do what ever I want with my objects without making unfounded assumptions about when my objects should be finalized. Please reconsider.
> This patch implements the scheme suggested by Tim; > * GtkWindows needs to be destroyed manually, it's the responsibility is the > application developers to do so sounds fine. > * GObjects which are referenced by a widget are ref_sinked when parsing is > finished, that reference is removed when the toplevel window is destroyed does this have the side-effect of keeping those objects alive until the widget goes away ? e.g if I construct object X and assign it to the property P of widget W, the way I read the code, X will stay alive until W goes away, even if the application later sets another value for P on W. That might have somewhat surprising consequences. Why is this necessary ? The fact that O is the value of property P on W should already keep it alive as long as necessary and ensure that it goes away together with W, no ? > * Floating toplevel objects are ref_sinked as soon ss they are constructed, not > sure this is correct, but it makes non-GtkWindow toplevels work I think if you want to stay as close to g_object_new as possible, they should remain floating ? or does the builder take over ownership of them and makes them go away if the builder does ?
Tristan: At this time, the focus of the builder is to construct graphical user interfaces. It is currently not intended to be a general purpose object factory, instead it is aimed at developers who wants to write graphical applications. GtkBuilder might be extended in the future to be generic to handle any kind of GObjects, but it is currently not. As Tim pointed out, the only object which is interesting to the user is a GtkWindow and it is therefor special cased in the Builder itself, all the operations on a builder are going to be focus on getting a user interface constructed and eventually destroyed. There for it makes sense to tie the creating of objects outside of a window/widget hierarchy to the toplevel window which they actually belongs to. I agree that GContainer might be a good idea, but is not the focus on this bug, do not hijack it, feel free to open up another bug, against builder, gtk or even gobject where we can discuss this functionality.
(In reply to comment #20) > Tristan: > > At this time, the focus of the builder is to construct graphical user > interfaces. > It is currently not intended to be a general purpose object factory, instead it > is aimed at developers who wants to write graphical applications. > GtkBuilder might be extended in the future to be generic to handle any kind of > GObjects, but it is currently not. Exactly, and at this time its important to have a flexible framework that is prepared for all future possibilities. > As Tim pointed out, the only object which is interesting to the user is a > GtkWindow and it is therefor special cased in the Builder itself, all the > operations on a builder are going to be focus on getting a user interface > constructed and eventually destroyed. > There for it makes sense to tie the creating of objects outside of a > window/widget hierarchy to the toplevel window which they actually belongs to. > > I agree that GContainer might be a good idea, but is not the focus on this bug, > do not hijack it, feel free to open up another bug, against builder, gtk or > even gobject where we can discuss this functionality. > I am not hijacking the bug, I am seriously objecting to the builder being meddlesome about how the application authors objects are being refcounted/bookkept. I am bringing up the possibility that, regardless of GContainer being available as an interface in GTK+ proper, people will use the builder in whatever ways that they will - with there own custom GObjects that they want bookkept/refcounted in their own ways - the builder is IMO severely overstepping its boundries by assuming that one object should be destroyed when the builder is destroyed and another should be left alive because one has a property reference to it, or based on the arbitrary fact that its type might be derived from GtkWindow. I even proposed an alternative that will allow for the convenient "passing of ownership" to the builder, which I believe satisfies the usecase for this feature (or bug is what you're calling it), and on top of it this alternative leaves the user with the possibility of bookkeeping their object references in their own way. Am I really the only one that sees this as a dangerous step ? refcounting in gtk+ is already tough enough as it is with GInitiallyUnowned in common practice; we dont need to cloud it up with special case code that mangles the refcounts even further.
(In reply to comment #19) > > * GObjects which are referenced by a widget are ref_sinked when parsing is > > finished, that reference is removed when the toplevel window is destroyed > > does this have the side-effect of keeping those objects alive until the > widget goes away ? e.g if I construct object X and assign it to the property > P of widget W, the way I read the code, X will stay alive until W goes away, > even if the application later sets another value for P on W. That might have > somewhat surprising consequences. Why is this necessary ? The fact that > O is the value of property P on W should already keep it alive as long as > necessary and ensure that it goes away together with W, no ? Unfortunate, I'll attach an updated and simplified patch which avoids this. However, We need to decide the following though. How long should root GObjects created by the builder stay around? * Until the parsing is finished, unclaimed/unreferences objects will go away immediately * Until the GtkBuilder is finalized. * Until the application developer calls g_object_unref() on the object.
Created attachment 90785 [details] [review] patch v3: do not tie constructed objects to their toplevels explicitly
> We need to decide the following though. How long should root GObjects created by the builder stay around? For non-widget objects, the builder needs to keep a reference, so that a second call to gtk_builder_get_object() can return the same instance. And the application needs to keep a reference so that the instance will continue to exist when the builder is destroyed. That seems perfectly logical and easy and consistent with other parts of GTK+. For non-GtkObject objects there's really no alternative. GtkWindows and GtkWidgets act differently, as they do in the rest of GTK+. Anyway, if you are not going to support non-GtkWidget objects in GTK+, I suggest you don't claim to in the API or its documentation. You can simplify this for now by changing GObject* gtk_builder_get_object(name) to GtkWidget* gtk_builder_get_widget(name). You could add gtk_builder_get_object() in a future version, deprecating gtk_builder_get_widget() if you like. (gtkmm will have both get_widget() and get_object() anyway, because non-widget objects are used via a automatically-refcounting RefPtr<>, while widgets cannot be managed via simple reference-counting. The distinction make it less necessary to understand the API documentation about memory management.)
> [...] or based on the arbitrary fact that its type might be derived from > GtkWindow. Tristan, GtkWindow inheritance is far from arbitrary in GTK+, and has very concrete consequences for the users of the objects. And we are talking about adding something to GTK+ here, not some abstract object system. > However, We need to decide the following though. How long should root GObjects > created by the builder stay around? > * Until the parsing is finished, unclaimed/unreferences objects will go away > immediately > * Until the GtkBuilder is finalized. > * Until the application developer calls g_object_unref() on the object. The first is not an option, imo. How can one even claim these objects before the parse is done ? The other options sound both ok to me. The second one is perhaps more convenient. The third one is a bit closer to the idea of a builder that can be thrown away after it has done its job, leaving the built stuff behind. > Anyway, if you are not going to support non-GtkWidget objects in GTK+ I didn't see anybody propose to drop non-widget support from the builder ?
> > Anyway, if you are not going to support non-GtkWidget objects in GTK+ > > I didn't see anybody propose to drop non-widget support from the builder ? Sorry, I guess I misinterpreted "the focus of the builder is to construct graphical user interfaces". I stand corrected.
(In reply to comment #25) > > However, We need to decide the following though. How long should root GObjects > > created by the builder stay around? > > > * Until the parsing is finished, unclaimed/unreferences objects will go away > > immediately > > * Until the GtkBuilder is finalized. > > * Until the application developer calls g_object_unref() on the object. > > The first is not an option, imo. > How can one even claim these objects before the parse is done ? > > The other options sound both ok to me. > The second one is perhaps more convenient. > The third one is a bit closer to the idea of a builder that can be thrown > away after it has done its job, leaving the built stuff behind. Okay, I'm pretty sure the second option is the way to go, it makes most sense to me. It'll work quite nicely with the API suggested in bug 447969. It's the way attachment 90785 [details] [review] is implemented. Floating objects should probably not be freed nor unreffed in GtkBuilders finalize. Apart from that I think only documentation is missing.
I'm not sure I understand the desire to treat floating objects differently. The way I see it, the most straightforward and understandable implementation is for the builder to take a reference (and thereby sink) on any object it creates, and drop all those references when it is finalized. Does this have any obvious problems that I am overlooking ?
(In reply to comment #28) > I'm not sure I understand the desire to treat floating objects differently. > > The way I see it, the most straightforward and understandable implementation is > for the builder to take a reference (and thereby sink) on any object it > creates, > and drop all those references when it is finalized. Does this have any obvious > problems that I am overlooking ? > The main reason is that I can't just do unref(floating_object), so to be able to release the reference in GtkBuilder->finalized I need to do a ref_sink(floating_object) after constructing it, just that.
Committed patch 3 as revision 18292.
What does all this mean for language bindings which do automatic memory management? What do I do with the GObject returned by gtk_builder_get_object? Is it mine already and do I need to unref if when not needed anymore? Or do I have to ref it? If it's a GInitiallyUnowned descendant, do I have to sink it? Does comment 5 still hold, ie. do GtkWindows need to treated specially? I think all this needs to be explained in the docs.
I don't think this bug should be closed until this is documented.
(In reply to comment #32) > I don't think this bug should be closed until this is documented. > Matthias did add documentation yesterday, have you checked it?
Does this not satisfy you ? <para> A GtkBuilder holds a reference to all objects that it has constructed and drops these references when it is finalized. To keep objects beyond the lifespan of the builder, they must be fetched with gtk_builder_get_object() and reffed with g_object_ref(). It is the responsibility of the user to destroy all toplevel windows that have been constructed by a builder (these are not automatically cleaned up when the builder is finalized, since GTK+ itself holds a reference to each toplevel window). </para>
Ah, thanks, I didn't know that some had been added yesterday. No, I don't think this is clear enough. There are three types of memory management in GTK+ applications in the default cases that I know of - ref-counted objects, child widgets that are destroyed by their parents, and top-level windows that the application must destroy itself. I think that the documentation should make clear what should be done for each of these types of object, ideally with examples. I made a suggestion in http://bugzilla.gnome.org/show_bug.cgi?id=447967#c1 but I don't know if that's accurate for the current implementation. If this isn't made very clear then we'll just have long confusing threads about it when people ask on the mailing lists.
> No, I don't think this is clear enough. Hmm, I'm scratching my head a bit here trying to make it more clear. The first sentence gives the general rule that holds for any kind of object: A GtkBuilder holds a reference to all objects that it has constructed and drops these references when it is finalized. The next sentence explains the consequence of this: To keep objects beyond the lifespan of the builder, they must be fetched with gtk_builder_get_object() and reffed with g_object_ref(). The last sentence points out the special case: It is the responsibility of the user to destroy all toplevel windows that have been constructed by a builder (these are not automatically cleaned up when the builder is finalized, since GTK+ itself holds a reference to each toplevel window). Please help me to improve this. What exactly is missing ?
(In reply to comment #36) > > No, I don't think this is clear enough. > To keep objects beyond the lifespan of the builder, they must be fetched with > gtk_builder_get_object() and reffed with g_object_ref(). While true, it should be pointed out that this in practice mostly applies to convenience functions such as the one in bug 447969 (gedit uses one too) where the builder is finalized _before_ the widgets are referenced. The application developers would not need to worry in the case where the builder is finalized _after_ the objects are referenced, either by a property or by adding them to a container. It would probably be good to have examples for this two common use cases, including the need to destroy the windows.
> The first sentence gives the general rule that holds for any kind of object: That's "To keep objects beyond the lifespan of the builder, they must be fetched with gtk_builder_get_object() and reffed with g_object_ref() " That's fine for objects such as GtkTreeModel. It's not helpful for (non top-level) child widgets such as GtkButton. You can ref them all you like but they will still be destroyed by their parent container. Therefore, there isn't much point in reffing them. The sentence about the top-level windows is fine, though I'd like it to explicitly mention gtk_widget_destroy(), because people (mis)interpret "destroy" in all kinds of ways. You might consider that all this should be obvious to the average GTK+ C coder, though I don't think it is. If you don't want to make it more obvious then nevermind. We'll just make it clearer in the gtkmm documentation.
Ok, thats a valid point. How about this ? <para> A GtkBuilder holds a reference to all objects that it has constructed and drops these references when it is finalized. This will get rid of all non-widget objects and all widgets which are not contained in a toplevel window. For toplevel windows constructed by a builder, it is the responsibility of the user to call gtk_widget_destroy() to get rid of them and all the widgets they contain. </para> <para> The functions gtk_builder_get_object() and gtk_builder_get_objects() can be used to access the widgets in the interface by the names assigned to them inside the UI description. Toplevel windows returned by these functions will stay around until the user explicitly destroys them with gtk_widget_destroy(). Other widgets will either be part of a larger hierarchy constructed by the builder (in which case you should not have to worry about their lifecycle), or without a parent, in which case they have to be added to some container to make use of them. Non-widget objects need to be reffed with g_object_ref() to keep them beyond the lifespan of the builder. </para>
Yeah, that's better. Thanks. Because I'm fussy, I would change: "This will get rid of all non-widget objects and all widgets which are not contained in a toplevel window." to "This finalization can cause the destruction of non-widget objects or widgets which are not contained in a toplevel window."
Ok, committed with that change. 2007-07-03 Matthias Clasen <mclasen@redhat.com> * gtk/tmpl/gtkbuilder.sgml: Some wordsmithing on the memory management explanation, asked for by Murray Cumming.