ThreadSanitizer: data race on vptr (ctor/dtor vs virtual call) in BaseIndex

fanquake commented at 6:11 pm on June 13, 2022: member

https://cirrus-ci.com/task/6564394053140480?logs=ci#L3875:

 0WARNING: ThreadSanitizer: data race on vptr (ctor/dtor vs virtual call) (pid=24158)
 1  Write of size 8 at 0x7ffe0efae9f8 by main thread:
 2    [#0](/bitcoin-bitcoin/0/) BaseIndex::~BaseIndex() src/index/base.cpp:53:1 (test_bitcoin+0xcc6b69)
 3    [#1](/bitcoin-bitcoin/1/) CoinStatsIndex::~CoinStatsIndex() src/./index/coinstatsindex.h:17:7 (test_bitcoin+0x3b9b21)
 4    [#2](/bitcoin-bitcoin/2/) coinstatsindex_tests::coinstatsindex_initial_sync::test_method() src/test/coinstatsindex_tests.cpp:84:1 (test_bitcoin+0x3b9b21)
 5    [#3](/bitcoin-bitcoin/3/) coinstatsindex_tests::coinstatsindex_initial_sync_invoker() src/test/coinstatsindex_tests.cpp:32:1 (test_bitcoin+0x3b814b)
 6    [#4](/bitcoin-bitcoin/4/) boost::detail::function::void_function_invoker0<void (*)(), void>::invoke(boost::detail::function::function_buffer&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:117:11 (test_bitcoin+0x2bbf1d)
 7    [#5](/bitcoin-bitcoin/5/) boost::function0<void>::operator()() const /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:763:14 (test_bitcoin+0x220877)
 8    [#6](/bitcoin-bitcoin/6/) boost::detail::forward::operator()() /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1388:32 (test_bitcoin+0x220877)
 9    [#7](/bitcoin-bitcoin/7/) boost::detail::function::function_obj_invoker0<boost::detail::forward, int>::invoke(boost::detail::function::function_buffer&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:137:18 (test_bitcoin+0x220877)
10    [#8](/bitcoin-bitcoin/8/) boost::function0<int>::operator()() const /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:763:14 (test_bitcoin+0x1ae59e)
11    [#9](/bitcoin-bitcoin/9/) int boost::detail::do_invoke<boost::shared_ptr<boost::detail::translator_holder_base>, boost::function<int ()> >(boost::shared_ptr<boost::detail::translator_holder_base> const&, boost::function<int ()> const&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:301:30 (test_bitcoin+0x1ae59e)
12    [#10](/bitcoin-bitcoin/10/) boost::execution_monitor::catch_signals(boost::function<int ()> const&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:903:16 (test_bitcoin+0x1ae59e)
13    [#11](/bitcoin-bitcoin/11/) boost::execution_monitor::execute(boost::function<int ()> const&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1301:16 (test_bitcoin+0x1ae8c0)
14    [#12](/bitcoin-bitcoin/12/) boost::execution_monitor::vexecute(boost::function<void ()> const&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1397:5 (test_bitcoin+0x1aa21b)
15    [#13](/bitcoin-bitcoin/13/) boost::unit_test::unit_test_monitor_t::execute_and_translate(boost::function<void ()> const&, unsigned long) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_monitor.ipp:49:9 (test_bitcoin+0x1aa21b)
16    [#14](/bitcoin-bitcoin/14/) boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:815:44 (test_bitcoin+0x1ddb63)
17    [#15](/bitcoin-bitcoin/15/) boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:784:58 (test_bitcoin+0x1de1d8)
18    [#16](/bitcoin-bitcoin/16/) boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:784:58 (test_bitcoin+0x1de1d8)
19    [#17](/bitcoin-bitcoin/17/) boost::unit_test::framework::run(unsigned long, bool) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:1721:29 (test_bitcoin+0x1a8e66)
20    [#18](/bitcoin-bitcoin/18/) boost::unit_test::unit_test_main(boost::unit_test::test_suite* (*)(int, char**), int, char**) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_main.ipp:250:9 (test_bitcoin+0x1c19c6)
21    [#19](/bitcoin-bitcoin/19/) main /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_main.ipp:306:12 (test_bitcoin+0x1c1ff6)
22  Previous read of size 8 at 0x7ffe0efae9f8 by thread T1 (mutexes: write M603):
23    [#0](/bitcoin-bitcoin/0/) BaseIndex::SetBestBlockIndex(CBlockIndex const*)::$_1::operator()() const src/index/base.cpp:388:9 (test_bitcoin+0xcc74e6)
24    [#1](/bitcoin-bitcoin/1/) BaseIndex::SetBestBlockIndex(CBlockIndex const*) src/index/base.cpp:388:9 (test_bitcoin+0xcc74e6)
25    [#2](/bitcoin-bitcoin/2/) BaseIndex::BlockConnected(std::__1::shared_ptr<CBlock const> const&, CBlockIndex const*) src/index/base.cpp:273:9 (test_bitcoin+0xcc9759)
26    [#3](/bitcoin-bitcoin/3/) CMainSignals::BlockConnected(std::__1::shared_ptr<CBlock const> const&, CBlockIndex const*)::$_8::operator()() const::'lambda'(CValidationInterface&)::operator()(CValidationInterface&) const src/validationinterface.cpp:225:79 (test_bitcoin+0x10223a4)
27    [#4](/bitcoin-bitcoin/4/) void MainSignalsImpl::Iterate<CMainSignals::BlockConnected(std::__1::shared_ptr<CBlock const> const&, CBlockIndex const*)::$_8::operator()() const::'lambda'(CValidationInterface&)>(CMainSignals::BlockConnected(std::__1::shared_ptr<CBlock const> const&, CBlockIndex const*)::$_8::operator()() const::'lambda'(CValidationInterface&)&&) src/validationinterface.cpp:86:17 (test_bitcoin+0x10223a4)
28    [#5](/bitcoin-bitcoin/5/) CMainSignals::BlockConnected(std::__1::shared_ptr<CBlock const> const&, CBlockIndex const*)::$_8::operator()() const src/validationinterface.cpp:225:22 (test_bitcoin+0x10223a4)
29    [#6](/bitcoin-bitcoin/6/) CMainSignals::BlockConnected(std::__1::shared_ptr<CBlock const> const&, CBlockIndex const*)::$_9::operator()() const src/validationinterface.cpp:227:5 (test_bitcoin+0x10223a4)
30    [#7](/bitcoin-bitcoin/7/) decltype(static_cast<CMainSignals::BlockConnected(std::__1::shared_ptr<CBlock const> const&, CBlockIndex const*)::$_9&>(fp)()) std::__1::__invoke<CMainSignals::BlockConnected(std::__1::shared_ptr<CBlock const> const&, CBlockIndex const*)::$_9&>(CMainSignals::BlockConnected(std::__1::shared_ptr<CBlock const> const&, CBlockIndex const*)::$_9&) /usr/lib/llvm-13/bin/../include/c++/v1/type_traits:3918:1 (test_bitcoin+0x10223a4)
31    [#8](/bitcoin-bitcoin/8/) void std::__1::__invoke_void_return_wrapper<void, true>::__call<CMainSignals::BlockConnected(std::__1::shared_ptr<CBlock const> const&, CBlockIndex const*)::$_9&>(CMainSignals::BlockConnected(std::__1::shared_ptr<CBlock const> const&, CBlockIndex const*)::$_9&) /usr/lib/llvm-13/bin/../include/c++/v1/__functional/invoke.h:61:9 (test_bitcoin+0x10223a4)
32    [#9](/bitcoin-bitcoin/9/) std::__1::__function::__alloc_func<CMainSignals::BlockConnected(std::__1::shared_ptr<CBlock const> const&, CBlockIndex const*)::$_9, std::__1::allocator<CMainSignals::BlockConnected(std::__1::shared_ptr<CBlock const> const&, CBlockIndex const*)::$_9>, void ()>::operator()() /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:171:16 (test_bitcoin+0x10223a4)
33    [#10](/bitcoin-bitcoin/10/) std::__1::__function::__func<CMainSignals::BlockConnected(std::__1::shared_ptr<CBlock const> const&, CBlockIndex const*)::$_9, std::__1::allocator<CMainSignals::BlockConnected(std::__1::shared_ptr<CBlock const> const&, CBlockIndex const*)::$_9>, void ()>::operator()() /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:345:12 (test_bitcoin+0x10223a4)
34    [#11](/bitcoin-bitcoin/11/) std::__1::__function::__value_func<void ()>::operator()() const /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:498:16 (test_bitcoin+0x10b6b71)
35    [#12](/bitcoin-bitcoin/12/) std::__1::function<void ()>::operator()() const /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:1175:12 (test_bitcoin+0x10b6b71)
36    [#13](/bitcoin-bitcoin/13/) SingleThreadedSchedulerClient::ProcessQueue() src/scheduler.cpp:175:5 (test_bitcoin+0x10b6b71)
37    [#14](/bitcoin-bitcoin/14/) SingleThreadedSchedulerClient::MaybeScheduleProcessQueue()::$_1::operator()() const src/scheduler.cpp:144:41 (test_bitcoin+0x10b8875)
38    [#15](/bitcoin-bitcoin/15/) decltype(static_cast<SingleThreadedSchedulerClient::MaybeScheduleProcessQueue()::$_1&>(fp)()) std::__1::__invoke<SingleThreadedSchedulerClient::MaybeScheduleProcessQueue()::$_1&>(SingleThreadedSchedulerClient::MaybeScheduleProcessQueue()::$_1&) /usr/lib/llvm-13/bin/../include/c++/v1/type_traits:3918:1 (test_bitcoin+0x10b8875)
39    [#16](/bitcoin-bitcoin/16/) void std::__1::__invoke_void_return_wrapper<void, true>::__call<SingleThreadedSchedulerClient::MaybeScheduleProcessQueue()::$_1&>(SingleThreadedSchedulerClient::MaybeScheduleProcessQueue()::$_1&) /usr/lib/llvm-13/bin/../include/c++/v1/__functional/invoke.h:61:9 (test_bitcoin+0x10b8875)
40    [#17](/bitcoin-bitcoin/17/) std::__1::__function::__alloc_func<SingleThreadedSchedulerClient::MaybeScheduleProcessQueue()::$_1, std::__1::allocator<SingleThreadedSchedulerClient::MaybeScheduleProcessQueue()::$_1>, void ()>::operator()() /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:171:16 (test_bitcoin+0x10b8875)
41    [#18](/bitcoin-bitcoin/18/) std::__1::__function::__func<SingleThreadedSchedulerClient::MaybeScheduleProcessQueue()::$_1, std::__1::allocator<SingleThreadedSchedulerClient::MaybeScheduleProcessQueue()::$_1>, void ()>::operator()() /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:345:12 (test_bitcoin+0x10b8875)
42    [#19](/bitcoin-bitcoin/19/) std::__1::__function::__value_func<void ()>::operator()() const /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:498:16 (test_bitcoin+0x10b5b5c)
43    [#20](/bitcoin-bitcoin/20/) std::__1::function<void ()>::operator()() const /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:1175:12 (test_bitcoin+0x10b5b5c)
44    [#21](/bitcoin-bitcoin/21/) CScheduler::serviceQueue() src/scheduler.cpp:62:17 (test_bitcoin+0x10b5b5c)
45    [#22](/bitcoin-bitcoin/22/) ChainTestingSetup::ChainTestingSetup(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::vector<char const*, std::__1::allocator<char const*> > const&)::$_0::operator()() const src/test/util/setup_common.cpp:160:110 (test_bitcoin+0xa4e7b8)
46    [#23](/bitcoin-bitcoin/23/) decltype(static_cast<ChainTestingSetup::ChainTestingSetup(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::vector<char const*, std::__1::allocator<char const*> > const&)::$_0&>(fp)()) std::__1::__invoke<ChainTestingSetup::ChainTestingSetup(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::vector<char const*, std::__1::allocator<char const*> > const&)::$_0&>(ChainTestingSetup::ChainTestingSetup(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::vector<char const*, std::__1::allocator<char const*> > const&)::$_0&) /usr/lib/llvm-13/bin/../include/c++/v1/type_traits:3918:1 (test_bitcoin+0xa4e7b8)
47    [#24](/bitcoin-bitcoin/24/) void std::__1::__invoke_void_return_wrapper<void, true>::__call<ChainTestingSetup::ChainTestingSetup(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::vector<char const*, std::__1::allocator<char const*> > const&)::$_0&>(ChainTestingSetup::ChainTestingSetup(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::vector<char const*, std::__1::allocator<char const*> > const&)::$_0&) /usr/lib/llvm-13/bin/../include/c++/v1/__functional/invoke.h:61:9 (test_bitcoin+0xa4e7b8)
48    [#25](/bitcoin-bitcoin/25/) std::__1::__function::__alloc_func<ChainTestingSetup::ChainTestingSetup(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::vector<char const*, std::__1::allocator<char const*> > const&)::$_0, std::__1::allocator<ChainTestingSetup::ChainTestingSetup(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::vector<char const*, std::__1::allocator<char const*> > const&)::$_0>, void ()>::operator()() /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:171:16 (test_bitcoin+0xa4e7b8)
49    [#26](/bitcoin-bitcoin/26/) std::__1::__function::__func<ChainTestingSetup::ChainTestingSetup(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::vector<char const*, std::__1::allocator<char const*> > const&)::$_0, std::__1::allocator<ChainTestingSetup::ChainTestingSetup(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::vector<char const*, std::__1::allocator<char const*> > const&)::$_0>, void ()>::operator()() /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:345:12 (test_bitcoin+0xa4e7b8)
50    [#27](/bitcoin-bitcoin/27/) std::__1::__function::__value_func<void ()>::operator()() const /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:498:16 (test_bitcoin+0x115760f)
51    [#28](/bitcoin-bitcoin/28/) std::__1::function<void ()>::operator()() const /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:1175:12 (test_bitcoin+0x115760f)
52    [#29](/bitcoin-bitcoin/29/) util::TraceThread(char const*, std::__1::function<void ()>) src/util/thread.cpp:18:9 (test_bitcoin+0x115760f)
53    [#30](/bitcoin-bitcoin/30/) decltype(static_cast<void (*>(fp)(static_cast<char const*>(fp0), static_cast<ChainTestingSetup::ChainTestingSetup(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::vector<char const*, std::__1::allocator<char const*> > const&)::$_0>(fp0))) std::__1::__invoke<void (*)(char const*, std::__1::function<void ()>), char const*, ChainTestingSetup::ChainTestingSetup(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::vector<char const*, std::__1::allocator<char const*> > const&)::$_0>(void (*&&)(char const*, std::__1::function<void ()>), char const*&&, ChainTestingSetup::ChainTestingSetup(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::vector<char const*, std::__1::allocator<char const*> > const&)::$_0&&) /usr/lib/llvm-13/bin/../include/c++/v1/type_traits:3918:1 (test_bitcoin+0xa4e3b1)
54    [#31](/bitcoin-bitcoin/31/) void std::__1::__thread_execute<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (*)(char const*, std::__1::function<void ()>), char const*, ChainTestingSetup::ChainTestingSetup(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::vector<char const*, std::__1::allocator<char const*> > const&)::$_0, 2ul, 3ul>(std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (*)(char const*, std::__1::function<void ()>), char const*, ChainTestingSetup::ChainTestingSetup(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::vector<char const*, std::__1::allocator<char const*> > const&)::$_0>&, std::__1::__tuple_indices<2ul, 3ul>) /usr/lib/llvm-13/bin/../include/c++/v1/thread:280:5 (test_bitcoin+0xa4e3b1)
55    [#32](/bitcoin-bitcoin/32/) void* std::__1::__thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (*)(char const*, std::__1::function<void ()>), char const*, ChainTestingSetup::ChainTestingSetup(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::vector<char const*, std::__1::allocator<char const*> > const&)::$_0> >(void*) /usr/lib/llvm-13/bin/../include/c++/v1/thread:291:5 (test_bitcoin+0xa4e3b1)
56  Location is stack of main thread.
57  Location is global '??' at 0x7ffe0ef91000 ([stack]+0x00000001d9f8)
58  Mutex M603 (0x558df2c934a0) created at:
59    [#0](/bitcoin-bitcoin/0/) pthread_mutex_init <null> (test_bitcoin+0x11cf6f)
60    [#1](/bitcoin-bitcoin/1/) std::__1::recursive_mutex::recursive_mutex() <null> (libc++.so.1+0x49fb3)
61    [#2](/bitcoin-bitcoin/2/) __libc_start_main <null> (libc.so.6+0x29eba)
62  Thread T1 'b-scheduler' (tid=24216, running) created by main thread at:
63    [#0](/bitcoin-bitcoin/0/) pthread_create <null> (test_bitcoin+0x11b7fd)
64    [#1](/bitcoin-bitcoin/1/) std::__1::__libcpp_thread_create(unsigned long*, void* (*)(void*), void*) /usr/lib/llvm-13/bin/../include/c++/v1/__threading_support:443:10 (test_bitcoin+0xa47a76)
65    [#2](/bitcoin-bitcoin/2/) std::__1::thread::thread<void (&)(char const*, std::__1::function<void ()>), char const (&) [10], ChainTestingSetup::ChainTestingSetup(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::vector<char const*, std::__1::allocator<char const*> > const&)::$_0, void>(void (&)(char const*, std::__1::function<void ()>), char const (&) [10], ChainTestingSetup::ChainTestingSetup(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::vector<char const*, std::__1::allocator<char const*> > const&)::$_0&&) /usr/lib/llvm-13/bin/../include/c++/v1/thread:307:16 (test_bitcoin+0xa47a76)
66    [#3](/bitcoin-bitcoin/3/) ChainTestingSetup::ChainTestingSetup(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::vector<char const*, std::__1::allocator<char const*> > const&) src/test/util/setup_common.cpp:160:42 (test_bitcoin+0xa47a76)
67    [#4](/bitcoin-bitcoin/4/) TestingSetup::TestingSetup(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::vector<char const*, std::__1::allocator<char const*> > const&) src/test/util/setup_common.cpp:198:7 (test_bitcoin+0xa47ed9)
68    [#5](/bitcoin-bitcoin/5/) TestChain100Setup::TestChain100Setup(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::vector<char const*, std::__1::allocator<char const*> > const&) src/test/util/setup_common.cpp:246:7 (test_bitcoin+0xa48be3)
69    [#6](/bitcoin-bitcoin/6/) coinstatsindex_tests::coinstatsindex_initial_sync::coinstatsindex_initial_sync() src/test/coinstatsindex_tests.cpp:32:1 (test_bitcoin+0x3b7c8b)
70    [#7](/bitcoin-bitcoin/7/) coinstatsindex_tests::coinstatsindex_initial_sync_invoker() src/test/coinstatsindex_tests.cpp:32:1 (test_bitcoin+0x3b7c8b)
71    [#8](/bitcoin-bitcoin/8/) boost::detail::function::void_function_invoker0<void (*)(), void>::invoke(boost::detail::function::function_buffer&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:117:11 (test_bitcoin+0x2bbf1d)
72    [#9](/bitcoin-bitcoin/9/) boost::function0<void>::operator()() const /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:763:14 (test_bitcoin+0x220877)
73    [#10](/bitcoin-bitcoin/10/) boost::detail::forward::operator()() /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1388:32 (test_bitcoin+0x220877)
74    [#11](/bitcoin-bitcoin/11/) boost::detail::function::function_obj_invoker0<boost::detail::forward, int>::invoke(boost::detail::function::function_buffer&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:137:18 (test_bitcoin+0x220877)
75    [#12](/bitcoin-bitcoin/12/) boost::function0<int>::operator()() const /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:763:14 (test_bitcoin+0x1ae59e)
76    [#13](/bitcoin-bitcoin/13/) int boost::detail::do_invoke<boost::shared_ptr<boost::detail::translator_holder_base>, boost::function<int ()> >(boost::shared_ptr<boost::detail::translator_holder_base> const&, boost::function<int ()> const&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:301:30 (test_bitcoin+0x1ae59e)
77    [#14](/bitcoin-bitcoin/14/) boost::execution_monitor::catch_signals(boost::function<int ()> const&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:903:16 (test_bitcoin+0x1ae59e)
78    [#15](/bitcoin-bitcoin/15/) boost::execution_monitor::execute(boost::function<int ()> const&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1301:16 (test_bitcoin+0x1ae8c0)
79    [#16](/bitcoin-bitcoin/16/) boost::execution_monitor::vexecute(boost::function<void ()> const&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1397:5 (test_bitcoin+0x1aa21b)
80    [#17](/bitcoin-bitcoin/17/) boost::unit_test::unit_test_monitor_t::execute_and_translate(boost::function<void ()> const&, unsigned long) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_monitor.ipp:49:9 (test_bitcoin+0x1aa21b)
81    [#18](/bitcoin-bitcoin/18/) boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:815:44 (test_bitcoin+0x1ddb63)
82    [#19](/bitcoin-bitcoin/19/) boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:784:58 (test_bitcoin+0x1de1d8)
83    [#20](/bitcoin-bitcoin/20/) boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:784:58 (test_bitcoin+0x1de1d8)
84    [#21](/bitcoin-bitcoin/21/) boost::unit_test::framework::run(unsigned long, bool) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:1721:29 (test_bitcoin+0x1a8e66)
85    [#22](/bitcoin-bitcoin/22/) boost::unit_test::unit_test_main(boost::unit_test::test_suite* (*)(int, char**), int, char**) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_main.ipp:250:9 (test_bitcoin+0x1c19c6)
86    [#23](/bitcoin-bitcoin/23/) main /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_main.ipp:306:12 (test_bitcoin+0x1c1ff6)
87SUMMARY: ThreadSanitizer: data race on vptr (ctor/dtor vs virtual call) src/index/base.cpp:53:1 in BaseIndex::~BaseIndex()
88==================
89Exit status: 2

jonatack commented at 6:34 pm on June 13, 2022: contributor

Thanks, I’ve been looking into the design of these virtual classes (in #24150 and revisiting it more deeply lately), will have a look.

vasild commented at 11:40 am on September 16, 2022: contributor

This should fix it:

 0diff --git i/src/test/coinstatsindex_tests.cpp w/src/test/coinstatsindex_tests.cpp
 1index 2a6a777cfe..b78cac8908 100644
 2--- i/src/test/coinstatsindex_tests.cpp
 3+++ w/src/test/coinstatsindex_tests.cpp
 4@@ -3,12 +3,13 @@
 5 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 6 
 7 #include <chainparams.h>
 8 #include <index/coinstatsindex.h>
 9 #include <interfaces/chain.h>
10 #include <kernel/coinstats.h>
11+#include <scheduler.h>
12 #include <test/util/setup_common.h>
13 #include <test/util/validation.h>
14 #include <util/time.h>
15 #include <validation.h>
16 
17 #include <boost/test/unit_test.hpp>
18@@ -76,12 +77,16 @@ BOOST_FIXTURE_TEST_CASE(coinstatsindex_initial_sync, TestChain100Setup)
19 
20     BOOST_CHECK(block_index != new_block_index);
21 
22     // Shutdown sequence (c.f. Shutdown() in init.cpp)
23     coin_stats_index.Stop();
24 
25+    // Stop the scheduler which may be using coin_stats_index (how?) before
26+    // coin_stats_index gets destroyed at the end of this function.
27+    m_node.scheduler->stop();
28+
29     // Rest of shutdown sequence and destructors happen in ~TestingSetup()
30 }
31 
32 // Test shutdown between BlockConnected and ChainStateFlushed notifications,
33 // make sure index is not corrupted and is able to reload.
34 BOOST_FIXTURE_TEST_CASE(coinstatsindex_unclean_shutdown, TestChain100Setup)

Though I do not understand how the scheduler picked the local variable coin_stats_index from the test.

maflcko commented at 12:26 pm on September 16, 2022: member

Oh good catch. Though, I don’t think your patch fixes it. Stopping the scheduler will have no effect on the queue. The event will still be triggered later on (when the queue is drained) and cause the segfault. You’d have to flush/sync the queue before destructing the index. See also init.cpp:

0    // After there are no more peers/RPC left to give us new data which may generate
1    // CValidationInterface callbacks, flush them...
2    GetMainSignals().FlushBackgroundCallbacks();
3
4    // Stop and delete all indexes only after flushing background callbacks.
5...

Though I do not understand how the scheduler picked the local variable coin_stats_index from the test.

The pointer is passed over in BaseIndex::Start:

0    RegisterValidationInterface(this);

maflcko added the label Tests on Sep 16, 2022

vasild commented at 2:36 pm on September 16, 2022: contributor

It would be good to reproduce this deterministically and then confirm that the possible fix actually fixes it. I tried to reproduce it by planting a few sleeps here and there, but in vain.

maflcko commented at 2:45 pm on September 16, 2022: member

Same here, I couldn’t reproduce. My next best guess would be that this is a race in the validationinterface, where the subscriber is unsubscribed at the same time that the block connected event is executed.

maflcko commented at 3:07 pm on September 16, 2022: member

For reference, my latest attempt was to add a sleep right after the best block was updated to unblock the test, but keep the event executing (hopefully while it the subscriber is removed). However, this didn’t work so far:

(I’ve also moved the memory into a unique_ptr to make it easier for sanitizers to find use-after-free)

 0diff --git a/src/index/base.cpp b/src/index/base.cpp
 1index 88c2ce98fa..348f203d63 100644
 2--- a/src/index/base.cpp
 3+++ b/src/index/base.cpp
 4@@ -415,6 +415,7 @@ void BaseIndex::SetBestBlockIndex(const CBlockIndex* block) {
 5     assert(!node::fPruneMode || AllowPrune());
 6 
 7     m_best_block_index = block;
 8+    UninterruptibleSleep(90ms);
 9     if (AllowPrune() && block) {
10         node::PruneLockInfo prune_lock;
11         prune_lock.height_first = block->nHeight;
12diff --git a/src/test/coinstatsindex_tests.cpp b/src/test/coinstatsindex_tests.cpp
13index 2a6a777cfe..4bfb62a1c5 100644
14--- a/src/test/coinstatsindex_tests.cpp
15+++ b/src/test/coinstatsindex_tests.cpp
16@@ -30,7 +30,8 @@ static void IndexWaitSynced(BaseIndex& index)
17 
18 BOOST_FIXTURE_TEST_CASE(coinstatsindex_initial_sync, TestChain100Setup)
19 {
20-    CoinStatsIndex coin_stats_index{interfaces::MakeChain(m_node), 1 << 20, true};
21+    auto coin_stats_index_p{std::make_unique<CoinStatsIndex>(interfaces::MakeChain(m_node), 1 << 20,true)};
22+    auto&coin_stats_index{*coin_stats_index_p};
23 
24     const CBlockIndex* block_index;
25     {
26@@ -89,7 +90,8 @@ BOOST_FIXTURE_TEST_CASE(coinstatsindex_unclean_shutdown, TestChain100Setup)
27     Chainstate& chainstate = Assert(m_node.chainman)->ActiveChainstate();
28     const CChainParams& params = Params();
29     {
30-        CoinStatsIndex index{interfaces::MakeChain(m_node), 1 << 20};
31+    auto coin_stats_index_p{std::make_unique<CoinStatsIndex>(interfaces::MakeChain(m_node), 1 << 20)};
32+    auto&index{*coin_stats_index_p};
33         BOOST_REQUIRE(index.Start());
34         IndexWaitSynced(index);
35         std::shared_ptr<const CBlock> new_block;

vasild commented at 1:40 pm on September 19, 2022: contributor

I stared at this and I don’t see how the scheduler is still using the local index variable after it is destroyed, as I suggested above. I am looking at the code as of b02411d2ddf8c010fc4e9d9ec27cae3a73e895a9 where the problem actually happened (in case it is already nonexistent in master).

The call to coin_stats_index.BlockUntilSyncedToCurrentChain() from the test will wait for that BaseIndex::SetBestBlockIndex() (where you added the sleep) to complete in the b-scheduler thread before continuing with the test.

Could this be a false alarm by the thread sanitizer? What is being reported is that a read from BaseIndex::SetBestBlockIndex() (the b-scheduler thread) was followed by a write from the destructor (main thread) without a syncronization, but maybe the sanitizer cannot grasp the logic from BlockUntilSyncedToCurrentChain()?

ryanofsky commented at 2:59 pm on September 19, 2022: contributor

IIUC, I would think calling FlushBackgroundCallbacks then m_node.scheduler->stop() right before the coin_stats_index destructor runs should fix this if just calling FlushBackgroundCallbacks by itself is not sufficient.

I stared at this and I don’t see how the scheduler is still using the local index variable after it is destroyed

The problem isn’t that the scheduler is reading the coin_stats_index vtable pointer after it is written by the destructor. The problem is that the scheduler is reading the vtable pointer before it is wriitten by the destructor, and the thread sanitizer can’t find any synchronization calls that order these read/write events, so it warns that there is probably a race condition.

The earlier suggestion to add the m_node.scheduler->stop() would probably silence the thread sanitizer because it joins the scheduler thread, forcing the any reads to happen before the write.

vasild commented at 3:57 pm on September 19, 2022: contributor

Yes! I actually wrote two things that contradict each other: “I don’t see how the scheduler is still using the local index variable after it is destroyed” and “What is being reported is that a read … was followed by a write from the destructor”.

So, this is probably a false alarm due to the thread sanitizer not being able to grasp our synchronization logic: SyncWithValidationInterfaceQueue() which should be called from BaseIndex::BlockUntilSyncedToCurrentChain().

ryanofsky commented at 5:18 pm on September 19, 2022: contributor

So, this is probably a false alarm due to the thread sanitizer not being able to grasp our synchronization logic: SyncWithValidationInterfaceQueue() which should be called from BaseIndex::BlockUntilSyncedToCurrentChain().

Maybe BlockUntilSyncedToCurrentChain is returning true without calling SyncWithValidationInterfaceQueue()? That would explain why TSAN doesn’t know about the synchronization between the test thread write and scheduler thread read. TSAN needs to assume scheduler thread might do another read because it has no way of knowing there will not be moreBlockConnected events in the future. In this case, I would expect putting an explicit SyncWithValidationInterfaceQueue call before the destructor to fix the problem.

Longer term more robust solution is probably to switch indexes from RegisterValidationInterface to RegisterSharedValidationInterface so notification thread can interact with the index through a shared pointer instead of a raw pointer, and destroy the index itself. #24230 actually already does this.

maflcko commented at 10:13 am on September 20, 2022: member

In this case, I would expect putting an explicit SyncWithValidationInterfaceQueue call before the destructor to fix the problem.

I don’t think tsan is smart enough to figure out synchronization primitives that are more complex than a std::atomic or mutex on the exact memory that is read/written.

ryanofsky commented at 12:01 pm on September 20, 2022: contributor

I don’t think tsan is smart enough to figure out synchronization primitives that are more complex than a std::atomic or mutex on the exact memory that is read/written.

I’m not sure, but I thought it might do this because I looked at the TSAN paper yesterday to get an idea of how it works: https://static.googleusercontent.com/media/research.google.com/en//pubs/archive/35604.pdf

Figure 1 seems to show that it also detects signal/waits between threads in addition to mutex locking. Adding a SyncWithValidationInterfaceQueue call would make the test thread wait for the scheduler thread to signal it, so could fix the problem. Adding a scheduler stop would make the test thread join the scheduler thread, which might also fix it if adding sync doesn’t work.

maflcko commented at 7:52 am on September 21, 2022: member

Steps to reproduce:

 0diff --git a/src/index/base.cpp b/src/index/base.cpp
 1index 88c2ce98fa..afa13c099b 100644
 2--- a/src/index/base.cpp
 3+++ b/src/index/base.cpp
 4@@ -353,6 +353,8 @@ bool BaseIndex::BlockUntilSyncedToCurrentChain() const
 5         return false;
 6     }
 7 
 8+    UninterruptibleSleep(10ms);
 9+
10     {
11         // Skip the queue-draining stuff if we know we're caught up with
12         // m_chain.Tip().
13@@ -420,4 +422,6 @@ void BaseIndex::SetBestBlockIndex(const CBlockIndex* block) {
14         prune_lock.height_first = block->nHeight;
15         WITH_LOCK(::cs_main, m_chainstate->m_blockman.UpdatePruneLock(GetName(), prune_lock));
16     }
17+    UninterruptibleSleep(90ms);
18+    std::cout << "a" << GetName() <<"a"<< std::endl;
19 }

valgrind:

 0==1043784== Memcheck, a memory error detector
 1==1043784== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
 2==1043784== Using Valgrind-3.19.0 and LibVEX; rerun with -h for copyright info
 3==1043784== Command: ./src/test/test_bitcoin -t coinstatsindex_tests
 4==1043784== 
 5Running 2 test cases...
 6acoinstatsindexa
 7acoinstatsindexa
 8acoinstatsindexa
 9==1043784== Thread 2 b-scheduler:
10==1043784== Invalid read of size 8
11==1043784==    at 0xAB05A6: _M_data (basic_string.h:234)
12==1043784==    by 0xAB05A6: data (basic_string.h:2568)
13==1043784==    by 0xAB05A6: operator<<<char, std::char_traits<char>, std::allocator<char> > (basic_string.h:3888)
14==1043784==    by 0xAB05A6: BaseIndex::SetBestBlockIndex(CBlockIndex const*) (base.cpp:426)

maflcko added this to the milestone 24.0 on Sep 21, 2022

maflcko added the label Bug on Sep 21, 2022

vasild referenced this in commit eaec74f3b9 on Sep 27, 2022

vasild commented at 4:21 pm on September 27, 2022: contributor

I can reproduce with #25365 (comment).

Using just

0@@ -420,4 +421,6 @@ void BaseIndex::SetBestBlockIndex(const CBlockIndex* block) {
1         prune_lock.height_first = block->nHeight;
2+        UninterruptibleSleep(90ms);
3         WITH_LOCK(::cs_main, m_chainstate->m_blockman.UpdatePruneLock(GetName(), prune_lock));
4     }

instead of

0@@ -420,4 +422,6 @@ void BaseIndex::SetBestBlockIndex(const CBlockIndex* block) {
1         prune_lock.height_first = block->nHeight;
2         WITH_LOCK(::cs_main, m_chainstate->m_blockman.UpdatePruneLock(GetName(), prune_lock));
3     }
4+    UninterruptibleSleep(90ms);
5+    std::cout << "a" << GetName() <<"a"<< std::endl;
6 }

also suffices to reproduce.

Changing the local variable to a dynamically allocated one with new and calling delete at the end of the test makes the valgrind error more obvious:

Invalid read … Address x is … inside a block … free’d our delete call … Block was alloc’d at our new call

Thank you, @MarcoFalke, @ryanofsky!

Fixed in https://github.com/bitcoin/bitcoin/pull/26188

vasild referenced this in commit 40ef8e064d on Sep 29, 2022

vasild referenced this in commit 3ae1fe4498 on Sep 29, 2022

ryanofsky referenced this in commit dd2ef55a86 on Sep 30, 2022

vasild referenced this in commit d2395c83c4 on Oct 4, 2022

ryanofsky referenced this in commit 8891949bdc on Oct 5, 2022

fanquake referenced this in commit 4175c332b9 on Oct 10, 2022

vasild referenced this in commit b3e76f92e0 on Oct 10, 2022

maflcko removed this from the milestone 24.0 on Oct 10, 2022

sidhujag referenced this in commit f4572f9ae4 on Oct 10, 2022

fanquake referenced this in commit 5ad82a09b4 on Oct 11, 2022

vasild referenced this in commit 6526dc3b78 on Oct 11, 2022

fanquake closed this on Oct 13, 2022

fanquake referenced this in commit 422efcad36 on Oct 13, 2022

adam2k referenced this in commit c8fca8a784 on Oct 19, 2022

adam2k referenced this in commit f892179899 on Oct 19, 2022

sidhujag referenced this in commit 945a820485 on Oct 23, 2022

janus referenced this in commit f7bf2d1a4a on Jan 20, 2023

janus referenced this in commit 6be697d2c4 on Jan 20, 2023

aguycalled referenced this in commit 2f61f6b4bb on Jan 25, 2023

bitcoin locked this on Oct 13, 2023

ThreadSanitizer: data race on vptr (ctor/dtor vs virtual call) in BaseIndex #25365