notmuch/util, branch 0.39_rc3

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) ...

config: ignore leading/trailing spaces in ';'-delimited lists

2021-12-04T16:16:12Z

In [1] Ciprian observed that it was easy for users to mistakenly introduce leading and trailing space to new.tags when editing a notmuch config file. This commit strips spaces on either side of the ';' delimiter when splitting. In principle it would be possible to support tags (or other config values) with leading or trailing spaces by processing '\s' escapes in the input string. Currently such processing is not done. [1]: id:CA+Tk8fzjPLaEd3vL1f9ebk_bF_RV8PDTLzDupraTkCLCpJAmCg@mail.gmail.com

cli: remove enum names from typedefs

2021-10-23T11:39:16Z

There are some enum typedefs with the enum name: typedef enum _name_t { ... } name_t; We don't need or use the enum names _name_t for anything, and not all of the enum typedefs have them. We have the typedefs specifically to use the typedef name. Use the anonymous enum in the typedefs: typedef enum { ... } name_t;

util/unicode: allow calling from C++

2021-09-05T00:07:19Z

The omission of the 'extern "C"' machinery seems like an oversight.

cli/show: produce "email" element in sigstatus

2021-06-26T16:07:47Z

When the certificate that signs a message is known to be valid, GMime is capable of reporting on the e-mail address embedded in the certificate. We pass this information along to the caller of "notmuch show", as often only the e-mail address of the certificate has actually been checked/verified. Furthermore, signature verification should probably at some point compare the e-mail address of the caller against the sender address of the message itself. Having to parse what gmime thinks is a "userid" to extract an e-mail address seems clunky and unnecessary if gmime already thinks it knows what the e-mail address is. See id:878s41ax6t.fsf@fifthhorseman.net for more motivation and discussion. Signed-off-by: Daniel Kahn Gillmor

compat: rename {,notmuch_}canonicalize_file_name

2021-04-24T11:07:00Z

When compat canonicalize_file_name was introduced, it was limited to C code only because it was used by C code only during that time. >From 5ec6fd4d, (lib/open: check for split configuration when creating database., 2021-02-16), lib/open.cc, which is C++, relies on the existent of canonicalize_file_name. However, we can't blindly enable canonicalize_file_name for C++ code, because different implementation has different additional signature for C++ and users can arbitrarily add -DHAVE_CANONICALIZE_FILE_NAME=0 to {C,CXX}FLAGS. Let's move our implementation into a util library. Helped-by: Tomi Ollila Signed-off-by: Đoàn Trần Công Danh

util: run uncrustify

2021-03-13T12:45:34Z

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

util: add strsplit_len: simplified strtok with delimiter escaping

2021-02-06T23:06:49Z

This will be used to make iterators for configuration values.

emacs: Use makefile-gmake-mode in Makefile*s

2020-08-10T00:14:36Z

Use `makefile-gmake-mode' instead of `makefile-mode' because the former also highlights ifdef et al. while the latter does not. "./Makefile.global" and one "Makefile.local" failed to specify any major mode at all but doing so is necessary because Emacs does not automatically figure out that these are Makefiles (of any flavor).

crypto: handle PKCS#7 envelopedData in _notmuch_crypto_decrypt

2020-05-23T01:11:40Z

In the two places where _notmuch_crypto_decrypt handles multipart/encrypted messages (PGP/MIME), we should also handle PKCS#7 envelopedData (S/MIME). This is insufficient for fully handling S/MIME encrypted data because _notmuch_crypto_decrypt isn't yet actually invoked for envelopedData parts, but that will happen in the following changes. Signed-off-by: Daniel Kahn Gillmor