Re: [DRAFT 1/1] lib: replace some uses of Query::MatchAll with a thread-safe alternative

Subject: Re: [DRAFT 1/1] lib: replace some uses of Query::MatchAll with a thread-safe alternative

Date: Sun, 26 Feb 2023 08:18:10 -0400

To: Kevin Boulain, notmuch@notmuchmail.org

Cc: Kevin Boulain

From: David Bremner


Kevin Boulain <kevin@boula.in> writes:

> 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 <null> (libtsan.so.2+0x62506)
>   +    #1 void std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char*>(char*, char*, std::forward_iterator_tag) [clone .isra.0] <null> (libxapian.so.30+0x872b3)
>   +
>   +  Previous write of size 8 at 0x7b10000001a0 by thread T1:
>   +    #0 operator new(unsigned long) <null> (libtsan.so.2+0x8ba83)
>   +    #1 Xapian::Query::Query(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, unsigned int, unsigned int) <null> (libxapian.so.30+0x855cd)
>   ...

I don't know what the difference between our environments is, but these
tests are failing in glib for me.

      +==================
	+WARNING: ThreadSanitizer: data race (pid=392122)
	+  Atomic read of size 1 at 0x7b10000025c0 by thread T2:
	+    #0 pthread_rwlock_rdlock ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:1406 (libtsan.so.2+0x447f1)
	+    #1 g_rw_lock_reader_lock <null> (libglib-2.0.so.0+0xa8b84)
	+
	+  Previous write of size 8 at 0x7b10000025c0 by thread T1:
	+    #0 malloc ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:647 (libtsan.so.2+0x3ebb8)
	+    #1 <null> <null> (libglib-2.0.so.0+0xa880a)
	+
        ...

FWIW I have libglib2.0-0 version 2.74.5-1 on Debian.

> +printf "C compiler supports address sanitizer... "
s/address/thread/

> +test_cmdline="${CC} ${CFLAGS} ${CPPFLAGS} -fsanitize=thread minimal.c ${LDFLAGS} -o minimal"
> +if ${test_cmdline} >/dev/null 2>&1 && ./minimal

> @@ -186,7 +186,7 @@ _notmuch_query_string_to_xapian_query (notmuch_database_t *notmuch,
>  {
>      try {
>  	if (query_string == "" || query_string == "*") {
> -	    output = Xapian::Query::MatchAll;
> +	    output = Xapian::Query(std::string());

I think this probably deserves a brief comment like "use thread-safe
alternative to Query::MatchAll.

That is why I was thinking a preprocessor macro / inline-function might
be appropriate, to only need that comment in one place.

> +test_begin_subtest "create"
> +test_C ${MAIL_DIR} ${MAIL_DIR}-2 <<EOF
> +#include <assert.h>
> +#include <notmuch.h>
> +#include <pthread.h>
> +#include <stdio.h>

I guess we will find out if any changes are needed to test harness to
portably support pthreads.  BSD and macOS are the usual places that have
slightly different expectations with respect to include files and
libraries; I don't know if that applies here.

d
_______________________________________________
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-leave@notmuchmail.org

Thread: