From: Carl Worth <cworth@cworth.org>
To: Ben Gamari <bgamari.foss@gmail.com>, notmuch <notmuch@notmuchmail.org>
Subject: Re: Rethinking *_destroy()
In-Reply-To: <87aaa0l1tc.fsf@gmail.com>
References: <87aaa0l1tc.fsf@gmail.com>
User-Agent: Notmuch/0.8 (http://notmuchmail.org) Emacs/23.3.1
	(i486-pc-linux-gnu)
Date: Mon, 19 Sep 2011 15:27:08 -0700
Message-ID: <87obygxj5v.fsf@yoom.home.cworth.org>
MIME-Version: 1.0
Content-Type: multipart/signed; boundary="=-=-=";
	micalg=pgp-sha1; protocol="application/pgp-signature"
Cc: Bertram Felgenhauer <bertram.felgenhauer@googlemail.com>,
	notmuch <notmuch@notmuchmail.org>, Austin Clements <amdragon@MIT.EDU>,
	Bart Massey <bart@cs.pdx.edu>
Precedence: list

--=-=-=
Content-Transfer-Encoding: quoted-printable

On Mon, 19 Sep 2011 16:22:39 -0400, Ben Gamari <bgamari.foss@gmail.com> wro=
te:
> As many might have noticed, there was recently a bit of a discussion on
> this list concerning the state of memory management in libnotmuch,
> especially regarding some classes of garbage collectors.
>=20
> To summarize (someone correct me if I get something wrong),

Thanks for the summary, Ben. As many might have noticed, I'm somewhat
behind on my reading of the notmuch mailing list right now. So I
appreciate you bringing this issue to my attention.

> Overall, this means that languages with cyclical garbage collectors
> (Python, Haskell, and I'm sure others) can not bind libnotmuch
> correctly.

That certainly sound unappealing.

> I can see at least two ways of doing this,
>=20
>   1) Acknowledging that we use talloc and allowing users to use
>      talloc_ref and talloc_unlink directly

I like this option, myself. I think talloc has been a wonderful boon to
my programming. So I don't have a problem with the notmuch API
documentation committing the implementation to using talloc. I'm also
quite glad to let the notmuch documentation advertise talloc to any
readers.

>   2) Wrapping talloc by adding a *_ref() and *_unref() to each object

That looks like a lot of extra API, but with no substantial benefit. (We
would get the freedom to switch to some other implementation of talloc,
but, I don't think we need that.)

> Additionally, we need to decide to what extent we want to break the
> libnotmuch API. While strictly speaking we could keep *_destroy() around
> without breaking existing code, this will mean we will have two ways of
> freeing an object. Perhaps a scheduled deprecation in a release or two
> is in order here?

Actually, I would prefer to leave *_destroy around, (as long as it's
doing nothing other than tall_free (or talloc_unlink? [*]) which does
appear to be the case for all existing functions).

To me, it doesn't really look like two ways of freeing an object.
Anyone who calls notmuch_query_create, (for example), should call
notmuch_query_destroy and those two calls pair nicely.

Then, *some* callers will want to take advantage of talloc. Those users
can add calls to talloc_reference and talloc_unlink (which pair nicely
themselves).

What I don't want is the unnatural pairing of something like
"notmuch_query_create" with "talloc_unlink". That just seems uselessly
harder to learn and remember.

> Any and all feedback is desired.

Thanks for pointing out the issue. Let me know if I've missed
anything. And, happy binding!

=2DCarl

[*] These destroy() functions are all currently calling talloc_free()
which won't work if the user has any remaining parents still present due
to calls to talloc_reference(). Perhaps we can just document that all
talloc_reference()/talloc_unlink() pairs must be complete before the
caller calls _destroy()?

=2D-=20
carl.d.worth@intel.com

--=-=-=
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)

iEYEARECAAYFAk53wbwACgkQ6JDdNq8qSWj44QCfaFAeMLgdU31BYNOirx6dMyB6
f10An1S7lBIKNteWoY4W94NZjb4LTBxp
=1uVb
-----END PGP SIGNATURE-----
--=-=-=--