On 2023-08-25 at 08:07 -03, David Bremner wrote:
> I can just disable tsan tests on ppc64el for Debian, but I wondered if
> there is an underlying bug that only shows up on ppc64el
I see you've skipped T810-tsan in 90c61828, I think it's fair for the
time being. I did some digging and sent a
On ppc64el, races are detected by TSan:
WARNING: ThreadSanitizer: data race (pid=4520)
Read of size 8 at 0x720016c0 by thread T1:
#0 strlen
../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:386
(libtsan.so.2+0x77c0c)
#1 strlen
On 2023-07-22 at 16:06 -03, David Bremner wrote:
> My first thought was that this should be static, but maybe it doesn't
> matter in C++; I see the other inline functions in that file are not
> declared static.
True, I should have marked this function static. I did the same for the
others.
> I
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.
---
The test fails reliably when Notmuch is compiled as such:
./configure
make CFLAGS=-fsanitize=thread LDFLAGS=-fsanitize=thread
It's still unclear how the test suite could be run with the correct
compilation flags.
Output:
T810-tsan: Testing run code with TSan enabled against the library
On 2023-08-25 at 08:07 -03, David Bremner wrote:
> Does this failure make any sense to you?
>
>
> https://buildd.debian.org/status/fetch.php?pkg=notmuch=ppc64el=0.38%7Erc0-1=1692959868=0
It doesn't ring any bell. Do you happen to know if this is reproducible
or if we could get a better
On 2023-07-22 at 15:48 -03, David Bremner wrote:
>
> I can't get this test to fail at all. I'm not sure what to conclude from
> that. This is on a 20 core system running linux.
I can't really remember what I did at the time but I believe only the
test is instrumented and not the Notmuch library.
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.
---
The test fails quite reliably for me:
T810-tsan: Testing run code with TSan enabled against the library
PASS create
PASS query
FAIL sexp query
--- T810-tsan.3.EXPECTED2023-04-03 19:53:04.400771102 +
+++ T810-tsan.3.OUTPUT 2023-04-03
On 2023-03-29 at 08:32 -03, David Bremner wrote:
> It would be nice to structure this in terms of a known broken test
> (perhaps modify the existing one to reopen the database and dump the
> properties)
> that is then fixed by patch adding the sync.
I have restructured the commits to hopefully
On 2023-03-29 at 07:42 -03, David Bremner wrote:
> Thanks for the speedy update. I hate to do this but I think the message
> is now _too_ specific. Goldilocks and the three tests I guess.
>
> In particular, is there some reason to think that the number 54 is
> stable here? I'm worried about the
Since libnotmuch exposes a C interface there's no way for clients to
catch this.
Inspired by what's done for tags (see notmuch_message_remove_tag).
---
lib/message-property.cc | 36 +--
test/T610-message-property.sh | 2 --
2 files changed, 30 insertions(+),
These two functions don't fail gracefully when editing a removed
message:
BROKEN edit property on removed message without uncaught exception
--- T610-message-property.20.EXPECTED 2023-02-27 11:33:25.792764376
+
+++ T610-message-property.20.OUTPUT 2023-02-27
It wasn't covered, though it shares most of its implementation with
notmuch_message_remove_all_properties.
---
test/T610-message-property.sh | 15 +++
1 file changed, 15 insertions(+)
diff --git a/test/T610-message-property.sh b/test/T610-message-property.sh
index e4a4b89c..480b04fc
notmuch_message_remove_all_properties should have removed the
testkey1 = testvalue1
property but hasn't. Delay the execution of the corresponding test
to avoid updating a few tests that actually relied on the broken
behavior.
---
test/T610-message-property.sh | 39
_notmuch_message_remove_all_properties wasn't syncing the message back
to the database but was still invalidating the metadata, giving the
impression the properties had actually been removed.
Also move the metadata invalidation to _notmuch_message_remove_terms
to be closer to what's done in
The other tests rely on a stable output.
---
test/T610-message-property.sh | 12
1 file changed, 12 deletions(-)
diff --git a/test/T610-message-property.sh b/test/T610-message-property.sh
index 7ebddae3..383090e8 100755
--- a/test/T610-message-property.sh
+++
To make the tests a bit easier to understand.
---
test/T610-message-property.sh | 26 +-
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/test/T610-message-property.sh b/test/T610-message-property.sh
index 2685f3b5..7ebddae3 100755
---
On 2023-03-27 at 08:30 -03, David Bremner wrote:
> So you should probably test that the status is not success, and
> ideally print the message. You can see some examples in
> T560-lib-error.sh, in particular in the created file c_tail which
> handles checking the error code and printing the
Since libnotmuch exposes a C interface there's no way for clients to
catch this.
Inspired by what's done for tags (see notmuch_message_remove_tag).
---
lib/message-property.cc | 36 +--
test/T610-message-property.sh | 2 --
2 files changed, 30 insertions(+),
These two functions don't fail gracefully when editing a removed
message:
BROKEN edit property on removed message without uncaught exception
--- T610-message-property.20.EXPECTED 2023-02-27 11:33:25.792764376
+
+++ T610-message-property.20.OUTPUT 2023-02-27
On 2023-03-03 at 00:39 +02, Tomi Ollila wrote:
> Somehow testkey1 = testvalue1 disappeared from the test code (which is
> probably expected -- perhaps the commit message of the *change* 1/2
> tried to point to that ;D)
Yes, that proves notmuch_message_remove_all_properties is broken without
the
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
_notmuch_message_remove_all_properties wasn't syncing the message back
to the database but was still invalidating the metadata, giving the
impression the properties had actually been removed.
Also move the metadata invalidation to _notmuch_message_remove_terms
to be closer to what's done in
To ease the review of the next patch.
---
test/T610-message-property.sh | 26 +-
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/test/T610-message-property.sh b/test/T610-message-property.sh
index 2685f3b5..7ebddae3 100755
---
Likely missed in 86cbd215e, when notmuch_query_search_messages_st was
renamed to notmuch_query_search_messages.
---
> I don't think we should encourage people to ignore the status code.
Makes sense, I wasn't too sure if the example was deliberately
minimal.
lib/notmuch.h | 5 -
1 file
Since libnotmuch exposes a C interface there's no way for clients to
catch this.
Inspired by what's done for tags (see notmuch_message_remove_tag).
---
lib/message-property.cc | 36 +--
test/T610-message-property.sh | 4 ++--
2 files changed, 32
These two functions don't fail gracefully when editing a removed
message:
BROKEN edit property on removed message without uncaught exception
--- T610-message-property.20.EXPECTED 2023-02-27 11:33:25.792764376
+
+++ T610-message-property.20.OUTPUT 2023-02-27
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
hub.com/mjsottile/sfsexp/blob/master/src/parser.c#L270
Thoughts? Should clients using the library gate every function call to
Notmuch behind a lock instead? I can also start a discussion in
sfsexp's issue tracker.
Kevin Boulain (1):
lib: replace some uses of Query::MatchAll with thread-safe alternative
Likely missed in 86cbd215e, when notmuch_query_search_messages_st was
renamed to notmuch_query_search_messages.
---
lib/notmuch.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/notmuch.h b/lib/notmuch.h
index ce375c04..73e246e0 100644
--- a/lib/notmuch.h
+++
On 2023-02-21 at 07:59 -04, David Bremner wrote:
> Thanks for tracking this down. I did try some experiments a while ago
> with multi-threaded indexing (which is where the locking you mentioned
> arose), but overall it isn't well tested (by me).
To be honest I struggled a bit, notably on GLib
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
#include
#include
void
33 matches
Mail list logo
