Date: Sun, 11 Sep 2011 22:18:18 -0400
From: Austin Clements <amdragon@MIT.EDU>
To: Ben Gamari <bgamari.foss@gmail.com>
Subject: Re: Memory management practices
Message-ID: <20110912021818.GD23603@mit.edu>
References: <20110829183010.GA2605@24f89f8c-e6a1-4e75-85ee-bb8a3743bb9f>
	<87liucyn7i.fsf@gmail.com> <87aaag3xaf.fsf@gmail.com>
	<CAH-f9WsfHUm_D-+wB89Lt9Wt=hjwDyywvJTK-0NwmHRg0TUsxQ@mail.gmail.com>
	<CAH-f9WveBfvmv2jOF+C81OeeQJt06g6U0q3J_idHrs60DLw7+g@mail.gmail.com>
	<87zkiff8in.fsf@SSpaeth.de> <20110908151557.GM5688@mit.edu>
	<8762l22hgk.fsf@SSpaeth.de> <20110909175328.GV5688@mit.edu>
	<87d3f64ups.fsf@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87d3f64ups.fsf@gmail.com>
User-Agent: Mutt/1.5.20 (2009-06-14)
Cc: Bertram Felgenhauer <bertram.felgenhauer@googlemail.com>,
	Bart Massey <bart@cs.pdx.edu>, notmuch <notmuch@notmuchmail.org>
Precedence: list

Quoth Ben Gamari on Sep 11 at  5:47 pm:
> Sorry I've been so quiet on this recently. I've been a little under the
> weather.

No worries.

> On Fri, 9 Sep 2011 13:53:28 -0400, Austin Clements <amdragon@MIT.EDU> wrote:
> > Hence my suggestion that, rather than trying to emulate C-style memory
> > management in bindings, bindings should create an additional talloc
> > reference to the underlying objects and rather than calling
> > notmuch_*_destroy during finalization, they should simply unlink this
> > additional reference.
> 
> Currently talloc's reference counting interface is hidden behind
> _destroy. While this might be a fairly intrusive change, perhaps notmuch
> wants to juse expose a pair of reference counting *_ref/unref functions
> instead of the *_destroy. Most users would simply need to change
> existing *_destroy()s to _unref()s. Furthermore, this would allow
> bindings authors to easily ensure non-broken GC behavior.

I think the _destroy functions are silly.  They all just call
talloc_free and, indeed, it would arguably be incorrect for them to do
anything more (any additional cleanup should be in a talloc
destructor).  talloc is never explicitly mentioned in lib/notmuch.h
(intentionally, I would assume) but talloc-style notions of
"ownership" pervade the library documentation.  IMO, the library
should just admit to using talloc, rather than try to wrap all of the
not-insubstantial talloc functionality a caller may need.

In the language of talloc, it's very natural to express the needs of
bindings in terms talloc_reference and talloc_unlink.  The bindings
could maintain a per-Database context and track their own ownership by
adding a talloc reference from this context to each object returned
from the bindings; the finalizer would simply unlink the finalized
object from this context.  Bindings could also use a global context
(though that would obviously be awkward in Haskell without biting the
unsafePerformIO bullet).  Alternatively, bindings could use the NULL
context, which has the advantage of not actually tracking ownership in
talloc, but the disadvantage of making it harder to track down bugs
(since any code can reference or unlink from NULL).