From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp12.migadu.com ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms9.migadu.com with LMTPS id cK5JAdM3K2SgjgAASxT56A (envelope-from ) for ; Mon, 03 Apr 2023 22:32:19 +0200 Received: from aspmx1.migadu.com ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp12.migadu.com with LMTPS id 0DUcAdM3K2S+XgEAauVa8A (envelope-from ) for ; Mon, 03 Apr 2023 22:32:19 +0200 Received: from mail.notmuchmail.org (yantan.tethera.net [135.181.149.255]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id D0B9235914 for ; Mon, 3 Apr 2023 22:32:18 +0200 (CEST) Received: from yantan.tethera.net (localhost [127.0.0.1]) by mail.notmuchmail.org (Postfix) with ESMTP id 35C625F804; Mon, 3 Apr 2023 20:32:16 +0000 (UTC) Received: from out-23.mta1.migadu.com (out-23.mta1.migadu.com [95.215.58.23]) by mail.notmuchmail.org (Postfix) with ESMTPS id 319605F7FF for ; Mon, 3 Apr 2023 20:32:14 +0000 (UTC) X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=boula.in; s=key1; t=1680553933; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=n7HzuJ+zfg2MwYQxGOh+QER8fkssh3SVDg76DUYWmpA=; b=iO57qZPEhpZr7d+BdEmtSs0MU/dSdyRyJg9M+30aPZnaWV68/jh7AQqM7e1cEx67gSoJZL IbQZhbsOqeLqSnfdnrtaHCbpbuZqBnXh+QbvnP/6nBnEjOXOHjOMQzLCDptr7F0HbtV1nV DCN+Owqe6sNZR3R9lebHS+uqJBvsNEAYhdMTs5yJXXhfTlEKBVU+oFWv2IcKpL61J8Hvq9 IgTjtdzglpNM8hOSHtOEZSfM9q56TQh6wwWh1Tiw6GnLs83jyWRCgzMEMmKR+Gf44Blgca gSKUoQbkti/HVzlZORDBGCAx9ALkHmv4DdW4IhzUl92HmKFrWftKtlcTLfjjYQ== From: Kevin Boulain To: notmuch@notmuchmail.org Subject: [PATCH 2/2] lib: thread-safe s-expression query parser Date: Mon, 3 Apr 2023 22:31:46 +0200 Message-Id: <20230403203146.39749-2-kevin@boula.in> In-Reply-To: <20230403203146.39749-1-kevin@boula.in> References: <20230403203146.39749-1-kevin@boula.in> MIME-Version: 1.0 Message-ID-Hash: 7SVGQKBSETGDDLDFIUZIWZBNHNAS4J7V X-Message-ID-Hash: 7SVGQKBSETGDDLDFIUZIWZBNHNAS4J7V X-MailFrom: kevin@boula.in X-Mailman-Rule-Hits: nonmember-moderation X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-notmuch.notmuchmail.org-0 CC: Kevin Boulain X-Mailman-Version: 3.3.3 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit X-Migadu-Country: DE X-Migadu-Flow: FLOW_IN X-Migadu-Spam-Score: -4.00 X-Migadu-Scanner: scn1.migadu.com Authentication-Results: aspmx1.migadu.com; none X-Spam-Score: -4.00 X-Migadu-Queue-Id: D0B9235914 X-TUID: gry6tkGWYffc 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. --- lib/parse-sexp.cc | 85 +++++++++++++++++++++++++++++------------------ test/T810-tsan.sh | 1 - 2 files changed, 53 insertions(+), 33 deletions(-) diff --git a/lib/parse-sexp.cc b/lib/parse-sexp.cc index 9cadbc13..825bb9f9 100644 --- a/lib/parse-sexp.cc +++ b/lib/parse-sexp.cc @@ -3,6 +3,7 @@ #if HAVE_SFSEXP #include "sexp.h" #include "unicode-util.h" +#include "xapian-extra.h" /* _sexp is used for file scope symbols to avoid clashing with * definitions from sexp.h */ @@ -53,68 +54,85 @@ operator& (_sexp_flag_t a, _sexp_flag_t b) static_cast(a) & static_cast(b)); } +typedef enum { + SEXP_INITIAL_MATCH_ALL, + SEXP_INITIAL_MATCH_NOTHING, +} _sexp_initial_t; + +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; + } + assert (! "unreachable"); +} + typedef struct { const char *name; Xapian::Query::op xapian_op; - Xapian::Query initial; + _sexp_initial_t initial; _sexp_flag_t flags; } _sexp_prefix_t; 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 }, - { "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 }, - { "body", Xapian::Query::OP_AND, Xapian::Query::MatchAll, + { "body", Xapian::Query::OP_AND, SEXP_INITIAL_MATCH_ALL, SEXP_FLAG_FIELD }, - { "date", Xapian::Query::OP_INVALID, Xapian::Query::MatchAll, + { "date", Xapian::Query::OP_INVALID, SEXP_INITIAL_MATCH_ALL, 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 }, - { "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_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 }, - { "infix", Xapian::Query::OP_INVALID, Xapian::Query::MatchAll, + { "infix", Xapian::Query::OP_INVALID, SEXP_INITIAL_MATCH_ALL, 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 }, - { "lastmod", Xapian::Query::OP_INVALID, Xapian::Query::MatchAll, + { "lastmod", Xapian::Query::OP_INVALID, SEXP_INITIAL_MATCH_ALL, SEXP_FLAG_RANGE }, - { "matching", Xapian::Query::OP_AND, Xapian::Query::MatchAll, + { "matching", Xapian::Query::OP_AND, SEXP_INITIAL_MATCH_ALL, 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 }, - { "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 }, - { "not", Xapian::Query::OP_AND_NOT, Xapian::Query::MatchAll, + { "not", Xapian::Query::OP_AND_NOT, SEXP_INITIAL_MATCH_ALL, SEXP_FLAG_NONE }, - { "of", Xapian::Query::OP_AND, Xapian::Query::MatchAll, + { "of", Xapian::Query::OP_AND, SEXP_INITIAL_MATCH_ALL, SEXP_FLAG_DO_EXPAND }, - { "or", Xapian::Query::OP_OR, Xapian::Query::MatchNothing, + { "or", Xapian::Query::OP_OR, SEXP_INITIAL_MATCH_NOTHING, 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_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 }, - { "query", Xapian::Query::OP_INVALID, Xapian::Query::MatchNothing, + { "query", Xapian::Query::OP_INVALID, SEXP_INITIAL_MATCH_NOTHING, 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 }, - { "rx", Xapian::Query::OP_INVALID, Xapian::Query::MatchAll, + { "rx", Xapian::Query::OP_INVALID, SEXP_INITIAL_MATCH_ALL, 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 }, - { "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 }, - { "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 }, - { "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 }, - { "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 }, { } }; @@ -318,7 +336,8 @@ _sexp_expand_query (notmuch_database_t *notmuch, 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); if (status) return status; @@ -370,7 +389,8 @@ _sexp_parse_header (notmuch_database_t *notmuch, const _sexp_prefix_t *parent, 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); } @@ -520,7 +540,7 @@ _sexp_parse_range (notmuch_database_t *notmuch, const _sexp_prefix_t *prefix, /* empty range matches everything */ if (! sx) { - output = Xapian::Query::MatchAll; + output = xapian_query_match_all (); return NOTMUCH_STATUS_SUCCESS; } @@ -628,7 +648,7 @@ _sexp_to_xapian_query (notmuch_database_t *notmuch, const _sexp_prefix_t *parent /* Empty list */ if (! sx->list) { - output = Xapian::Query::MatchAll; + output = xapian_query_match_all (); 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_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); } } diff --git a/test/T810-tsan.sh b/test/T810-tsan.sh index c9008c6b..e876306c 100755 --- a/test/T810-tsan.sh +++ b/test/T810-tsan.sh @@ -91,7 +91,6 @@ test_expect_equal_file EXPECTED OUTPUT if [ $NOTMUCH_HAVE_SFSEXP -eq 1 ]; then test_begin_subtest "sexp query" - test_subtest_known_broken test_C ${MAIL_DIR} ${MAIL_DIR}-2 < #include