On Wed, Sep 7, 2011 at 4:36 PM, Ben Gamari <bgamari.foss@gmail.com> wrote: > On Mon, 29 Aug 2011 16:30:57 -0400, Ben Gamari <bgamari.foss@gmail.com> wrote: >> [SNIP] >> >> In general, it seems to me that memory management in notmuch bindings is >> a little bit harder than it needs to me due to the decision not to >> talloc_ref parent objects when a new child object is created. This means >> that a bindings author needs to recreate the ownership tree in their >> binding, a task which is fairly easily done (except in the case of >> Haskell due to the weak GC finalization guarantees) but seems quite >> unnecessary. Is there a reason this decision was made? Would a patch be >> accepted adding talloc_ref'ing parents in those functions creating >> children and talloc_frees in *_destroys? >> > Any opinions concerning whether this is an acceptable idea? I wouldn't > mind putting together a patch-set, but I'd rather not waste my time if > the set would ultimately be rejected due to some technical objection I > have yet to think of. > > Cheers, I've been meaning to look in to this in depth. (I still haven't, but wanted to give you some reply.) In general (though perhaps not always?), libnotmuch uses talloc() to allocate children objects, which already implicitly creates a talloc reference from the parent object to the child object. You've certainly thought about this harder than I have, but it seems like the bindings should simply create an additional talloc reference and unlink that reference in the GC finalizer, so that the library-created references would maintain the integrity of the data structures, while the binding-created references would maintain their extent. Hence, I don't see why simultaneous GC would cause problems with talloc, or why the bindings would have to recreate the reference tree. I'm a bit confused by the reference tree you drew. The references in the underlying libnotmuch objects are the other way around. notmuch_query_t holds a talloc reference to every notmuch_messages_t it produces, not the other way around. (Though, in reality, these objects are completely independent of each other. This reference exists purely as a convenience for C programmers to make it easy to clean up all notmuch_messages_t objects when you destroy the notmuch_query_t. This is probably a poor interface; it may be better to take an explicit talloc context, which could be the query object, or could be something else. In fact, I would expect this to cause memory *leaks* in bindings if it were not handled carefully, rather than premature GC.)