From a025f0ba571a8c8b6586446b2eb25c0d4c94c2cf Mon Sep 17 00:00:00 2001 From: Justus Winter <4winter@informatik.uni-hamburg.de> Date: Thu, 1 Mar 2012 07:59:30 +0100 Subject: [PATCH] Re: [PATCH] Actually close the xapian database in notmuch_database_close --- f6/6479963ddb41c5fa8d63274d4b5dc2158379fc | 123 ++++++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100644 f6/6479963ddb41c5fa8d63274d4b5dc2158379fc diff --git a/f6/6479963ddb41c5fa8d63274d4b5dc2158379fc b/f6/6479963ddb41c5fa8d63274d4b5dc2158379fc new file mode 100644 index 000000000..405e95c47 --- /dev/null +++ b/f6/6479963ddb41c5fa8d63274d4b5dc2158379fc @@ -0,0 +1,123 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by olra.theworths.org (Postfix) with ESMTP id 0E35D431E64 + for ; Wed, 29 Feb 2012 22:59:39 -0800 (PST) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: 0 +X-Spam-Level: +X-Spam-Status: No, score=0 tagged_above=-999 required=5 tests=[none] + autolearn=disabled +Received: from olra.theworths.org ([127.0.0.1]) + by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) + with ESMTP id IOJmPwFFVzIA for ; + Wed, 29 Feb 2012 22:59:36 -0800 (PST) +Received: from mail.cryptobitch.de (cryptobitch.de [88.198.7.68]) + (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) + (No client certificate requested) + by olra.theworths.org (Postfix) with ESMTPS id 8AAD0431FAE + for ; Wed, 29 Feb 2012 22:59:36 -0800 (PST) +Received: from mail.jade-hamburg.de (unknown [85.183.11.228]) + (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) + (No client certificate requested) + by mail.cryptobitch.de (Postfix) with ESMTPSA id 00F2D2CACE3 + for ; Thu, 1 Mar 2012 07:59:34 +0100 (CET) +Received: by mail.jade-hamburg.de (Postfix, from userid 401) + id 6BB89DF2A7; Thu, 1 Mar 2012 07:59:33 +0100 (CET) +Received: from thinkbox.jade-hamburg.de (unknown [10.1.1.153]) + (using TLSv1 with cipher AES256-SHA (256/256 bits)) + (No client certificate requested) (Authenticated sender: teythoon) + by mail.jade-hamburg.de (Postfix) with ESMTPSA id 61D48DF2A2; + Thu, 1 Mar 2012 07:59:31 +0100 (CET) +Received: from teythoon by thinkbox.jade-hamburg.de with local (Exim 4.77) + (envelope-from ) + id 1S2zz8-0000Su-Ht; Thu, 01 Mar 2012 07:59:30 +0100 +Content-Type: text/plain; charset="utf-8" +MIME-Version: 1.0 +Content-Transfer-Encoding: quoted-printable +To: Austin Clements , +From: Justus Winter <4winter@informatik.uni-hamburg.de> +In-Reply-To: <20120229221754.GD772@mit.edu> +References: + <1330507157-22859-1-git-send-email-4winter@informatik.uni-hamburg.de> + <1330507157-22859-2-git-send-email-4winter@informatik.uni-hamburg.de> + <20120229154833.GB772@mit.edu> <20120229211922.GQ24964@survex.com> + <20120229221754.GD772@mit.edu> +Message-ID: <20120301065930.5076.72748@thinkbox.jade-hamburg.de> +User-Agent: alot/0.21+ +Subject: Re: [PATCH] Actually close the xapian database in + notmuch_database_close +Date: Thu, 01 Mar 2012 07:59:30 +0100 +Cc: Olly Betts , notmuch@notmuchmail.org +X-BeenThere: notmuch@notmuchmail.org +X-Mailman-Version: 2.1.13 +Precedence: list +List-Id: "Use and development of the notmuch mail system." + +List-Unsubscribe: , + +List-Archive: +List-Post: +List-Help: +List-Subscribe: , + +X-List-Received-Date: Thu, 01 Mar 2012 06:59:39 -0000 + +Hi :) + +Olly wrote: +>It is hard to say if calling close() is actually useful here from just +>seeing the patch. + +Huh? I provided a test case... + +Quoting Austin Clements (2012-02-29 23:17:54) +>Quoth Olly Betts on Feb 29 at 9:19 pm: +>> On Wed, Feb 29, 2012 at 10:48:33AM -0500, Austin Clements wrote: +>> > Quoth Justus Winter on Feb 29 at 10:19 am: +>> > > Formerly the xapian database object was deleted and closed in its +>> > > destructor once the object was garbage collected. Explicitly call +>> > > close() so that the database and the associated lock is released +>> > > immediately. +>> > = + +>> > Interesting. Is this a bug in Xapian? According to the docs, +>> > ~Database is supposed to close the database (if there are no other +>> > copies, which there shouldn't be), so this should be redundant with +>> > the delete notmuch->xapian_db a few lines down, but your experience +>> > obviously suggests that it isn't and I can't find the code path in +>> > Xapian that would close it in the destructor. +>> = + +>> Most Xapian API classes (including Database and WritableDatabase) just +>> hold a reference-counted pointer, and so it's the destructor of the +>> reference-counted object which closes the database. If "PIMPL" means +>> anything to you, that's what we have here. +>> = + +>> Some other API classes objects (such as PostingIterator) internally hold +>> a reference to the database they are using, so calling close() +>> explicitly is useful if you don't want to have to worry about such +>> objects still existing and holding onto references which keep the +>> database open. +> +>Makes sense. Justus, could you add a comment to your patch explaining +>that we explicitly close the database because there may be other +>objects with references to it that would keep it open? + +I thought I did, I'm not a native speaker though, so if you want to +reword my message be my guest ;) + +>Also, since close could throw an exception, it should get wrapped in a +>try/catch like flush currently is. + +My interpretation of [0] was that Xapian::Database::close() does not +throw any exceptions. + +Cheers, +Justus + +0: http://xapian.org/docs/apidoc/html/classXapian_1_1Database.html#59f5f8b1= +37723dcaaabdbdccbc0cf1eb -- 2.26.2