Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
rec: Fix a race reported by TSAN around the secpoll status
We used to store the security polling status in a regular integer, and the status can be updated from the `rec/web+stat` thread while being read from a Rust-based web-server thread, which is correctly reported by TSAN as a data race: ``` WARNING: ThreadSanitizer: data race (pid=2006) Write of size 4 at 0x55f19579db54 by thread T5: #0 doSecPoll(long*, std::shared_ptr<Logr::Logger> const&) /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/secpoll-recursor.cc:84:23 (pdns_recursor+0x814e3c) #1 SyncRes::doResolveAt(std::unordered_map<DNSName, std::pair<std::vector<ComboAddress, std::allocator<ComboAddress> >, bool>, std::hash<DNSName>, std::equal_to<DNSName>, std::allocator<std::pair<DNSName const, std::pair<std::vector<ComboAddress, std::allocator<ComboAddress> >, bool> > > >&, DNSName, bool, DNSName const&, QType, std::vector<DNSRecord, std::allocator<DNSRecord> >&, unsigned int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::set<SyncRes::GetBestNSAnswer, std::less<SyncRes::GetBestNSAnswer>, std::allocator<SyncRes::GetBestNSAnswer> >&, SyncRes::Context&, SyncRes::StopAtDelegation*, std::map<DNSName, std::vector<ComboAddress, std::allocator<ComboAddress> >, std::less<DNSName>, std::allocator<std::pair<DNSName const, std::vector<ComboAddress, std::allocator<ComboAddress> > > > >*) /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/syncres.cc:6040:25 (pdns_recursor+0x84be60) #2 SyncRes::doResolveNoQNameMinimization(DNSName const&, QType, std::vector<DNSRecord, std::allocator<DNSRecord> >&, unsigned int, std::set<SyncRes::GetBestNSAnswer, std::less<SyncRes::GetBestNSAnswer>, std::allocator<SyncRes::GetBestNSAnswer> >&, SyncRes::Context&, bool*, SyncRes::StopAtDelegation*) /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/syncres.cc:2099:11 (pdns_recursor+0x838903) #3 SyncRes::doResolve(DNSName const&, QType, std::vector<DNSRecord, std::allocator<DNSRecord> >&, unsigned int, std::set<SyncRes::GetBestNSAnswer, std::less<SyncRes::GetBestNSAnswer>, std::allocator<SyncRes::GetBestNSAnswer> >&, SyncRes::Context&) /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/syncres.cc:1835:13 (pdns_recursor+0x825337) #4 SyncRes::beginResolve(DNSName const&, QType, QClass, std::vector<DNSRecord, std::allocator<DNSRecord> >&, unsigned int) /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/syncres.cc:797:13 (pdns_recursor+0x828974) #5 doSecPoll(long*, std::shared_ptr<Logr::Logger> const&) /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/secpoll-recursor.cc:55:22 (pdns_recursor+0x814039) #6 houseKeepingWork(std::shared_ptr<Logr::Logger> const&)::$_18::operator()() const /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/rec-main.cc:2651:9 (pdns_recursor+0x68abb3) #7 void std::__invoke_impl<void, houseKeepingWork(std::shared_ptr<Logr::Logger> const&)::$_18&>(std::__invoke_other, houseKeepingWork(std::shared_ptr<Logr::Logger> const&)::$_18&) /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/invoke.h:61:14 (pdns_recursor+0x68abb3) #8 std::enable_if<is_invocable_r_v<void, houseKeepingWork(std::shared_ptr<Logr::Logger> const&)::$_18&>, void>::type std::__invoke_r<void, houseKeepingWork(std::shared_ptr<Logr::Logger> const&)::$_18&>(houseKeepingWork(std::shared_ptr<Logr::Logger> const&)::$_18&) /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/invoke.h:111:2 (pdns_recursor+0x68abb3) #9 std::_Function_handler<void (), houseKeepingWork(std::shared_ptr<Logr::Logger> const&)::$_18>::_M_invoke(std::_Any_data const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/std_function.h:290:9 (pdns_recursor+0x68abb3) #10 std::function<void ()>::operator()() const /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/std_function.h:591:9 (pdns_recursor+0x688561) #11 PeriodicTask::runIfDue(timeval&, std::function<void ()> const&) /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/rec-main.cc:2490:7 (pdns_recursor+0x688561) #12 houseKeepingWork(std::shared_ptr<Logr::Logger> const&) /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/rec-main.cc:2649:17 (pdns_recursor+0x688561) #13 houseKeeping(void*) /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/rec-main.cc:2715:5 (pdns_recursor+0x688561) #14 MTasker<std::shared_ptr<PacketID>, std::vector<unsigned char, noinit_adaptor<std::allocator<unsigned char> > >, PacketIDCompare>::makeThread(void (*)(void*), void*)::'lambda'()::operator()() const /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/./mtasker.hh:397:5 (pdns_recursor+0x5f04b5) #15 void std::__invoke_impl<void, MTasker<std::shared_ptr<PacketID>, std::vector<unsigned char, noinit_adaptor<std::allocator<unsigned char> > >, PacketIDCompare>::makeThread(void (*)(void*), void*)::'lambda'()&>(std::__invoke_other, MTasker<std::shared_ptr<PacketID>, std::vector<unsigned char, noinit_adaptor<std::allocator<unsigned char> > >, PacketIDCompare>::makeThread(void (*)(void*), void*)::'lambda'()&) /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/invoke.h:61:14 (pdns_recursor+0x5f0191) PowerDNS#16 std::enable_if<is_invocable_r_v<void, MTasker<std::shared_ptr<PacketID>, std::vector<unsigned char, noinit_adaptor<std::allocator<unsigned char> > >, PacketIDCompare>::makeThread(void (*)(void*), void*)::'lambda'()&>, void>::type std::__invoke_r<void, MTasker<std::shared_ptr<PacketID>, std::vector<unsigned char, noinit_adaptor<std::allocator<unsigned char> > >, PacketIDCompare>::makeThread(void (*)(void*), void*)::'lambda'()&>(MTasker<std::shared_ptr<PacketID>, std::vector<unsigned char, noinit_adaptor<std::allocator<unsigned char> > >, PacketIDCompare>::makeThread(void (*)(void*), void*)::'lambda'()&) /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/invoke.h:111:2 (pdns_recursor+0x5f0191) PowerDNS#17 std::_Function_handler<void (), MTasker<std::shared_ptr<PacketID>, std::vector<unsigned char, noinit_adaptor<std::allocator<unsigned char> > >, PacketIDCompare>::makeThread(void (*)(void*), void*)::'lambda'()>::_M_invoke(std::_Any_data const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/std_function.h:290:9 (pdns_recursor+0x5f0191) PowerDNS#18 std::function<void ()>::operator()() const /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/std_function.h:591:9 (pdns_recursor+0x591fdf) PowerDNS#19 threadWrapper(boost::context::detail::transfer_t) /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/mtasker_context.cc:163:7 (pdns_recursor+0x591fdf) PowerDNS#20 MTasker<std::shared_ptr<PacketID>, std::vector<unsigned char, noinit_adaptor<std::allocator<unsigned char> > >, PacketIDCompare>::schedule(timeval const&) /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/./mtasker.hh:426:7 (pdns_recursor+0x6ade0b) PowerDNS#21 recLoop() /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/rec-main.cc:2764:29 (pdns_recursor+0x666577) PowerDNS#22 recursorThread() /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/rec-main.cc:2959:5 (pdns_recursor+0x666577) PowerDNS#23 recLoop() /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/rec-main.cc:2773:24 (pdns_recursor+0x66664d) PowerDNS#24 recursorThread() /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/rec-main.cc:2959:5 (pdns_recursor+0x66664d) PowerDNS#25 recLoop() /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/rec-main.cc:2806:26 (pdns_recursor+0x668385) PowerDNS#26 recursorThread() /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/rec-main.cc:2959:5 (pdns_recursor+0x668385) PowerDNS#27 RecThreadInfo::start(unsigned int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::map<unsigned int, std::set<int, std::less<int>, std::allocator<int> >, std::less<unsigned int>, std::allocator<std::pair<unsigned int const, std::set<int, std::less<int>, std::allocator<int> > > > > const&, std::shared_ptr<Logr::Logger> const&)::$_0::operator()() const /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/rec-main.cc:241:5 (pdns_recursor+0x69caf7) PowerDNS#28 void std::__invoke_impl<void, RecThreadInfo::start(unsigned int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::map<unsigned int, std::set<int, std::less<int>, std::allocator<int> >, std::less<unsigned int>, std::allocator<std::pair<unsigned int const, std::set<int, std::less<int>, std::allocator<int> > > > > const&, std::shared_ptr<Logr::Logger> const&)::$_0>(std::__invoke_other, RecThreadInfo::start(unsigned int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::map<unsigned int, std::set<int, std::less<int>, std::allocator<int> >, std::less<unsigned int>, std::allocator<std::pair<unsigned int const, std::set<int, std::less<int>, std::allocator<int> > > > > const&, std::shared_ptr<Logr::Logger> const&)::$_0&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/invoke.h:61:14 (pdns_recursor+0x69caf7) PowerDNS#29 std::__invoke_result<RecThreadInfo::start(unsigned int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::map<unsigned int, std::set<int, std::less<int>, std::allocator<int> >, std::less<unsigned int>, std::allocator<std::pair<unsigned int const, std::set<int, std::less<int>, std::allocator<int> > > > > const&, std::shared_ptr<Logr::Logger> const&)::$_0>::type std::__invoke<RecThreadInfo::start(unsigned int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::map<unsigned int, std::set<int, std::less<int>, std::allocator<int> >, std::less<unsigned int>, std::allocator<std::pair<unsigned int const, std::set<int, std::less<int>, std::allocator<int> > > > > const&, std::shared_ptr<Logr::Logger> const&)::$_0>(RecThreadInfo::start(unsigned int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::map<unsigned int, std::set<int, std::less<int>, std::allocator<int> >, std::less<unsigned int>, std::allocator<std::pair<unsigned int const, std::set<int, std::less<int>, std::allocator<int> > > > > const&, std::shared_ptr<Logr::Logger> const&)::$_0&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/invoke.h:96:14 (pdns_recursor+0x69caf7) PowerDNS#30 void std::thread::_Invoker<std::tuple<RecThreadInfo::start(unsigned int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::map<unsigned int, std::set<int, std::less<int>, std::allocator<int> >, std::less<unsigned int>, std::allocator<std::pair<unsigned int const, std::set<int, std::less<int>, std::allocator<int> > > > > const&, std::shared_ptr<Logr::Logger> const&)::$_0> >::_M_invoke<0ul>(std::_Index_tuple<0ul>) /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/std_thread.h:252:13 (pdns_recursor+0x69caf7) PowerDNS#31 std::thread::_Invoker<std::tuple<RecThreadInfo::start(unsigned int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::map<unsigned int, std::set<int, std::less<int>, std::allocator<int> >, std::less<unsigned int>, std::allocator<std::pair<unsigned int const, std::set<int, std::less<int>, std::allocator<int> > > > > const&, std::shared_ptr<Logr::Logger> const&)::$_0> >::operator()() /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/std_thread.h:259:11 (pdns_recursor+0x69caf7) PowerDNS#32 std::thread::_State_impl<std::thread::_Invoker<std::tuple<RecThreadInfo::start(unsigned int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::map<unsigned int, std::set<int, std::less<int>, std::allocator<int> >, std::less<unsigned int>, std::allocator<std::pair<unsigned int const, std::set<int, std::less<int>, std::allocator<int> > > > > const&, std::shared_ptr<Logr::Logger> const&)::$_0> > >::_M_run() /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/std_thread.h:210:13 (pdns_recursor+0x69caf7) PowerDNS#33 <null> <null> (libstdc++.so.6+0xd44a2) Previous read of size 4 at 0x55f19579db54 by thread T6: #0 getAllStatsMap[abi:cxx11](StatComponent) /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/rec_channel_rec.cc:231:101 (pdns_recursor+0x760f2e) #1 productServerStatisticsFetch(std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >&) /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/ws-recursor.cc:55:16 (pdns_recursor+0x8fcd70) #2 apiServerStatistics(HttpRequest*, HttpResponse*) /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/ws-api.cc:189:3 (pdns_recursor+0x8f5f71) #3 void std::__invoke_impl<void, void (*&)(HttpRequest*, HttpResponse*), HttpRequest*, HttpResponse*>(std::__invoke_other, void (*&)(HttpRequest*, HttpResponse*), HttpRequest*&&, HttpResponse*&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/invoke.h:61:14 (pdns_recursor+0x918cc8) #4 std::enable_if<is_invocable_r_v<void, void (*&)(HttpRequest*, HttpResponse*), HttpRequest*, HttpResponse*>, void>::type std::__invoke_r<void, void (*&)(HttpRequest*, HttpResponse*), HttpRequest*, HttpResponse*>(void (*&)(HttpRequest*, HttpResponse*), HttpRequest*&&, HttpResponse*&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/invoke.h:111:2 (pdns_recursor+0x918cc8) #5 std::_Function_handler<void (HttpRequest*, HttpResponse*), void (*)(HttpRequest*, HttpResponse*)>::_M_invoke(std::_Any_data const&, HttpRequest*&&, HttpResponse*&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/std_function.h:290:9 (pdns_recursor+0x918cc8) #6 std::function<void (HttpRequest*, HttpResponse*)>::operator()(HttpRequest*, HttpResponse*) const /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/std_function.h:591:9 (pdns_recursor+0x8ff173) #7 rustWrapper(std::function<void (HttpRequest*, HttpResponse*)> const&, pdns::rust::web::rec::Request const&, pdns::rust::web::rec::Response&) /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/ws-recursor.cc:1067:5 (pdns_recursor+0x8ff173) #8 pdns::rust::web::rec::apiServerStatistics(pdns::rust::web::rec::Request const&, pdns::rust::web::rec::Response&) /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/ws-recursor.cc:1102:1 (pdns_recursor+0x907b80) #9 pdns$rust$web$rec$cxxbridge1$apiServerStatistics <null> (pdns_recursor+0x9984b2) Location is global 'g_security_status' of size 4 at 0x55f19579db54 (pdns_recursor+0x000004479b54) Thread T5 'rec/web+stat' (tid=2012, running) created by main thread at: #0 pthread_create <null> (pdns_recursor+0x1fe42d) #1 std::thread::_M_start_thread(std::unique_ptr<std::thread::_State, std::default_delete<std::thread::_State> >, void (*)()) <null> (libstdc++.so.6+0xd4578) #2 RecThreadInfo::runThreads(std::shared_ptr<Logr::Logger> const&) /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/rec-main.cc:355:10 (pdns_recursor+0x663382) #3 serviceMain(std::shared_ptr<Logr::Logger> const&) /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/rec-main.cc:2402:9 (pdns_recursor+0x67fdaf) #4 main /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/rec-main.cc:3316:11 (pdns_recursor+0x678755) Thread T6 (tid=2013, running) created by main thread at: #0 pthread_create <null> (pdns_recursor+0x1fe42d) #1 std::sys::pal::unix::thread::Thread::new::he1793c71df66b318 /rustc/e71f9a9a98b0faf423844bf0ba7438f29dc27d58/library/std/src/sys/pal/unix/thread.rs:84:19 (pdns_recursor+0xc1ef11) #2 RecThreadInfo::runThreads(std::shared_ptr<Logr::Logger> const&) /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/rec-main.cc:358:7 (pdns_recursor+0x663417) #3 serviceMain(std::shared_ptr<Logr::Logger> const&) /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/rec-main.cc:2402:9 (pdns_recursor+0x67fdaf) #4 main /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/rec-main.cc:3316:11 (pdns_recursor+0x678755) SUMMARY: ThreadSanitizer: data race /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/secpoll-recursor.cc:84:23 in doSecPoll(long*, std::shared_ptr<Logr::Logger> const&) ``` This commit switches to an atomic type to store the security polling status and clarify that the security polling message is not actually shared outside of the security polling function. It appears that the security polling status was the last metric stored in a `uint32_t` type so we can get rid of some now unused code in the process.
- Loading branch information