Review the CCheckQueue logic for Thread Sanitizer issues

[stevenroose]

Some tests are failing using the Thread Sanitizer build that point in the direction of our custom CCheck queue that is generic over signature and rangeproof checks.

There doesn't seem to have been a change to this logic since 0.14, so it's not a new issue, it's just that upstream introduced the thread sanitizer build that wasn't there before.

[jhfrontz]

This seems to sometimes succeed and sometimes fail in travis. For example, running with a setting of BOOST_TEST_RANDOM=1542431049 induces the failure:

make[3]: *** [test/checkqueue_tests.cpp.test] Error 1
make[3]: *** Waiting for unfinished jobs....
Makefile:13603: recipe for target 'test/checkqueue_tests.cpp.test' failed
make[3]: Leaving directory '/home/travis/build/jhfrontz/elements/build/elements-x86_64-unknown-linux-gnu/src'
Makefile:13308: recipe for target 'check-am' failed
make[2]: Leaving directory '/home/travis/build/jhfrontz/elements/build/elements-x86_64-unknown-linux-gnu/src'
make[2]: *** [check-am] Error 2
Makefile:12996: recipe for target 'check-recursive' failed
make[1]: *** [check-recursive] Error 1
make[1]: Leaving directory '/home/travis/build/jhfrontz/elements/build/elements-x86_64-unknown-linux-gnu/src'
Makefile:783: recipe for target 'check-recursive' failed
make: *** [check-recursive] Error 1
==================
WARNING: ThreadSanitizer: heap-use-after-free (pid=16596)
  Write of size 8 at 0x7d04001831b0 by thread T17:
    #0 operator delete(void*) <null> (test_bitcoin+0x000000508069)
    #1 CCheckQueue<checkqueue_tests::FakeCheckCheckCompletion>::Loop(bool) /home/travis/build/jhfrontz/elements/build/elements-x86_64-unknown-linux-gnu/src/./checkqueue.h:120 (test_bitcoin+0x0000005d5726)
    #2 CCheckQueue<checkqueue_tests::FakeCheckCheckCompletion>::Thread() /home/travis/build/jhfrontz/elements/build/elements-x86_64-unknown-linux-gnu/src/./checkqueue.h:136 (test_bitcoin+0x0000005d3102)
    #3 operator() /home/travis/build/jhfrontz/elements/build/elements-x86_64-unknown-linux-gnu/src/test/checkqueue_tests.cpp:154 (test_bitcoin+0x0000005d3102)
    #4 boost::detail::thread_data<checkqueue_tests::Correct_Queue_range(std::vector<unsigned long, std::allocator<unsigned long> >)::$_11>::run() /usr/include/boost/thread/detail/thread.hpp:116 (test_bitcoin+0x0000005d3102)
    #5 boost::this_thread::interruption_point() <null> (libboost_thread.so.1.58.0+0x0000000115d4)

  Previous write of size 8 at 0x7d04001831b0 by thread T9:
    #0 operator delete(void*) <null> (test_bitcoin+0x000000508069)
    #1 CCheckQueue<checkqueue_tests::FakeCheckCheckCompletion>::Loop(bool) /home/travis/build/jhfrontz/elements/build/elements-x86_64-unknown-linux-gnu/src/./checkqueue.h:120 (test_bitcoin+0x0000005d5726)
    #2 CCheckQueue<checkqueue_tests::FakeCheckCheckCompletion>::Thread() /home/travis/build/jhfrontz/elements/build/elements-x86_64-unknown-linux-gnu/src/./checkqueue.h:136 (test_bitcoin+0x0000005d3102)
    #3 operator() /home/travis/build/jhfrontz/elements/build/elements-x86_64-unknown-linux-gnu/src/test/checkqueue_tests.cpp:154 (test_bitcoin+0x0000005d3102)
    #4 boost::detail::thread_data<checkqueue_tests::Correct_Queue_range(std::vector<unsigned long, std::allocator<unsigned long> >)::$_11>::run() /usr/include/boost/thread/detail/thread.hpp:116 (test_bitcoin+0x0000005d3102)
    #5 boost::this_thread::interruption_point() <null> (libboost_thread.so.1.58.0+0x0000000115d4)

  Thread T17 (tid=16848, running) created by main thread at:
    #0 pthread_create <null> (test_bitcoin+0x000000488086)
    #1 boost::thread::start_thread_noexcept() <null> (libboost_thread.so.1.58.0+0x0000000102e8)
    #2 thread<(lambda at test/checkqueue_tests.cpp:154:25) &> /usr/include/boost/thread/detail/thread.hpp:266 (test_bitcoin+0x0000005cc81c)
    #3 boost::thread* boost::thread_group::create_thread<checkqueue_tests::Correct_Queue_range(std::vector<unsigned long, std::allocator<unsigned long> >)::$_11>(checkqueue_tests::Correct_Queue_range(std::vector<unsigned long, std::allocator<unsigned long> >)::$_11) /usr/include/boost/thread/detail/thread_group.hpp:78 (test_bitcoin+0x0000005cc81c)
    #4 checkqueue_tests::Correct_Queue_range(std::vector<unsigned long, std::allocator<unsigned long> >) /home/travis/build/jhfrontz/elements/build/elements-x86_64-unknown-linux-gnu/src/test/checkqueue_tests.cpp:154 (test_bitcoin+0x0000005cc81c)
    #5 checkqueue_tests::test_CheckQueue_Correct_Random::test_method() /home/travis/build/jhfrontz/elements/build/elements-x86_64-unknown-linux-gnu/src/test/checkqueue_tests.cpp:212 (test_bitcoin+0x0000005cd9e9)
    #6 checkqueue_tests::test_CheckQueue_Correct_Random_invoker() /home/travis/build/jhfrontz/elements/build/elements-x86_64-unknown-linux-gnu/src/test/checkqueue_tests.cpp:206 (test_bitcoin+0x0000005cd7d0)
    #7 boost::unit_test::ut_detail::unused boost::unit_test::ut_detail::invoker<boost::unit_test::ut_detail::unused>::invoke<void (*)()>(void (*&)()) /usr/include/boost/test/utils/callback.hpp:56 (test_bitcoin+0x0000005291b9)
    #8 boost::unit_test::ut_detail::callback0_impl_t<boost::unit_test::ut_detail::unused, void (*)()>::invoke() /usr/include/boost/test/utils/callback.hpp:89 (test_bitcoin+0x0000005291b9)
    #9 boost::unit_test::test_case_filter::test_case_filter(boost::unit_test::basic_cstring<char const>) <null> (libboost_unit_test_framework.so.1.58.0+0x00000006acb0)
    #10 __libc_start_main <null> (libc.so.6+0x00000002082f)

  Thread T9 (tid=16847, running) created by main thread at:
    #0 pthread_create <null> (test_bitcoin+0x000000488086)
    #1 boost::thread::start_thread_noexcept() <null> (libboost_thread.so.1.58.0+0x0000000102e8)
    #2 thread<(lambda at test/checkqueue_tests.cpp:154:25) &> /usr/include/boost/thread/detail/thread.hpp:266 (test_bitcoin+0x0000005cc81c)
    #3 boost::thread* boost::thread_group::create_thread<checkqueue_tests::Correct_Queue_range(std::vector<unsigned long, std::allocator<unsigned long> >)::$_11>(checkqueue_tests::Correct_Queue_range(std::vector<unsigned long, std::allocator<unsigned long> >)::$_11) /usr/include/boost/thread/detail/thread_group.hpp:78 (test_bitcoin+0x0000005cc81c)
    #4 checkqueue_tests::Correct_Queue_range(std::vector<unsigned long, std::allocator<unsigned long> >) /home/travis/build/jhfrontz/elements/build/elements-x86_64-unknown-linux-gnu/src/test/checkqueue_tests.cpp:154 (test_bitcoin+0x0000005cc81c)
    #5 checkqueue_tests::test_CheckQueue_Correct_Random::test_method() /home/travis/build/jhfrontz/elements/build/elements-x86_64-unknown-linux-gnu/src/test/checkqueue_tests.cpp:212 (test_bitcoin+0x0000005cd9e9)
    #6 checkqueue_tests::test_CheckQueue_Correct_Random_invoker() /home/travis/build/jhfrontz/elements/build/elements-x86_64-unknown-linux-gnu/src/test/checkqueue_tests.cpp:206 (test_bitcoin+0x0000005cd7d0)
    #7 boost::unit_test::ut_detail::unused boost::unit_test::ut_detail::invoker<boost::unit_test::ut_detail::unused>::invoke<void (*)()>(void (*&)()) /usr/include/boost/test/utils/callback.hpp:56 (test_bitcoin+0x0000005291b9)
    #8 boost::unit_test::ut_detail::callback0_impl_t<boost::unit_test::ut_detail::unused, void (*)()>::invoke() /usr/include/boost/test/utils/callback.hpp:89 (test_bitcoin+0x0000005291b9)
    #9 boost::unit_test::test_case_filter::test_case_filter(boost::unit_test::basic_cstring<char const>) <null> (libboost_unit_test_framework.so.1.58.0+0x00000006acb0)
    #10 __libc_start_main <null> (libc.so.6+0x00000002082f)

SUMMARY: ThreadSanitizer: heap-use-after-free (/home/travis/build/jhfrontz/elements/build/elements-x86_64-unknown-linux-gnu/src/test/test_bitcoin+0x508069) in operator delete(void*)
==================
ThreadSanitizer: reported 1 warnings

But running with BOOST_TEST_RANDOM=1542517932 results in a successful run (i.e., Done. Your build exited with 0.). I think this difference is possibly because of the different test mixes that are keyed to the random seed.

I've tried (unsuccessfully) to reproduce this failure on local systems (Centos 7.x, Arch, Ubuntu 16.04, Ubuntu 16.08, as well as within a docker-based gitian instance of bionic) using version 7 of the clang SDK and the following script (which I think mimics the testing done in travis with the thread sanitizer):

#! /bin/bash
set -x
set -e
export MAKEJOBS=-j9
export RUN_UNIT_TESTS=true
export RUN_FUNCTIONAL_TESTS=true
export RUN_FUZZ_TESTS=false
export BOOST_TEST_RANDOM=1542431049
export TSAN_OPTIONS="suppressions=/home/jhf/elements/test/sanitizer_suppressions/tsan:log_path=/home/jhf/elements/sanitizer-output/tsan"
./autogen.sh
$PWD/configure --disable-dependency-tracking --enable-zmq --disable-wallet --with-gui=qt5 CPPFLAGS=-DDEBUG_LOCKORDER --with-sanitizers=thread --disable-hardening --disable-asm CC=clang-7 CXX=clang++-7 LD=lld-7   --prefix=`pwd`/depends/x86_64-pc-linux-gnu
make clean
make ${MAKEJOBS} VERBOSE=1
make ${MAKEJOBS} install VERBOSE=1
make ${MAKEJOBS} check VERBOSE=1
./src/test/test_bitcoin --run_test=checkqueue_tests
cd src
make ${MAKEJOBS} check-TESTS check-local VERBOSE=1

Examining (and instrumenting) the code identified in the thread sanitizer output reveals no obvious errors. However, it did reveal that the same blocks of memory were being repeatedly allocated/freed (albeit in proper order) over the course of the test. My suspicion is that there is something in the travis/docker environment that confuses the sanitizer.