From: Justus Winter <4winter@informatik.uni-hamburg.de>
To: Austin Clements <amdragon@MIT.EDU>,
Cc: Olly Betts <olly@survex.com>, notmuch@notmuchmail.org
Subject: Re: [PATCH] Actually close the xapian database in notmuch_database_close
Date: Thu, 01 Mar 2012 07:59:30 +0100 [thread overview]
Message-ID: <20120301065930.5076.72748@thinkbox.jade-hamburg.de> (raw)
In-Reply-To: <20120229221754.GD772@mit.edu>
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#59f5f8b137723dcaaabdbdccbc0cf1eb
next prev parent reply other threads:[~2012-03-01 6:59 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-02-29 9:19 fix notmuch_database_close Justus Winter
2012-02-29 9:19 ` [PATCH] Actually close the xapian database in notmuch_database_close Justus Winter
2012-02-29 15:48 ` Austin Clements
2012-02-29 21:19 ` Olly Betts
2012-02-29 22:17 ` Austin Clements
2012-03-01 6:59 ` Justus Winter [this message]
2012-03-01 18:11 ` Austin Clements
2012-03-02 14:58 ` Justus Winter
2012-03-03 16:18 ` David Bremner
2012-03-01 21:20 ` Olly Betts
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://notmuchmail.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20120301065930.5076.72748@thinkbox.jade-hamburg.de \
--to=4winter@informatik.uni-hamburg.de \
--cc=amdragon@MIT.EDU \
--cc=notmuch@notmuchmail.org \
--cc=olly@survex.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://yhetil.org/notmuch.git/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).