On Thu, 12 Apr 2012, Austin Clements <amdragon at MIT.EDU> wrote: > Quoth Justus Winter on Apr 12 at 11:05 am: >> Quoting Austin Clements (2012-04-01 05:23:23) >> >Quoth Justus Winter on Mar 21 at 1:55 am: >> >> I propose to split the function notmuch_database_close into >> >> notmuch_database_close and notmuch_database_destroy so that long >> >> running processes like alot can close the database while still using >> >> data obtained from queries to that database. >> > >> >Is this actually safe? My understanding of Xapian::Database::close is >> >that, once you've closed the database, basically anything can throw a >> >Xapian exception. A lot of data is retrieved lazily, both by notmuch >> >and by Xapian, so simply having, say, a notmuch_message_t object isn't >> >enough to guarantee that you'll be able to get data out of it after >> >closing the database. Hence, I don't see how this interface could be >> >used correctly. >> >> I do not know how, but both alot and afew (and occasionally the >> notmuch binary) are somehow safely using this interface on my box for >> the last three weeks. > > I see. TL;DR: This isn't safe, but that's okay if we document it. > > The bug report [0] you pointed to was quite informative. At its core, > this is really a memory management issue. To sum up for the record > (and to check my own thinking): It sounds like alot is careful not to > use any notmuch objects after closing the database. The problem is > that, currently, closing the database also talloc_free's it, which > recursively free's everything derived from it. Python later GCs the > wrapper objects, which *also* try to free their underlying objects, > resulting in a double free. > > Before the change to expose notmuch_database_close, the Python > bindings would only talloc_free from destructors. Furthermore, they > prevented the library from recursively freeing things at other times > by internally maintaining a reverse reference for every library talloc > reference (e.g., message is a sub-allocation of query, so the bindings > keep a reference from each message to its query to ensure the query > doesn't get freed). The ability to explicitly talloc_free the > database subverts this mechanism. > > > So, I've come around to thinking that splitting notmuch_database_close > and _destroy is okay. It certainly parallels the rest of the API > better. However, notmuch_database_close needs a big warning similar > to Xapian::Database::close's warning that retrieving information from > objects derived from this database may not work after calling close. > notmuch_database_close is really a specialty interface, and about the > only thing you can guarantee after closing the database is that you > can destroy other objects. This is also going to require a SONAME > major version bump, as mentioned by others. Which, to be fair, would > be a good opportunity to fix some other issues, too, like how > notmuch_database_open can't return errors and how > notmuch_database_get_directory is broken on read-only databases. The > actual bump should be done at release time, but maybe we should drop a > note somewhere (NEWS?) so we don't forget.
Can I just check that there is no way to reopen the Xapian database readonly? (I may be using the wrong term: I mean is there a way of switching an open read-write database to read-only without losing the attached structures/messages/threads etc) If I understand it this would be sufficient as it would free the lock, but could be more generally useful for long lived notmuch processes. Best wishes Mark
