Hey,

I quickly searched the mailing list archives but couldn't find a related
topic, apologies if this has already been discussed.
Should Notmuch be thread-safe?

Here's a simple repro (notmuch 09f2ad8e, xapian-core 1.4.22):
// cc -Wall -g -lpthread -lnotmuch
#include <stdlib.h>
#include <pthread.h>
#include <notmuch.h>
void *thread(void *_) {
  char path[] = "/tmp/notmuch.XXXXXX";
  mkdtemp(path);
  notmuch_database_create(path, NULL);
  return NULL;
}
int main() {
  pthread_t t1, t2;
  pthread_create(&t1, NULL, thread, NULL);
  pthread_create(&t2, NULL, thread, NULL);
  pthread_join(t1, NULL);
  pthread_join(t2, NULL);
  return 0;
}

(gdb) b _exit
(gdb) commands
> run
> end
(gdb) run
... let it run until the crash happens.
(gdb) thread apply all bt
Thread 3 (Thread 0x7ffff666e640 (LWP 154253) "a.out"):
#0  0x00007ffff76822af in Xapian::Query::get_terms_begin (this=0x7fffe80137f0) at api/query.cc:141
#1  0x00007ffff7f933f5 in _notmuch_query_cache_terms (query=0x7fffe80137c0) at lib/query.cc:176
#2  0x00007ffff7f93784 in _notmuch_query_ensure_parsed_xapian (query=0x7fffe80137c0) at lib/query.cc:225
#3  0x00007ffff7f9381a in _notmuch_query_ensure_parsed (query=0x7fffe80137c0) at lib/query.cc:260
#4  0x00007ffff7f93bfe in _notmuch_query_search_documents (query=0x7fffe80137c0, type=0x7ffff7fa9b1e "mail", out=0x7ffff666da18) at lib/query.cc:361
#5  0x00007ffff7f93ba4 in notmuch_query_search_messages (query=0x7fffe80137c0, out=0x7ffff666da18) at lib/query.cc:349
#6  0x00007ffff7f83d98 in notmuch_database_upgrade (notmuch=0x7fffe8000bd0, progress_notify=0x0, closure=0x0) at lib/database.cc:934
#7  0x00007ffff7fa110f in notmuch_database_create_with_config (database_path=0x7ffff666dcb0 "/tmp/notmuch.MZ2AGr", config_path=0x7ffff7faab3c "", profile=0x0, database=0x0, status_string=0x7ffff666dc90) at lib/open.cc:754
#8  0x00007ffff7fa0d6f in notmuch_database_create_verbose (path=0x7ffff666dcb0 "/tmp/notmuch.MZ2AGr", database=0x0, status_string=0x7ffff666dc90) at lib/open.cc:653
#9  0x00007ffff7fa0ceb in notmuch_database_create (path=0x7ffff666dcb0 "/tmp/notmuch.MZ2AGr", database=0x0) at lib/open.cc:637
...

Thread 2 (Thread 0x7ffff6e6f640 (LWP 154252) "a.out"):
#0  0x00007ffff76822af in Xapian::Query::get_terms_begin (this=0x7ffff001e8a0) at api/query.cc:141
#1  0x00007ffff7f933f5 in _notmuch_query_cache_terms (query=0x7ffff001e870) at lib/query.cc:176
#2  0x00007ffff7f93784 in _notmuch_query_ensure_parsed_xapian (query=0x7ffff001e870) at lib/query.cc:225
#3  0x00007ffff7f9381a in _notmuch_query_ensure_parsed (query=0x7ffff001e870) at lib/query.cc:260
#4  0x00007ffff7f93bfe in _notmuch_query_search_documents (query=0x7ffff001e870, type=0x7ffff7fa9b1e "mail", out=0x7ffff6e6ea18) at lib/query.cc:361
#5  0x00007ffff7f93ba4 in notmuch_query_search_messages (query=0x7ffff001e870, out=0x7ffff6e6ea18) at lib/query.cc:349
#6  0x00007ffff7f83d98 in notmuch_database_upgrade (notmuch=0x7ffff0003120, progress_notify=0x0, closure=0x0) at lib/database.cc:934
#7  0x00007ffff7fa110f in notmuch_database_create_with_config (database_path=0x7ffff6e6ecb0 "/tmp/notmuch.eKNT5x", config_path=0x7ffff7faab3c "", profile=0x0, database=0x0, status_string=0x7ffff6e6ec90) at lib/open.cc:754
#8  0x00007ffff7fa0d6f in notmuch_database_create_verbose (path=0x7ffff6e6ecb0 "/tmp/notmuch.eKNT5x", database=0x0, status_string=0x7ffff6e6ec90) at lib/open.cc:653
#9  0x00007ffff7fa0ceb in notmuch_database_create (path=0x7ffff6e6ecb0 "/tmp/notmuch.eKNT5x", database=0x0) at lib/open.cc:637
...

Thread 1 (Thread 0x7ffff74468c0 (LWP 154248) "a.out"):
...

The weird number of references in this(Xapian::Query)->internal gave me
a clue and the documentation acknowledges the issue:
https://github.com/xapian/xapian/blob/4715de3a9fcee741587439dc3cc1d2ff01ffeaf2/xapian-core/include/xapian/query.h#L65

With the following patch I can't get it to crash anymore:

diff --git i/lib/query.cc w/lib/query.cc
index 707f6222..348e1ec2 100644
--- i/lib/query.cc
+++ w/lib/query.cc
@@ -188,3 +188,3 @@ _notmuch_query_string_to_xapian_query (notmuch_database_t *notmuch,
        if (query_string == "" || query_string == "*") {
-           output = Xapian::Query::MatchAll;
+           output = Xapian::Query(std::string());
        } else {

So, is Notmuch expected to be thread safe? There are some indications it
should be (e.g.: lib/init.cc has locking) but all instances of
Xapian::Query::MatchAll would have to be replaced.
Xapian states it's thread safe:
https://github.com/xapian/xapian-docsprint/blob/master/concepts/concurrency.rst

Thanks!
_______________________________________________
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-leave@notmuchmail.org