Re: [PATCH 3/7] go: Allow notmuch objects to be garbage collected
authorAustin Clements <amdragon@MIT.EDU>
Sat, 4 Aug 2012 19:28:40 +0000 (15:28 +2000)
committerW. Trevor King <wking@tremily.us>
Fri, 7 Nov 2014 17:48:50 +0000 (09:48 -0800)
3a/8136c38d95bebad5bfa515e78c58541954b289 [new file with mode: 0644]

diff --git a/3a/8136c38d95bebad5bfa515e78c58541954b289 b/3a/8136c38d95bebad5bfa515e78c58541954b289
new file mode 100644 (file)
index 0000000..39599f7
--- /dev/null
@@ -0,0 +1,547 @@
+Return-Path: <amdragon@mit.edu>\r
+X-Original-To: notmuch@notmuchmail.org\r
+Delivered-To: notmuch@notmuchmail.org\r
+Received: from localhost (localhost [127.0.0.1])\r
+       by olra.theworths.org (Postfix) with ESMTP id CC114431FB6\r
+       for <notmuch@notmuchmail.org>; Sat,  4 Aug 2012 12:28:52 -0700 (PDT)\r
+X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
+X-Spam-Flag: NO\r
+X-Spam-Score: -0.7\r
+X-Spam-Level: \r
+X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
+       tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
+Received: from olra.theworths.org ([127.0.0.1])\r
+       by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
+       with ESMTP id Go2nI8fI-34y for <notmuch@notmuchmail.org>;\r
+       Sat,  4 Aug 2012 12:28:50 -0700 (PDT)\r
+Received: from dmz-mailsec-scanner-4.mit.edu (DMZ-MAILSEC-SCANNER-4.MIT.EDU\r
+       [18.9.25.15])\r
+       by olra.theworths.org (Postfix) with ESMTP id 44632431FAF\r
+       for <notmuch@notmuchmail.org>; Sat,  4 Aug 2012 12:28:50 -0700 (PDT)\r
+X-AuditID: 1209190f-b7f306d0000008b4-f3-501d77f0b043\r
+Received: from mailhub-auth-1.mit.edu ( [18.9.21.35])\r
+       by dmz-mailsec-scanner-4.mit.edu (Symantec Messaging Gateway) with SMTP\r
+       id 71.B3.02228.0F77D105; Sat,  4 Aug 2012 15:28:48 -0400 (EDT)\r
+Received: from outgoing.mit.edu (OUTGOING-AUTH.MIT.EDU [18.7.22.103])\r
+       by mailhub-auth-1.mit.edu (8.13.8/8.9.2) with ESMTP id q74JSkYa009393; \r
+       Sat, 4 Aug 2012 15:28:47 -0400\r
+Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91])\r
+       (authenticated bits=0)\r
+       (User authenticated as amdragon@ATHENA.MIT.EDU)\r
+       by outgoing.mit.edu (8.13.6/8.12.4) with ESMTP id q74JSe3B009978\r
+       (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NOT);\r
+       Sat, 4 Aug 2012 15:28:45 -0400 (EDT)\r
+Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.77)\r
+       (envelope-from <amdragon@mit.edu>)\r
+       id 1Sxk1g-0004Xa-Iu; Sat, 04 Aug 2012 15:28:40 -0400\r
+Date: Sat, 4 Aug 2012 15:28:40 -0400\r
+From: Austin Clements <amdragon@MIT.EDU>\r
+To: Adrien Bustany <adrien@bustany.org>\r
+Subject: Re: [PATCH 3/7] go: Allow notmuch objects to be garbage collected\r
+Message-ID: <20120804192840.GI22601@mit.edu>\r
+References: <1342636475-16057-1-git-send-email-adrien@bustany.org>\r
+       <1342636475-16057-4-git-send-email-adrien@bustany.org>\r
+       <20120718204001.GT31670@mit.edu> <50085106.8040804@bustany.org>\r
+       <20120720032352.GX31670@mit.edu> <500DCA3A.8060407@bustany.org>\r
+MIME-Version: 1.0\r
+Content-Type: text/plain; charset=iso-8859-1\r
+Content-Disposition: inline\r
+Content-Transfer-Encoding: 8bit\r
+In-Reply-To: <500DCA3A.8060407@bustany.org>\r
+User-Agent: Mutt/1.5.21 (2010-09-15)\r
+X-Brightmail-Tracker:\r
+ H4sIAAAAAAAAA+NgFprNKsWRmVeSWpSXmKPExsUixCmqrPuhXDbAoHMOi8X6O2vZLK7fnMns\r
+       wOTx8cA9Jo9nq24xBzBFcdmkpOZklqUW6dslcGWcevOSqeBYG2PFu7WdTA2Mr7O6GDk5JARM\r
+       JJrmHGSBsMUkLtxbzwZiCwnsY5Q4PicBwl7PKNG/QqaLkQvIPsEkcfXXGnaIxBJGiS/NoiA2\r
+       i4CKxJc355lAbDYBDYlt+5czgtgiAuoSOzrbwWxmAWmJb7+bwWqEBbwkbm/uYQaxeQV0JL6c\r
+       6meHWNDMJHHy2R0miISgxMmZT1ggmnUkdm69A3QdB9ig5f84IMLyEs1bZ4PN4RTQlpi04g7Y\r
+       LlGge6ac3MY2gVF4FpJJs5BMmoUwaRaSSQsYWVYxyqbkVunmJmbmFKcm6xYnJ+blpRbpmujl\r
+       ZpbopaaUbmIEx4Ek/w7GbweVDjEKcDAq8fAmq8gECLEmlhVX5h5ilORgUhLl/VAmGyDEl5Sf\r
+       UpmRWJwRX1Sak1p8iFGCg1lJhPenPFCONyWxsiq1KB8mJc3BoiTOezXlpr+QQHpiSWp2ampB\r
+       ahFMVoaDQ0mCVxsY70KCRanpqRVpmTklCGkmDk6Q4TxAw1VAaniLCxJzizPTIfKnGBWlxHkV\r
+       QRICIImM0jy4XliaesUoDvSKMK8bSBUPMMXBdb8CGswENNjOTApkcEkiQkqqgTE//HzUYu7c\r
+       2xN+fvE8y/Tmj9HvzhNLuR8sXOaZ/nLT68t/HAQe892/82LTMVWbf4H+l1ctTflyZfuuitUK\r
+       6dWX/i9+8uZg8Pxy89n/ltlf2HVjzUMzpn+Sua/CAkwOGuzsfey2q/T98kmH91Yzvt3ovf3K\r
+       9xNz5kvU7v3///wep/2ujgI3VlY3sSuxFGckGmoxFxUnAgBGM9cSLgMAAA==\r
+Cc: notmuch@notmuchmail.org\r
+X-BeenThere: notmuch@notmuchmail.org\r
+X-Mailman-Version: 2.1.13\r
+Precedence: list\r
+List-Id: "Use and development of the notmuch mail system."\r
+       <notmuch.notmuchmail.org>\r
+List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
+       <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
+List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
+List-Post: <mailto:notmuch@notmuchmail.org>\r
+List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
+List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
+       <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
+X-List-Received-Date: Sat, 04 Aug 2012 19:28:52 -0000\r
+\r
+Quoth Adrien Bustany on Jul 24 at  1:03 am:\r
+> Le 20/07/2012 06:23, Austin Clements a écrit :\r
+> >Quoth Adrien Bustany on Jul 19 at  9:25 pm:\r
+> >>Le 18/07/2012 23:40, Austin Clements a écrit :\r
+> >>>This is subtle enough that I think it deserves a comment in the source\r
+> >>>code explaining that tracking the talloc owner reference, combined\r
+> >>>with the fact that Go finalizers are run in dependency order, ensures\r
+> >>>that the C objects will always be destroyed from the talloc leaves up.\r
+> >>\r
+> >>Definitely, I tend to comment in the commit message and forget about\r
+> >>the code...\r
+> >>\r
+> >>>\r
+> >>>Just one inline comment below.  Otherwise, I think this is all\r
+> >>>correct.\r
+> >>\r
+> >>Agree with the comment, the Database should be the parent. I guess I\r
+> >>wasn't sure of the talloc parenting.\r
+> >>\r
+> >>>\r
+> >>>Is reproducing the talloc hierarchy in all of the bindings really the\r
+> >>>right approach?  I feel like there has to be a better way (or that the\r
+> >>>way we use talloc in the library is slightly broken).  What if the\r
+> >>>bindings created an additional talloc reference to each managed\r
+> >>>object, just to keep the object alive, and used talloc_unlink instead\r
+> >>>of the destroy functions?\r
+> >>\r
+> >>Reproducing the hierarchy is probably error prone, and not that\r
+> >>simple indeed :/\r
+> >>I haven't checked at all the way you suggest, but if we use\r
+> >>talloc_reference/unlink, we get the same issue no?\r
+> >>- If we do for each new wrapped object talloc_reference(NULL,\r
+> >>wrapped_object), the the object will be kept alive until we\r
+> >>talloc_unlink(NULL, wrapped_object), but what about its parents? For\r
+> >>example will doing that on a notmuch_message_t keep the\r
+> >>notmuch_messages_t alive?\r
+> >\r
+> >Hmm.  This is what I was thinking.  You have an interesting point; I\r
+> >think it's slightly wrong, but it exposes something deeper.  I believe\r
+> >there are two different things going on here: some of the talloc\r
+> >relationships are for convenience, while some are structural.  In the\r
+> >former case, I'm pretty sure my suggestion will work, but in the\r
+> >latter case the objects should *never* be freed by the finalizer!\r
+> >\r
+> >For example, notmuch_query_search_messages returns a new\r
+> >notmuch_messages_t with the query as the talloc parent, but that\r
+> >notmuch_messages_t doesn't depend on the query object; this is just so\r
+> >you can conveniently delete everything retrieved from the query by\r
+> >deleting the query.  In this case, you can either use parent\r
+> >references like you did---which will prevent a double-free by forcing\r
+> >destruction to happen from the leaves up but at the cost of having to\r
+> >encode these relationships and of extending the parent object\r
+> >lifetimes beyond what's strictly necessary---or you can use my\r
+> >suggestion of creating an additional talloc reference.\r
+> \r
+> Actually, checking the code of notmuch_query_search_messages, it\r
+> seems that the notmuch_messages_t (and the notmuch_message_t as\r
+> well) object *does* depend on the database and the query... So in\r
+> that case I think we need the "owner" Object reference as I\r
+> currently have (we want the Messages to keep the Query alive, and\r
+> the Query keeps the Database alive).\r
+\r
+It does depends on the database (I think just about everything depends\r
+on the database, directly or indirectly, so I suppose everything will\r
+need some parent pointer), but could you explain how it depends on the\r
+query?  It uses the MSet derived from the query, but Xapian internally\r
+handles the sharing and referencing counting of all of its objects.\r
+\r
+> That said, you example below looks valid, and it seems I'll need to\r
+> add a flag to createMessage() (and some others) to disable the\r
+> SetFinalizer call for certain instances (we probably want to keep it\r
+> for eg. SearchMessageByFilename).\r
+> \r
+> - The candidates I found for adding a tmalloc reference and not a\r
+> "full" Go reference (therefore preventing to keep the parent alive\r
+> too long needlessly) are GetAllTags, Thread.GetTags,\r
+> Messages.CollectTags, and Message.GetTags (those are basically\r
+> string lists)\r
+\r
+Sounds reasonable (but I haven't gone through carefully).\r
+\r
+> - The methods for which I should remove the SetFinalizer on the\r
+> wrapper (as you showed in the example below) while keeping the Go\r
+> reference are Threads.Get and Messages.Get\r
+\r
+Sounds right.  I think those are the only cases where the object is\r
+still owned by a container, other than strings (which Go has to copy\r
+anyway).\r
+\r
+> I would also maybe remove all the Destroy() functions, since they\r
+> now seem more dangerous than anything else...\r
+\r
+Yeah, probably.\r
+\r
+> I tried to write a test using runtime.GC to test the behaviour of\r
+> the bindings, but for some reasons some cases which are supposed to\r
+> crash don't, which makes me sceptical about the validity of the test\r
+> :-/\r
+\r
+Hmm.  Go's collector is partially conservative, IIRC, so maybe it's\r
+following a technically dead pointer?\r
+\r
+> Cheers\r
+> \r
+> Adrien\r
+> \r
+> >\r
+> >However, in your example, the notmuch_message_t's are structurally\r
+> >related to the notmuch_messages_t from whence they came.  They're all\r
+> >part of one data structure and hence it *never* makes sense for a\r
+> >caller to delete the notmuch_message_t's.  For example, even with the\r
+> >code in this patch, I think the following could lead to a crash:\r
+> >\r
+> >1. Obtain a Messages object, say ms.\r
+> >2. m1 := ms.Get()\r
+> >3. m1 = nil\r
+> >4. m2 := ms.Get()\r
+> >5. m2.whatever()\r
+> >\r
+> >If a garbage collection happens between steps 3 and 4, the Message in\r
+> >m1 will get finalized and destroyed.  But step 4 will return the same,\r
+> >now dangling, pointer, leading to a potential crash in step 5.\r
+> >\r
+> >Maybe the answer in the structural case is to include the parent\r
+> >pointer in the Go struct and not set a finalizer on the child?  That\r
+> >way, if there's a Go reference to the parent wrapper, it won't go away\r
+> >and the children won't get destroyed (collecting wrappers of children\r
+> >is fine) and if there's a Go reference to the child wrapper, it will\r
+> >keep the parent alive so it won't get destroyed and neither will the\r
+> >child.\r
+> >\r
+> >>- If we do talloc_reference(parent, wrapped), then we reproduce the\r
+> >>hierarchy again?\r
+> >>\r
+> >>Note that I have 0 experience with talloc, so I might as well be\r
+> >>getting things wrong here.\r
+> >>\r
+> >>>\r
+> >>>Quoth Adrien Bustany on Jul 18 at  9:34 pm:\r
+> >>>>This makes notmuch appropriately free the underlying notmuch C objects\r
+> >>>>when garbage collecting their Go wrappers. To make sure we don't break\r
+> >>>>the underlying links between objects (for example, a notmuch_messages_t\r
+> >>>>being GC'ed before a notmuch_message_t belonging to it), we add for each\r
+> >>>>wraper struct a pointer to the owner object (Go objects with a reference\r
+> >>>>pointing to them don't get garbage collected).\r
+> >>>>---\r
+> >>>>  bindings/go/src/notmuch/notmuch.go |  153 +++++++++++++++++++++++++++++++-----\r
+> >>>>  1 files changed, 134 insertions(+), 19 deletions(-)\r
+> >>>>\r
+> >>>>diff --git a/bindings/go/src/notmuch/notmuch.go b/bindings/go/src/notmuch/notmuch.go\r
+> >>>>index 1d77fd2..3f436a0 100644\r
+> >>>>--- a/bindings/go/src/notmuch/notmuch.go\r
+> >>>>+++ b/bindings/go/src/notmuch/notmuch.go\r
+> >>>>@@ -11,6 +11,7 @@ package notmuch\r
+> >>>>  #include "notmuch.h"\r
+> >>>>  */\r
+> >>>>  import "C"\r
+> >>>>+import "runtime"\r
+> >>>>  import "unsafe"\r
+> >>>>\r
+> >>>>  // Status codes used for the return values of most functions\r
+> >>>>@@ -47,40 +48,152 @@ func (self Status) String() string {\r
+> >>>>  /* Various opaque data types. For each notmuch_<foo>_t see the various\r
+> >>>>   * notmuch_<foo> functions below. */\r
+> >>>>\r
+> >>>>+type Object interface {}\r
+> >>>>+\r
+> >>>>  type Database struct {\r
+> >>>>         db *C.notmuch_database_t\r
+> >>>>  }\r
+> >>>>\r
+> >>>>+func createDatabase(db *C.notmuch_database_t) *Database {\r
+> >>>>+        self := &Database{db: db}\r
+> >>>>+\r
+> >>>>+        runtime.SetFinalizer(self, func(x *Database) {\r
+> >>>>+                if (x.db != nil) {\r
+> >>>>+                        C.notmuch_database_destroy(x.db)\r
+> >>>>+                }\r
+> >>>>+        })\r
+> >>>>+\r
+> >>>>+        return self\r
+> >>>>+}\r
+> >>>>+\r
+> >>>>  type Query struct {\r
+> >>>>         query *C.notmuch_query_t\r
+> >>>>+        owner Object\r
+> >>>>+}\r
+> >>>>+\r
+> >>>>+func createQuery(query *C.notmuch_query_t, owner Object) *Query {\r
+> >>>>+        self := &Query{query: query, owner: owner}\r
+> >>>>+\r
+> >>>>+        runtime.SetFinalizer(self, func(x *Query) {\r
+> >>>>+                if (x.query != nil) {\r
+> >>>>+                        C.notmuch_query_destroy(x.query)\r
+> >>>>+                }\r
+> >>>>+        })\r
+> >>>>+\r
+> >>>>+        return self\r
+> >>>>  }\r
+> >>>>\r
+> >>>>  type Threads struct {\r
+> >>>>         threads *C.notmuch_threads_t\r
+> >>>>+        owner Object\r
+> >>>>+}\r
+> >>>>+\r
+> >>>>+func createThreads(threads *C.notmuch_threads_t, owner Object) *Threads {\r
+> >>>>+        self := &Threads{threads: threads, owner: owner}\r
+> >>>>+\r
+> >>>>+        runtime.SetFinalizer(self, func(x *Threads) {\r
+> >>>>+                if (x.threads != nil) {\r
+> >>>>+                        C.notmuch_threads_destroy(x.threads)\r
+> >>>>+                }\r
+> >>>>+        })\r
+> >>>>+\r
+> >>>>+        return self\r
+> >>>>  }\r
+> >>>>\r
+> >>>>  type Thread struct {\r
+> >>>>         thread *C.notmuch_thread_t\r
+> >>>>+        owner Object\r
+> >>>>+}\r
+> >>>>+\r
+> >>>>+func createThread(thread *C.notmuch_thread_t, owner Object) *Thread {\r
+> >>>>+        self := &Thread{thread: thread, owner: owner}\r
+> >>>>+\r
+> >>>>+        runtime.SetFinalizer(self, func(x *Thread) {\r
+> >>>>+                if (x.thread != nil) {\r
+> >>>>+                        C.notmuch_thread_destroy(x.thread)\r
+> >>>>+                }\r
+> >>>>+        })\r
+> >>>>+\r
+> >>>>+        return self\r
+> >>>>  }\r
+> >>>>\r
+> >>>>  type Messages struct {\r
+> >>>>         messages *C.notmuch_messages_t\r
+> >>>>+        owner Object\r
+> >>>>+}\r
+> >>>>+\r
+> >>>>+func createMessages(messages *C.notmuch_messages_t, owner Object) *Messages {\r
+> >>>>+        self := &Messages{messages: messages, owner: owner}\r
+> >>>>+\r
+> >>>>+        return self\r
+> >>>>  }\r
+> >>>>\r
+> >>>>  type Message struct {\r
+> >>>>         message *C.notmuch_message_t\r
+> >>>>+        owner Object\r
+> >>>>+}\r
+> >>>>+\r
+> >>>>+func createMessage(message *C.notmuch_message_t, owner Object) *Message {\r
+> >>>>+        self := &Message{message: message, owner: owner}\r
+> >>>>+\r
+> >>>>+        runtime.SetFinalizer(self, func(x *Message) {\r
+> >>>>+                if (x.message != nil) {\r
+> >>>>+                        C.notmuch_message_destroy(x.message)\r
+> >>>>+                }\r
+> >>>>+        })\r
+> >>>>+\r
+> >>>>+        return self\r
+> >>>>  }\r
+> >>>>\r
+> >>>>  type Tags struct {\r
+> >>>>         tags *C.notmuch_tags_t\r
+> >>>>+        owner Object\r
+> >>>>+}\r
+> >>>>+\r
+> >>>>+func createTags(tags *C.notmuch_tags_t, owner Object) *Tags {\r
+> >>>>+        self := &Tags{tags: tags, owner: owner}\r
+> >>>>+\r
+> >>>>+        runtime.SetFinalizer(self, func(x *Tags) {\r
+> >>>>+                if (x.tags != nil) {\r
+> >>>>+                        C.notmuch_tags_destroy(x.tags)\r
+> >>>>+                }\r
+> >>>>+        })\r
+> >>>>+\r
+> >>>>+        return self\r
+> >>>>  }\r
+> >>>>\r
+> >>>>  type Directory struct {\r
+> >>>>         dir *C.notmuch_directory_t\r
+> >>>>+        owner Object\r
+> >>>>+}\r
+> >>>>+\r
+> >>>>+func createDirectory(directory *C.notmuch_directory_t, owner Object) *Directory {\r
+> >>>>+        self := &Directory{dir: directory, owner: owner}\r
+> >>>>+\r
+> >>>>+        runtime.SetFinalizer(self, func(x *Directory) {\r
+> >>>>+                if (x.dir != nil) {\r
+> >>>>+                        C.notmuch_directory_destroy(x.dir)\r
+> >>>>+                }\r
+> >>>>+        })\r
+> >>>>+\r
+> >>>>+        return self\r
+> >>>>  }\r
+> >>>>\r
+> >>>>  type Filenames struct {\r
+> >>>>         fnames *C.notmuch_filenames_t\r
+> >>>>+        owner Object\r
+> >>>>+}\r
+> >>>>+\r
+> >>>>+func createFilenames(filenames *C.notmuch_filenames_t, owner Object) *Filenames {\r
+> >>>>+        self := &Filenames{fnames: filenames, owner: owner}\r
+> >>>>+\r
+> >>>>+        runtime.SetFinalizer(self, func(x *Filenames) {\r
+> >>>>+                if (x.fnames != nil) {\r
+> >>>>+                        C.notmuch_filenames_destroy(x.fnames)\r
+> >>>>+                }\r
+> >>>>+        })\r
+> >>>>+\r
+> >>>>+        return self\r
+> >>>>  }\r
+> >>>>\r
+> >>>>  type DatabaseMode C.notmuch_database_mode_t\r
+> >>>>@@ -100,12 +213,13 @@ func NewDatabase(path string) (*Database, Status) {\r
+> >>>>                 return nil, STATUS_OUT_OF_MEMORY\r
+> >>>>         }\r
+> >>>>\r
+> >>>>-        self := &Database{db: nil}\r
+> >>>>-        st := Status(C.notmuch_database_create(c_path, &self.db))\r
+> >>>>+        var db *C.notmuch_database_t;\r
+> >>>>+        st := Status(C.notmuch_database_create(c_path, &db))\r
+> >>>>         if st != STATUS_SUCCESS {\r
+> >>>>                 return nil, st\r
+> >>>>         }\r
+> >>>>-        return self, st\r
+> >>>>+\r
+> >>>>+        return createDatabase(db), st\r
+> >>>>  }\r
+> >>>>\r
+> >>>>  /* Open an existing notmuch database located at 'path'.\r
+> >>>>@@ -134,12 +248,13 @@ func OpenDatabase(path string, mode DatabaseMode) (*Database, Status) {\r
+> >>>>                 return nil, STATUS_OUT_OF_MEMORY\r
+> >>>>         }\r
+> >>>>\r
+> >>>>-        self := &Database{db: nil}\r
+> >>>>-        st := Status(C.notmuch_database_open(c_path, C.notmuch_database_mode_t(mode), &self.db))\r
+> >>>>+        var db *C.notmuch_database_t;\r
+> >>>>+        st := Status(C.notmuch_database_open(c_path, C.notmuch_database_mode_t(mode), &db))\r
+> >>>>         if st != STATUS_SUCCESS {\r
+> >>>>                 return nil, st\r
+> >>>>         }\r
+> >>>>-        return self, st\r
+> >>>>+\r
+> >>>>+        return createDatabase(db), st\r
+> >>>>  }\r
+> >>>>\r
+> >>>>  /* Close the given notmuch database, freeing all associated\r
+> >>>>@@ -204,7 +319,7 @@ func (self *Database) GetDirectory(path string) (*Directory, Status) {\r
+> >>>>         if st != STATUS_SUCCESS || c_dir == nil {\r
+> >>>>                 return nil, st\r
+> >>>>         }\r
+> >>>>-        return &Directory{dir: c_dir}, st\r
+> >>>>+        return createDirectory(c_dir, nil), st\r
+> >>>\r
+> >>>It looks like you have a nil owner for anything whose talloc parent is\r
+> >>>the database.  Is this intentional?  Shouldn't the owner be self in\r
+> >>>these cases, too?\r
+> >>>\r
+> >>>>  }\r
+> >>>>\r
+> >>>>  /* Add a new message to the given notmuch database.\r
+> >>>>@@ -258,7 +373,7 @@ func (self *Database) AddMessage(fname string) (*Message, Status) {\r
+> >>>>         var c_msg *C.notmuch_message_t = new(C.notmuch_message_t)\r
+> >>>>         st := Status(C.notmuch_database_add_message(self.db, c_fname, &c_msg))\r
+> >>>>\r
+> >>>>-        return &Message{message: c_msg}, st\r
+> >>>>+        return createMessage(c_msg, nil), st\r
+> >>>>  }\r
+> >>>>\r
+> >>>>  /* Remove a message from the given notmuch database.\r
+> >>>>@@ -319,12 +434,12 @@ func (self *Database) FindMessage(message_id string) (*Message, Status) {\r
+> >>>>                 return nil, STATUS_OUT_OF_MEMORY\r
+> >>>>         }\r
+> >>>>\r
+> >>>>-        msg := &Message{message: nil}\r
+> >>>>-        st := Status(C.notmuch_database_find_message(self.db, c_msg_id, &msg.message))\r
+> >>>>+        var msg *C.notmuch_message_t\r
+> >>>>+        st := Status(C.notmuch_database_find_message(self.db, c_msg_id, &msg))\r
+> >>>>         if st != STATUS_SUCCESS {\r
+> >>>>                 return nil, st\r
+> >>>>         }\r
+> >>>>-        return msg, st\r
+> >>>>+        return createMessage(msg, nil), st\r
+> >>>>  }\r
+> >>>>\r
+> >>>>  /* Return a list of all tags found in the database.\r
+> >>>>@@ -339,7 +454,7 @@ func (self *Database) GetAllTags() *Tags {\r
+> >>>>         if tags == nil {\r
+> >>>>                 return nil\r
+> >>>>         }\r
+> >>>>-        return &Tags{tags: tags}\r
+> >>>>+        return createTags(tags, nil)\r
+> >>>>  }\r
+> >>>>\r
+> >>>>  /* Create a new query for 'database'.\r
+> >>>>@@ -379,7 +494,7 @@ func (self *Database) CreateQuery(query string) *Query {\r
+> >>>>         if q == nil {\r
+> >>>>                 return nil\r
+> >>>>         }\r
+> >>>>-        return &Query{query: q}\r
+> >>>>+        return createQuery(q, nil)\r
+> >>>>  }\r
+> >>>>\r
+> >>>>  /* Sort values for notmuch_query_set_sort */\r
+> >>>>@@ -459,7 +574,7 @@ func (self *Query) SearchThreads() *Threads {\r
+> >>>>         if threads == nil {\r
+> >>>>                 return nil\r
+> >>>>         }\r
+> >>>>-        return &Threads{threads: threads}\r
+> >>>>+        return createThreads(threads, self)\r
+> >>>>  }\r
+> >>>>\r
+> >>>>  /* Execute a query for messages, returning a notmuch_messages_t object\r
+> >>>>@@ -505,7 +620,7 @@ func (self *Query) SearchMessages() *Messages {\r
+> >>>>         if msgs == nil {\r
+> >>>>                 return nil\r
+> >>>>         }\r
+> >>>>-        return &Messages{messages: msgs}\r
+> >>>>+        return createMessages(msgs, self)\r
+> >>>>  }\r
+> >>>>\r
+> >>>>  /* Destroy a notmuch_query_t along with any associated resources.\r
+> >>>>@@ -607,7 +722,7 @@ func (self *Messages) Get() *Message {\r
+> >>>>         if msg == nil {\r
+> >>>>                 return nil\r
+> >>>>         }\r
+> >>>>-        return &Message{message: msg}\r
+> >>>>+        return createMessage(msg, self)\r
+> >>>>  }\r
+> >>>>\r
+> >>>>  /* Move the 'messages' iterator to the next message.\r
+> >>>>@@ -659,7 +774,7 @@ func (self *Messages) CollectTags() *Tags {\r
+> >>>>         if tags == nil {\r
+> >>>>                 return nil\r
+> >>>>         }\r
+> >>>>-        return &Tags{tags: tags}\r
+> >>>>+        return createTags(tags, self)\r
+> >>>>  }\r
+> >>>>\r
+> >>>>  /* Get the message ID of 'message'.\r
+> >>>>@@ -739,7 +854,7 @@ func (self *Message) GetReplies() *Messages {\r
+> >>>>         if msgs == nil {\r
+> >>>>                 return nil\r
+> >>>>         }\r
+> >>>>-        return &Messages{messages: msgs}\r
+> >>>>+        return createMessages(msgs, self)\r
+> >>>>  }\r
+> >>>>\r
+> >>>>  /* Get a filename for the email corresponding to 'message'.\r
+> >>>>@@ -871,7 +986,7 @@ func (self *Message) GetTags() *Tags {\r
+> >>>>         if tags == nil {\r
+> >>>>                 return nil\r
+> >>>>         }\r
+> >>>>-        return &Tags{tags: tags}\r
+> >>>>+        return createTags(tags, self)\r
+> >>>>  }\r
+> >>>>\r
+> >>>>  /* The longest possible tag value. */\r
+> \r
+> \r
+\r
+-- \r
+Austin Clements                                      MIT/'06/PhD/CSAIL\r
+amdragon@mit.edu                           http://web.mit.edu/amdragon\r
+       Somewhere in the dream we call reality you will find me,\r
+              searching for the reality we call dreams.\r