Memory management practices

[Austin Clements]

On Wed, Sep 7, 2011 at 4:36 PM, Ben Gamari <bgamari.foss at gmail.com> wrote:
> On Mon, 29 Aug 2011 16:30:57 -0400, Ben Gamari <bgamari.foss at 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.)