Quoth Justus Winter on Mar 01 at 7:59 am: > 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 ;) Sorry, I meant a code comment. Perhaps something like, /* Many Xapian objects (and thus notmuch objects) hold references to * the database, so merely deleting the database may not suffice to * close it. Thus, we explicitly close it here. */ > >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. Olly mentioned on IRC that it can throw an exception (because, for example, close calls flush). > Cheers, > Justus > > 0: http://xapian.org/docs/apidoc/html/classXapian_1_1Database.html#59f5f8b137723dcaaabdbdccbc0cf1eb