[RFC] Split notmuch_database_close into two functions

Justus Winter 4winter at informatik.uni-hamburg.de
Thu Apr 12 02:05:33 PDT 2012


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.

>Maybe you could describe your use case in more detail?

Uh, okay. There are two long running processes (alot and afew in my
case) competing for a resource ("writable access to the notmuch
database"). This access is serialized by a lock maintained by
libxapian. This is a rather classic cs problem.

In order to avoid starvation of one process, both processes must
release the lock after a finite amount of time. Now for all practical
purposes the requirement is usually that both processes should
minimize the time spent while they aquired the lock.

The only way to ensure that the lock is released is to ask notmuch to
release the lock. When Patrick asked me for a way to release the lock
I exposed the function notmuch_database_close [0].

I made a mistake. The mistake was to assume that a function named
notmuch_database_close would actually do as the name implies. But that
wasn't the case. I should have been more suspicious, but being
somewhat naive I patched notmuch_database_close to actually close the
database [1].

I'm basically trying to correct that mistake. One way to fix that is
this patchset, the other one is to rename notmuch_database_close to
notmuch_database_destroy and to remove the ability to call this
function from the python bindings.

There is a branch of alot [2] that needs this functionality and hence
this patchset to avoid crashes (the ticket contains more information
why this leads to crashes). Incidentally this branch also fixes
another bug in alot [3]. Patrick spoke up in this thread stating that
this patchset is useful for alot. There is a unpublished branch of
afew that requires this functionality.

I think it very much boils down to the question whether or not
libnotmuch and its users are second class citizens. This issue is not
a problem for the notmuch binary since it is short lived in nature.

In fact I feel reminded of [4] when I pointed out problems in the code
that are problematic for libnotmuch users. You - Austin - suggested to
whip up patches instead of raising concerns. That's a valid response,
after all, talk is cheap ;)

But now that I have a patchset that is rather small (yes, it changes
the API but any program can be adjusted automatically using sed...)
and I kind of thought that it would be accepted more easily. And
although the changes are trivial it took me quite some time to track
it down.

The thread [4] dried out without someone like you or David stating
that he cares about libnotmuch and its users as much as the users of
the notmuch binary. These kind of problems require invasive code and
API changes, I asked in that very same thread how to do this properly
but noone answered.

I think I'm just not willing to devote my time to fix these problems
if these issues aren't even perceived as problems by the majority of
notmuch users and developers b/c they are using the emacs interface
which just calls the notmuch binary that has no such problems.

Cheers,
Justus

0: b2734519db78fdec76eeafc5fe8f5631a6436cf6
1: cfc5f1059aa16753cba610c41601cacc97260e08
2: https://github.com/pazz/alot/issues/413
3: https://github.com/pazz/alot/issues/414
4: 20120221002921.8534.57091 at thinkbox.jade-hamburg.de


More information about the notmuch mailing list