rec: Fix a race reported by TSAN around the secpoll status #15196

rgacogne · 2025-02-21T09:34:42Z

Short description

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)
      #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)
      #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)
      #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)
      #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)
      #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)
      #21 recLoop() /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/rec-main.cc:2764:29 (pdns_recursor+0x666577)
      #22 recursorThread() /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/rec-main.cc:2959:5 (pdns_recursor+0x666577)
      #23 recLoop() /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/rec-main.cc:2773:24 (pdns_recursor+0x66664d)
      #24 recursorThread() /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/rec-main.cc:2959:5 (pdns_recursor+0x66664d)
      #25 recLoop() /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/rec-main.cc:2806:26 (pdns_recursor+0x668385)
      #26 recursorThread() /__w/pdns/pdns/pdns/recursordist/pdns-recursor-0.0.0-git1/rec-main.cc:2959:5 (pdns_recursor+0x668385)
      #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)
      #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)
      #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)
      #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)
      #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)
      #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)
      #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.

Checklist

I have:

read the CONTRIBUTING.md document
compiled this code
tested this code
included documentation (including possible behaviour changes)
documented the code
added or modified regression test(s)
added or modified unit test(s)

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.

miodvallat

Looking good.

pdns/recursordist/metrics_table.py

coveralls · 2025-02-21T10:06:39Z

Pull Request Test Coverage Report for Build 13454354691

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

0 of 8 (0.0%) changed or added relevant lines in 2 files are covered.
6519 unchanged lines in 79 files lost coverage.
Overall coverage decreased (-0.02%) to 64.49%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pdns/recursordist/rec_channel_rec.cc	0	4	0.0%
pdns/recursordist/secpoll-recursor.cc	0	4	0.0%

Files with Coverage Reduction	New Missed Lines	%
modules/gpgsqlbackend/gpgsqlbackend.cc	1	88.62%
pdns/recursordist/rec-rust-lib/rust/src/bridge.hh	1	0.0%
pdns/recursordist/sortlist.hh	1	75.0%
ext/probds/murmur3.cc	2	94.12%
pdns/backends/gsql/gsqlbackend.hh	2	97.71%
pdns/distributor.hh	2	51.86%
pdns/dnsname.hh	2	91.24%
pdns/dnstap.cc	2	70.97%
pdns/rcpgenerator.cc	3	90.37%
pdns/recursordist/rec-tcpout.cc	3	58.73%

Totals
Change from base Build 13451963662:	-0.02%
Covered Lines:	127615
Relevant Lines:	166896

💛 - Coveralls

rgacogne · 2025-02-21T10:39:58Z

Weird failure in the recursor regression tests:

  =================================== FAILURES ===================================
  _______________________ FWCatzXFRRecursorTest.testFWCatz _______________________
  
  self = <test_FWCatz.FWCatzXFRRecursorTest testMethod=testFWCatz>
  
      def testFWCatz(self):
          # Fresh catz does not need a notify
          self.waitForTCPSocket("127.0.0.1", self._wsPort)
          # first zone
          self.waitUntilCorrectSerialIsLoaded(1)
  >       self.checkForwards({'forward_zones': [
              {'zone': 'a.', 'forwarders': ['1.2.3.4']}
          ]})
  
  test_FWCatz.py:318: 
  _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
  
  self = <test_FWCatz.FWCatzXFRRecursorTest testMethod=testFWCatz>
  expected = {'forward_zones': [{'forwarders': ['1.2.3.4'], 'zone': 'a.'}]}
  
      def checkForwards(self, expected):
  >       with open('configs/' + self._confdir + '/catzone.forward.catz.') as file:
  E       FileNotFoundError: [Errno 2] No such file or directory: 'configs/FWCatzXFRRecursor/catzone.forward.catz.'
  
  test_FWCatz.py:290: FileNotFoundError

omoerbeek · 2025-02-21T10:41:34Z

test_FWCatz.py:290: FileNotFoundError

I'll check. Maybe there's a race in the test setup?

omoerbeek · 2025-02-21T10:49:00Z

It looks like a race indeed. waitUntilCorrectSerialIsLoaded checks the test axfr server running in the pytest driver process. After this one agrees to continue, it does not mean the recursor has updated it's copy and written it to storage, as this can take time. This is the first time I actually see this race. Will fix separately, for now try a rerun.

rgacogne added rec defect labels Feb 21, 2025

rgacogne requested a review from omoerbeek February 21, 2025 09:34

miodvallat approved these changes Feb 21, 2025

View reviewed changes

pdns/recursordist/metrics_table.py Show resolved Hide resolved

omoerbeek approved these changes Feb 21, 2025

View reviewed changes

rec: Fix clang-tidy's warning

c5fc0af

rgacogne merged commit 725f809 into PowerDNS:master Feb 21, 2025
83 checks passed

rgacogne deleted the rec-fix-secpoll-tsan branch February 21, 2025 11:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rec: Fix a race reported by TSAN around the secpoll status #15196

rec: Fix a race reported by TSAN around the secpoll status #15196

rgacogne commented Feb 21, 2025

miodvallat left a comment

coveralls commented Feb 21, 2025 •

edited

Loading

rgacogne commented Feb 21, 2025

omoerbeek commented Feb 21, 2025

omoerbeek commented Feb 21, 2025

rec: Fix a race reported by TSAN around the secpoll status #15196

rec: Fix a race reported by TSAN around the secpoll status #15196

Conversation

rgacogne commented Feb 21, 2025

Short description

Checklist

miodvallat left a comment

Choose a reason for hiding this comment

coveralls commented Feb 21, 2025 • edited Loading

Pull Request Test Coverage Report for Build 13454354691

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

rgacogne commented Feb 21, 2025

omoerbeek commented Feb 21, 2025

omoerbeek commented Feb 21, 2025

coveralls commented Feb 21, 2025 •

edited

Loading