lib: replace some uses of Query::MatchAll with a thread-safe alternative

2023-03-31T11:11:39Z

This replaces two instances of Xapian::Query::MatchAll with the equivalent but thread-safe alternative Xapian::Query(std::string()). Xapian::Query::MatchAll maintains an internal pointer to a refcounted Xapian::Internal::QueryTerm. None of this is thread-safe but that wouldn't be an issue if Xapian::Query::MatchAll wasn't static. Because it's static, the refcounting goes awry when Notmuch is called from multiple threads. This is actually documented by Xapian: https://github.com/xapian/xapian/blob/4715de3a9fcee741587439dc3cc1d2ff01ffeaf2/xapian-core/include/xapian/query.h#L65 While static, Xapian::Query::MatchNothing is safe because it doesn't maintain an internal object and as such, doesn't use references. Two best-effort tests making use of TSan were added to showcase the issue (I couldn't figure out a way to deterministically reproduce it without making an unmaintainable mess). First, when two databases are created in parallel, a query that uses Xapian::Query::MatchAll is made (lib/query.cc), resulting in the following backtrace on a segfault: #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 ... Second, some queries would make use of Xapian::Query::MatchAll (lib/regexp-fields.cc), resulting in the following backtrace on a segfault: #0 0x00007f629828b690 in Xapian::Internal::QueryBranch::gather_terms (this=0x7f628800def0, void_terms=0x7f629726d5a0) at api/queryinternal.cc:1245 #1 0x00007f629828c260 in Xapian::Internal::QueryScaleWeight::gather_terms (this=0x7f628800df70, void_terms=0x7f629726d5a0) at api/queryinternal.cc:1434 #2 0x00007f629828b69f in Xapian::Internal::QueryBranch::gather_terms (this=0x7f628800dd90, void_terms=0x7f629726d5a0) at api/queryinternal.cc:1245 #3 0x00007f6298282571 in Xapian::Query::get_unique_terms_begin (this=0x7f628800dcd8) at api/query.cc:166 #4 0x00007f629841a59b in Xapian::Weight::Internal::accumulate_stats (this=0x7f628800dca0, subdb=..., rset=...) at weight/weightinternal.cc:86 #5 0x00007f62983c15ba in LocalSubMatch::prepare_match (this=0x7f628800df20, nowait=true, total_stats=...) at matcher/localsubmatch.cc:172 #6 0x00007f62983c8fcc in prepare_sub_matches (leaves=std::vector of length 1, capacity 1 = {...}, stats=...) at matcher/multimatch.cc:237 #7 0x00007f62983c98a3 in MultiMatch::MultiMatch (this=0x7f629726d9a0, db_=..., query_=..., qlen=3, omrset=0x0, collapse_max_=0, collapse_key_=4294967295, percent_cutoff_=0, weight_cutoff_=0, order_=Xapian::Enquire::ASCENDING, sort_key_=0, sort_by_=Xapian::Enquire::Internal::VAL, sort_value_forward_=true, time_limit_=0, stats=..., weight_=0x7f6288008d50, matchspies_=std::vector of length 0, capacity 0, have_sorter=false, have_mdecider=false) at matcher/multimatch.cc:353 #8 0x00007f629826fcba in Xapian::Enquire::Internal::get_mset (this=0x7f628800e0b0, first=0, maxitems=0, check_at_least=0, rset=0x0, mdecider=0x0) at api/omenquire.cc:569 #9 0x00007f629827181c in Xapian::Enquire::get_mset (this=0x7f629726db80, first=0, maxitems=0, check_at_least=0, rset=0x0, mdecider=0x0) at api/omenquire.cc:937 #10 0x00007f6298be529a in _notmuch_query_search_documents (query=0x7f6288009750, type=0x7f6298bfaafe "mail", out=0x7f629726dcc0) at lib/query.cc:447 #11 0x00007f6298be4ae8 in notmuch_query_search_messages (query=0x7f6288009750, out=0x7f629726dcc0) at lib/query.cc:349 ... Printing Xapian::Query::MatchAll->internal.px->_refs in these circumstances can help quickly identifying this scenario. This is motivated by some test frameworks (like Rust's Cargo) that runs unit tests in parallel and would easily encounter this issue, unless client code gates every call to Notmuch behind a lock. This is what can be expected from the tests when they fail: == stderr == +================== +WARNING: ThreadSanitizer: data race (pid=207931) + Read of size 1 at 0x7b10000001a0 by thread T2: + #0 memcpy (libtsan.so.2+0x62506) + #1 void std::__cxx11::basic_string, std::allocator >::_M_construct(char*, char*, std::forward_iterator_tag) [clone .isra.0] (libxapian.so.30+0x872b3) + + Previous write of size 8 at 0x7b10000001a0 by thread T1: + #0 operator new(unsigned long) (libtsan.so.2+0x8ba83) + #1 Xapian::Query::Query(std::__cxx11::basic_string, std::allocator > const&, unsigned int, unsigned int) (libxapian.so.30+0x855cd) ...

fix build without sfsexp

2022-04-15T17:17:31Z

a1d139de ("lib: add sexp: prefix to Xapian (infix) query parser.", 2022-04-09) introduced sfsexp infix queries. This requires the infix preprocessor to be built in in a way which does not require sfsexp when notmuch is built without it. Make the preprocessor throw a Xapian error in this case (and fix the build). Signed-off-by: Michael J Gruber

lib/thread-fp: factor out query expansion, rewrite in Xapian

2021-09-05T00:07:19Z

It will be convenient not to have to construct a notmuch query object when parsing subqueries, so the commit rewrites the query expansion (currently only used for thread:{} queries) using only Xapian. As a bonus it seems about 15% faster in initial experiments.

lib/query: factor out _notmuch_query_string_to_xapian_query

2021-09-05T00:07:19Z

When dealing with recursive queries (i.e. thread:{foo}) it turns out to be useful just to deal with the underlying Xapian objects, and not wrap them in notmuch objects.

lib: generate actual Xapian query for "*" and ""

2021-09-05T00:07:19Z

The previous code had the somewhat bizarre effect that the (notmuch specific) query string was "*" (interpreted as MatchAll) and the allegedly parsed xapian_query was "MatchNothing". This commit also reduces code duplication.

lib/query: generalize exclude handling to s-expression queries

2021-09-05T00:07:19Z

In fact most of the code path is in common, only the caching of terms in the query needs to be added for s-expression queries.

lib/parse-sexp: parse single terms and the empty list.

2021-09-05T00:07:19Z

There is not much of a parser here yet, but it already does some useful error reporting. Most functionality sketched in the documentation is not implemented yet; detailed documentation will follow with the implementation.

lib: define notmuch_query_create_with_syntax

2021-09-05T00:07:19Z

Set the parsing syntax when the (notmuch) query object is created. Initially the library always returns a trivial query that matches all messages when using s-expression syntax. It seems better to select the syntax at query creation time because the lazy parsing is an implementation detail.

lib: split notmuch_query_create

2021-09-05T00:07:19Z

Most of the function will be re-usable when creating a query from an s-expression.

lib: run uncrustify

2019-06-14T10:41:27Z

This is the result of running $ uncrustify --replace --config ../devel/uncrustify.cfg *.c *.h *.cc in the lib directory

notmuch/lib/query.cc, branch 0.38.3

lib: replace some uses of Query::MatchAll with a thread-safe alternative

fix build without sfsexp

lib/thread-fp: factor out query expansion, rewrite in Xapian

lib/query: factor out _notmuch_query_string_to_xapian_query

lib: generate actual Xapian query for "*" and ""

lib/query: generalize exclude handling to s-expression queries

lib/parse-sexp: parse single terms and the empty list.

lib: define notmuch_query_create_with_syntax

lib: split notmuch_query_create

lib: run uncrustify