Re: [PATCH] Actually close the xapian database in notmuch_database_close

diff --git a/57/5ebe81ccb3a3f19e39fd59b7e186a1ca995c8f b/57/5ebe81ccb3a3f19e39fd59b7e186a1ca995c8f
new file mode 100644 (file)
index 0000000..a7ca7aa
--- /dev/null
+++ b/57/5ebe81ccb3a3f19e39fd59b7e186a1ca995c8f
@@ -0,0 +1,89 @@
+Return-Path: <olly@survex.com>\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 33060431E64\r
+       for <notmuch@notmuchmail.org>; Wed, 29 Feb 2012 13:19:29 -0800 (PST)\r
+X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
+X-Spam-Flag: NO\r
+X-Spam-Score: -2.3\r
+X-Spam-Level: \r
+X-Spam-Status: No, score=-2.3 tagged_above=-999 required=5\r
+       tests=[RCVD_IN_DNSWL_MED=-2.3] 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 MMKVcRo9quLs for <notmuch@notmuchmail.org>;\r
+       Wed, 29 Feb 2012 13:19:28 -0800 (PST)\r
+Received: from atreus.tartarus.org (atreus.tartarus.org [80.252.125.10])\r
+       (using TLSv1 with cipher AES256-SHA (256/256 bits))\r
+       (No client certificate requested)\r
+       by olra.theworths.org (Postfix) with ESMTPS id 48740431FAE\r
+       for <notmuch@notmuchmail.org>; Wed, 29 Feb 2012 13:19:28 -0800 (PST)\r
+Received: from olly by atreus.tartarus.org with local (Exim 4.69)\r
+       (envelope-from <olly@survex.com>)\r
+       id 1S2qvi-00067j-MC; Wed, 29 Feb 2012 21:19:22 +0000\r
+Date: Wed, 29 Feb 2012 21:19:22 +0000\r
+From: Olly Betts <olly@survex.com>\r
+To: Austin Clements <amdragon@MIT.EDU>\r
+Subject: Re: [PATCH] Actually close the xapian database in\r
+       notmuch_database_close\r
+Message-ID: <20120229211922.GQ24964@survex.com>\r
+References:\r
+ <1330507157-22859-1-git-send-email-4winter@informatik.uni-hamburg.de>\r
+       <1330507157-22859-2-git-send-email-4winter@informatik.uni-hamburg.de>\r
+       <20120229154833.GB772@mit.edu>\r
+MIME-Version: 1.0\r
+Content-Type: text/plain; charset=us-ascii\r
+Content-Disposition: inline\r
+In-Reply-To: <20120229154833.GB772@mit.edu>\r
+User-Agent: Mutt/1.5.18 (2008-05-17)\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: Wed, 29 Feb 2012 21:19:29 -0000\r
+\r
+On Wed, Feb 29, 2012 at 10:48:33AM -0500, Austin Clements wrote:\r
+> Quoth Justus Winter on Feb 29 at 10:19 am:\r
+> > Formerly the xapian database object was deleted and closed in its\r
+> > destructor once the object was garbage collected. Explicitly call\r
+> > close() so that the database and the associated lock is released\r
+> > immediately.\r
+> \r
+> Interesting.  Is this a bug in Xapian?  According to the docs,\r
+> ~Database is supposed to close the database (if there are no other\r
+> copies, which there shouldn't be), so this should be redundant with\r
+> the delete notmuch->xapian_db a few lines down, but your experience\r
+> obviously suggests that it isn't and I can't find the code path in\r
+> Xapian that would close it in the destructor.\r
+\r
+Most Xapian API classes (including Database and WritableDatabase) just\r
+hold a reference-counted pointer, and so it's the destructor of the\r
+reference-counted object which closes the database.  If "PIMPL" means\r
+anything to you, that's what we have here.\r
+\r
+Some other API classes objects (such as PostingIterator) internally hold\r
+a reference to the database they are using, so calling close()\r
+explicitly is useful if you don't want to have to worry about such\r
+objects still existing and holding onto references which keep the\r
+database open.\r
+\r
+The main motivation for adding close() was the bindings though - e.g. in\r
+Python the wrapped Database object gets destroyed when the GC gets run,\r
+which is at some essentially arbitrary time after you remove the last\r
+reference to it.\r
+\r
+It is hard to say if calling close() is actually useful here from just\r
+seeing the patch.\r
+\r
+Cheers,\r
+    Olly\r