debian/notmuch
1
0
Fork 0
mirror of https://git.notmuchmail.org/git/notmuch synced 2024-11-21 18:38:08 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

lib: thread-safe s-expression query parser

Browse source 
Follow-up of 6273966d, now that sfsexp 1.4.1 doesn't rely on globals
anymore by default (https://github.com/mjsottile/sfsexp/issues/21).

This simply defers the initial query generation to use the thread-safe
helper (xapian_query_match_all) instead of Xapian::Query::MatchAll.
This commit is contained in:
Kevin Boulain 2023-08-27 14:31:02 +02:00 committed by David Bremner
parent b8fe20f339
commit a1921a25b7
2 changed files with 96 additions and 35 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

89
lib/parse-sexp.cc
View file

@ -3,6 +3,7 @@
#if HAVE_SFSEXP #if HAVE_SFSEXP
#include "sexp.h" #include "sexp.h"
#include "unicode-util.h" #include "unicode-util.h"
#include "xapian-extra.h"
/* _sexp is used for file scope symbols to avoid clashing with /* _sexp is used for file scope symbols to avoid clashing with
* definitions from sexp.h */ * definitions from sexp.h */
@ -39,82 +40,99 @@ typedef enum {
/* /*
* define bitwise operators to hide casts */ * define bitwise operators to hide casts */
inline _sexp_flag_t static inline _sexp_flag_t
operator| (_sexp_flag_t a, _sexp_flag_t b) operator| (_sexp_flag_t a, _sexp_flag_t b)
{ {
return static_cast<_sexp_flag_t>( return static_cast<_sexp_flag_t>(
static_cast<unsigned>(a) | static_cast<unsigned>(b)); static_cast<unsigned>(a) | static_cast<unsigned>(b));
} }
inline _sexp_flag_t static inline _sexp_flag_t
operator& (_sexp_flag_t a, _sexp_flag_t b) operator& (_sexp_flag_t a, _sexp_flag_t b)
{ {
return static_cast<_sexp_flag_t>( return static_cast<_sexp_flag_t>(
static_cast<unsigned>(a) & static_cast<unsigned>(b)); static_cast<unsigned>(a) & static_cast<unsigned>(b));
} }
typedef enum {
SEXP_INITIAL_MATCH_ALL,
SEXP_INITIAL_MATCH_NOTHING,
} _sexp_initial_t;
static inline Xapian::Query
_sexp_initial_query (_sexp_initial_t initial)
{
switch (initial) {
case SEXP_INITIAL_MATCH_ALL:
return xapian_query_match_all ();
case SEXP_INITIAL_MATCH_NOTHING:
return Xapian::Query::MatchNothing;
}
INTERNAL_ERROR ("invalid initial sexp value %d", initial);
}
typedef struct { typedef struct {
const char *name; const char *name;
Xapian::Query::op xapian_op; Xapian::Query::op xapian_op;
Xapian::Query initial; _sexp_initial_t initial;
_sexp_flag_t flags; _sexp_flag_t flags;
} _sexp_prefix_t; } _sexp_prefix_t;
static _sexp_prefix_t prefixes[] = static _sexp_prefix_t prefixes[] =
{ {
{ "and", Xapian::Query::OP_AND, Xapian::Query::MatchAll, { "and", Xapian::Query::OP_AND, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_NONE }, SEXP_FLAG_NONE },
{ "attachment", Xapian::Query::OP_AND, Xapian::Query::MatchAll, { "attachment", Xapian::Query::OP_AND, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_FIELD | SEXP_FLAG_WILDCARD | SEXP_FLAG_EXPAND }, SEXP_FLAG_FIELD | SEXP_FLAG_WILDCARD | SEXP_FLAG_EXPAND },
{ "body", Xapian::Query::OP_AND, Xapian::Query::MatchAll, { "body", Xapian::Query::OP_AND, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_FIELD }, SEXP_FLAG_FIELD },
{ "date", Xapian::Query::OP_INVALID, Xapian::Query::MatchAll, { "date", Xapian::Query::OP_INVALID, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_RANGE }, SEXP_FLAG_RANGE },
{ "from", Xapian::Query::OP_AND, Xapian::Query::MatchAll, { "from", Xapian::Query::OP_AND, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_FIELD | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX | SEXP_FLAG_EXPAND }, SEXP_FLAG_FIELD | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX | SEXP_FLAG_EXPAND },
{ "folder", Xapian::Query::OP_OR, Xapian::Query::MatchNothing, { "folder", Xapian::Query::OP_OR, SEXP_INITIAL_MATCH_NOTHING,
SEXP_FLAG_FIELD | SEXP_FLAG_BOOLEAN | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX | SEXP_FLAG_EXPAND | SEXP_FLAG_FIELD | SEXP_FLAG_BOOLEAN | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX | SEXP_FLAG_EXPAND |
SEXP_FLAG_PATHNAME }, SEXP_FLAG_PATHNAME },
{ "id", Xapian::Query::OP_OR, Xapian::Query::MatchNothing, { "id", Xapian::Query::OP_OR, SEXP_INITIAL_MATCH_NOTHING,
SEXP_FLAG_FIELD | SEXP_FLAG_BOOLEAN | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX }, SEXP_FLAG_FIELD | SEXP_FLAG_BOOLEAN | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX },
{ "infix", Xapian::Query::OP_INVALID, Xapian::Query::MatchAll, { "infix", Xapian::Query::OP_INVALID, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_SINGLE | SEXP_FLAG_ORPHAN }, SEXP_FLAG_SINGLE | SEXP_FLAG_ORPHAN },
{ "is", Xapian::Query::OP_AND, Xapian::Query::MatchAll, { "is", Xapian::Query::OP_AND, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_FIELD | SEXP_FLAG_BOOLEAN | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX | SEXP_FLAG_EXPAND }, SEXP_FLAG_FIELD | SEXP_FLAG_BOOLEAN | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX | SEXP_FLAG_EXPAND },
{ "lastmod", Xapian::Query::OP_INVALID, Xapian::Query::MatchAll, { "lastmod", Xapian::Query::OP_INVALID, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_RANGE }, SEXP_FLAG_RANGE },
{ "matching", Xapian::Query::OP_AND, Xapian::Query::MatchAll, { "matching", Xapian::Query::OP_AND, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_DO_EXPAND }, SEXP_FLAG_DO_EXPAND },
{ "mid", Xapian::Query::OP_OR, Xapian::Query::MatchNothing, { "mid", Xapian::Query::OP_OR, SEXP_INITIAL_MATCH_NOTHING,
SEXP_FLAG_FIELD | SEXP_FLAG_BOOLEAN | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX }, SEXP_FLAG_FIELD | SEXP_FLAG_BOOLEAN | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX },
{ "mimetype", Xapian::Query::OP_AND, Xapian::Query::MatchAll, { "mimetype", Xapian::Query::OP_AND, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_FIELD | SEXP_FLAG_WILDCARD | SEXP_FLAG_EXPAND }, SEXP_FLAG_FIELD | SEXP_FLAG_WILDCARD | SEXP_FLAG_EXPAND },
{ "not", Xapian::Query::OP_AND_NOT, Xapian::Query::MatchAll, { "not", Xapian::Query::OP_AND_NOT, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_NONE }, SEXP_FLAG_NONE },
{ "of", Xapian::Query::OP_AND, Xapian::Query::MatchAll, { "of", Xapian::Query::OP_AND, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_DO_EXPAND }, SEXP_FLAG_DO_EXPAND },
{ "or", Xapian::Query::OP_OR, Xapian::Query::MatchNothing, { "or", Xapian::Query::OP_OR, SEXP_INITIAL_MATCH_NOTHING,
SEXP_FLAG_NONE }, SEXP_FLAG_NONE },
{ "path", Xapian::Query::OP_OR, Xapian::Query::MatchNothing, { "path", Xapian::Query::OP_OR, SEXP_INITIAL_MATCH_NOTHING,
SEXP_FLAG_FIELD | SEXP_FLAG_BOOLEAN | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX | SEXP_FLAG_FIELD | SEXP_FLAG_BOOLEAN | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX |
SEXP_FLAG_PATHNAME }, SEXP_FLAG_PATHNAME },
{ "property", Xapian::Query::OP_AND, Xapian::Query::MatchAll, { "property", Xapian::Query::OP_AND, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_FIELD | SEXP_FLAG_BOOLEAN | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX | SEXP_FLAG_EXPAND }, SEXP_FLAG_FIELD | SEXP_FLAG_BOOLEAN | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX | SEXP_FLAG_EXPAND },
{ "query", Xapian::Query::OP_INVALID, Xapian::Query::MatchNothing, { "query", Xapian::Query::OP_INVALID, SEXP_INITIAL_MATCH_NOTHING,
SEXP_FLAG_SINGLE | SEXP_FLAG_ORPHAN }, SEXP_FLAG_SINGLE | SEXP_FLAG_ORPHAN },
{ "regex", Xapian::Query::OP_INVALID, Xapian::Query::MatchAll, { "regex", Xapian::Query::OP_INVALID, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_SINGLE | SEXP_FLAG_DO_REGEX }, SEXP_FLAG_SINGLE | SEXP_FLAG_DO_REGEX },
{ "rx", Xapian::Query::OP_INVALID, Xapian::Query::MatchAll, { "rx", Xapian::Query::OP_INVALID, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_SINGLE | SEXP_FLAG_DO_REGEX }, SEXP_FLAG_SINGLE | SEXP_FLAG_DO_REGEX },
{ "starts-with", Xapian::Query::OP_WILDCARD, Xapian::Query::MatchAll, { "starts-with", Xapian::Query::OP_WILDCARD, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_SINGLE }, SEXP_FLAG_SINGLE },
{ "subject", Xapian::Query::OP_AND, Xapian::Query::MatchAll, { "subject", Xapian::Query::OP_AND, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_FIELD | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX | SEXP_FLAG_EXPAND }, SEXP_FLAG_FIELD | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX | SEXP_FLAG_EXPAND },
{ "tag", Xapian::Query::OP_AND, Xapian::Query::MatchAll, { "tag", Xapian::Query::OP_AND, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_FIELD | SEXP_FLAG_BOOLEAN | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX | SEXP_FLAG_EXPAND }, SEXP_FLAG_FIELD | SEXP_FLAG_BOOLEAN | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX | SEXP_FLAG_EXPAND },
{ "thread", Xapian::Query::OP_OR, Xapian::Query::MatchNothing, { "thread", Xapian::Query::OP_OR, SEXP_INITIAL_MATCH_NOTHING,
SEXP_FLAG_FIELD | SEXP_FLAG_BOOLEAN | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX | SEXP_FLAG_EXPAND }, SEXP_FLAG_FIELD | SEXP_FLAG_BOOLEAN | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX | SEXP_FLAG_EXPAND },
{ "to", Xapian::Query::OP_AND, Xapian::Query::MatchAll, { "to", Xapian::Query::OP_AND, SEXP_INITIAL_MATCH_ALL,
SEXP_FLAG_FIELD | SEXP_FLAG_WILDCARD | SEXP_FLAG_EXPAND }, SEXP_FLAG_FIELD | SEXP_FLAG_WILDCARD | SEXP_FLAG_EXPAND },
{ } { }
}; };
@ -318,7 +336,8 @@ _sexp_expand_query (notmuch_database_t *notmuch,
return NOTMUCH_STATUS_BAD_QUERY_SYNTAX; return NOTMUCH_STATUS_BAD_QUERY_SYNTAX;
} }
status = _sexp_combine_query (notmuch, NULL, NULL, prefix->xapian_op, prefix->initial, sx, status = _sexp_combine_query (notmuch, NULL, NULL, prefix->xapian_op,
_sexp_initial_query (prefix->initial), sx,
subquery); subquery);
if (status) if (status)
return status; return status;
@ -370,7 +389,8 @@ _sexp_parse_header (notmuch_database_t *notmuch, const _sexp_prefix_t *parent,
parent = &user_prefix; parent = &user_prefix;
return _sexp_combine_query (notmuch, parent, env, Xapian::Query::OP_AND, Xapian::Query::MatchAll, return _sexp_combine_query (notmuch, parent, env, Xapian::Query::OP_AND,
xapian_query_match_all (),
sx->list->next, output); sx->list->next, output);
} }
@ -520,7 +540,7 @@ _sexp_parse_range (notmuch_database_t *notmuch, const _sexp_prefix_t *prefix,
/* empty range matches everything */ /* empty range matches everything */
if (! sx) { if (! sx) {
output = Xapian::Query::MatchAll; output = xapian_query_match_all ();
return NOTMUCH_STATUS_SUCCESS; return NOTMUCH_STATUS_SUCCESS;
} }
@ -628,7 +648,7 @@ _sexp_to_xapian_query (notmuch_database_t *notmuch, const _sexp_prefix_t *parent
/* Empty list */ /* Empty list */
if (! sx->list) { if (! sx->list) {
output = Xapian::Query::MatchAll; output = xapian_query_match_all ();
return NOTMUCH_STATUS_SUCCESS; return NOTMUCH_STATUS_SUCCESS;
} }
@ -704,7 +724,8 @@ _sexp_to_xapian_query (notmuch_database_t *notmuch, const _sexp_prefix_t *parent
return _sexp_expand_query (notmuch, prefix, parent, env, sx->list->next, output); return _sexp_expand_query (notmuch, prefix, parent, env, sx->list->next, output);
} }
return _sexp_combine_query (notmuch, parent, env, prefix->xapian_op, prefix->initial, return _sexp_combine_query (notmuch, parent, env, prefix->xapian_op,
_sexp_initial_query (prefix->initial),
sx->list->next, output); sx->list->next, output);
} }
} }

42
test/T810-tsan.sh
View file

@ -4,7 +4,8 @@ test_directory=$(cd "$(dirname "${BASH_SOURCE[0]}")" > /dev/null && pwd)
test_description='run code with TSan enabled against the library' test_description='run code with TSan enabled against the library'
# Note it is hard to ensure race conditions are deterministic so this # Note it is hard to ensure race conditions are deterministic so this
# only provides best effort detection. # only provides best effort detection. Compile Notmuch with
# make CFLAGS=-fsanitize=thread LDFLAGS=-fsanitize=thread
. "$test_directory"/test-lib.sh || exit 1 . "$test_directory"/test-lib.sh || exit 1
@ -89,4 +90,43 @@ cat <<EOF > EXPECTED
EOF EOF
test_expect_equal_file EXPECTED OUTPUT test_expect_equal_file EXPECTED OUTPUT
if [ $NOTMUCH_HAVE_SFSEXP -eq 1 ]; then
test_begin_subtest "sexp query"
test_C ${MAIL_DIR} ${MAIL_DIR}-2 <<EOF
#include <notmuch-test.h>
#include <pthread.h>
void *thread (void *arg) {
char *mail_dir = arg;
notmuch_database_t *db;
/*
* Query generation from s-expression used the tread-unsafe
* Xapian::Query::MatchAll.
*/
EXPECT0(notmuch_database_open_with_config (mail_dir,
NOTMUCH_DATABASE_MODE_READ_ONLY,
NULL, NULL, &db, NULL));
notmuch_query_t *query;
EXPECT0(notmuch_query_create_with_syntax (db, "(from *)", NOTMUCH_QUERY_SYNTAX_SEXP, &query));
notmuch_messages_t *messages;
EXPECT0(notmuch_query_search_messages (query, &messages));
return NULL;
}
int main (int argc, char **argv) {
pthread_t t1, t2;
EXPECT0(pthread_create (&t1, NULL, thread, argv[1]));
EXPECT0(pthread_create (&t2, NULL, thread, argv[2]));
EXPECT0(pthread_join (t1, NULL));
EXPECT0(pthread_join (t2, NULL));
return 0;
}
EOF
cat <<EOF > EXPECTED
== stdout ==
== stderr ==
EOF
test_expect_equal_file EXPECTED OUTPUT
fi
test_done test_done