Skip to content

Use atomic enabled flag in request_aggregator::request#5022

Merged
pwojcikdev merged 1 commit intonanocurrency:developfrom
pwojcikdev:fix-req-aggr-shutdown
Feb 2, 2026
Merged

Use atomic enabled flag in request_aggregator::request#5022
pwojcikdev merged 1 commit intonanocurrency:developfrom
pwojcikdev:fix-req-aggr-shutdown

Conversation

@pwojcikdev
Copy link
Copy Markdown
Contributor

@pwojcikdev pwojcikdev commented Feb 2, 2026

Should fix:

WARNING: ThreadSanitizer: data race (pid=57940)
    Read of size 8 at 0x000114b008e0 by thread T1708:
      #0 std::__1::vector<std::__1::thread, std::__1::allocator<std::__1::thread>>::empty[abi:ue170006]() const vector:610 (rpc_test:arm64+0x10058ccb4)
      #1 nano::request_aggregator::request(std::__1::vector<std::__1::pair<nano::block_hash, nano::root>, std::__1::allocator<std::__1::pair<nano::block_hash, nano::root>>> const&, std::__1::shared_ptr<nano::transport::channel> const&) request_aggregator.cpp:97 (rpc_test:arm64+0x100bf8c14)
      #2 (anonymous namespace)::process_visitor::confirm_req(nano::messages::confirm_req const&) message_processor.cpp:212 (rpc_test:arm64+0x100a5601c)
      #3 nano::messages::confirm_req::visit(nano::messages::message_visitor&) const confirm.cpp:55 (rpc_test:arm64+0x10117957c)
      #4 nano::message_processor::process(nano::messages::message const&, std::__1::shared_ptr<nano::transport::channel> const&) message_processor.cpp:297 (rpc_test:arm64+0x100a4a170)
      #5 nano::node::inbound(nano::messages::message const&, std::__1::shared_ptr<nano::transport::channel> const&) node.cpp:489 (rpc_test:arm64+0x100aec338)
      #6 nano::transport::loopback_channel::send_impl(nano::messages::message const&, nano::transport::traffic_type, std::__1::function<void (boost::system::error_code const&, unsigned long)>) loopback.cpp:20 (rpc_test:arm64+0x100d2f188)
      #7 nano::transport::channel::send(nano::messages::message const&, nano::transport::traffic_type, std::__1::function<void (boost::system::error_code const&, unsigned long)>) channel.cpp:19 (rpc_test:arm64+0x100cd7688)
      #8 nano::confirmation_solicitor::flush() confirmation_solicitor.cpp:109 (rpc_test:arm64+0x1006a81e0)
      #9 nano::active_elections::tick_elections(std::__1::unique_lock<nano::mutex>&) active_elections.cpp:529 (rpc_test:arm64+0x1003f3330)
      #10 nano::active_elections::run() active_elections.cpp:549 (rpc_test:arm64+0x1003f3ba8)
      #11 nano::active_elections::start()::$_3::operator()() const active_elections.cpp:116 (rpc_test:arm64+0x1004371f0)
      #12 decltype(std::declval<nano::active_elections::start()::$_3>()()) std::__1::__invoke[abi:ue170006]<nano::active_elections::start()::$_3>(nano::active_elections::start()::$_3&&) invoke.h:340 (rpc_test:arm64+0x10043712c)
      #13 void std::__1::__thread_execute[abi:ue170006]<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, nano::active_elections::start()::$_3>(std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, nano::active_elections::start()::$_3>&, std::__1::__tuple_indices<>) thread.h:227 (rpc_test:arm64+0x1004370d8)
      #14 void* std::__1::__thread_proxy[abi:ue170006]<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, nano::active_elections::start()::$_3>>(void*) thread.h:238 (rpc_test:arm64+0x100436b6c)
  
    Previous write of size 8 at 0x000114b008e0 by thread T1740:
      #0 std::__1::vector<std::__1::thread, std::__1::allocator<std::__1::thread>>::__base_destruct_at_end[abi:ue170006](std::__1::thread*) vector:945 (rpc_test:arm64+0x100598130)
      #1 std::__1::vector<std::__1::thread, std::__1::allocator<std::__1::thread>>::__clear[abi:ue170006]() vector:938 (rpc_test:arm64+0x100597e70)
      #2 std::__1::vector<std::__1::thread, std::__1::allocator<std::__1::thread>>::clear[abi:ue170006]() vector:723 (rpc_test:arm64+0x10058d328)
      #3 nano::request_aggregator::stop() request_aggregator.cpp:76 (rpc_test:arm64+0x100bf87e0)
      #4 nano::node::stop() node.cpp:618 (rpc_test:arm64+0x100aeb980)
      #5 nano::test::system::stop_node(nano::node&)::$_2::operator()() const system.cpp:211 (rpc_test:arm64+0x100395a44)
      #6 decltype(std::declval<nano::test::system::stop_node(nano::node&)::$_2>()()) std::__1::__invoke[abi:ue170006]<nano::test::system::stop_node(nano::node&)::$_2>(nano::test::system::stop_node(nano::node&)::$_2&&) invoke.h:340 (rpc_test:arm64+0x100395998)
      #7 void std::__1::__async_func<nano::test::system::stop_node(nano::node&)::$_2>::__execute[abi:ue170006]<>(std::__1::__tuple_indices<>) future:2195 (rpc_test:arm64+0x100395944)
      #8 std::__1::__async_func<nano::test::system::stop_node(nano::node&)::$_2>::operator()[abi:ue170006]() future:2188 (rpc_test:arm64+0x1003958ec)
      #9 std::__1::__async_assoc_state<void, std::__1::__async_func<nano::test::system::stop_node(nano::node&)::$_2>>::__execute() future:1020 (rpc_test:arm64+0x10039565c)
      #10 decltype(*std::declval<std::__1::__async_assoc_state<void, std::__1::__async_func<nano::test::system::stop_node(nano::node&)::$_2>>*>().*std::declval<void (std::__1::__async_assoc_state<void, std::__1::__async_func<nano::test::system::stop_node(nano::node&)::$_2>>::*)()>()()) std::__1::__invoke[abi:ue170006]<void (std::__1::__async_assoc_state<void, std::__1::__async_func<nano::test::system::stop_node(nano::node&)::$_2>>::*)(), std::__1::__async_assoc_state<void, std::__1::__async_func<nano::test::system::stop_node(nano::node&)::$_2>>*, void>(void (std::__1::__async_assoc_state<void, std::__1::__async_func<nano::test::system::stop_node(nano::node&)::$_2>>::*&&)(), std::__1::__async_assoc_state<void, std::__1::__async_func<nano::test::system::stop_node(nano::node&)::$_2>>*&&) invoke.h:308 (rpc_test:arm64+0x1003966f4)
      #11 void std::__1::__thread_execute[abi:ue170006]<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, void (std::__1::__async_assoc_state<void, std::__1::__async_func<nano::test::system::stop_node(nano::node&)::$_2>>::*)(), std::__1::__async_assoc_state<void, std::__1::__async_func<nano::test::system::stop_node(nano::node&)::$_2>>*, 2ul>(std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, void (std::__1::__async_assoc_state<void, std::__1::__async_func<nano::test::system::stop_node(nano::node&)::$_2>>::*)(), std::__1::__async_assoc_state<void, std::__1::__async_func<nano::test::system::stop_node(nano::node&)::$_2>>*>&, std::__1::__tuple_indices<2ul>) thread.h:227 (rpc_test:arm64+0x100396614)
      #12 void* std::__1::__thread_proxy[abi:ue170006]<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, void (std::__1::__async_assoc_state<void, std::__1::__async_func<nano::test::system::stop_node(nano::node&)::$_2>>::*)(), std::__1::__async_assoc_state<void, std::__1::__async_func<nano::test::system::stop_node(nano::node&)::$_2>>*>>(void*) thread.h:238 (rpc_test:arm64+0x100395fac)

@gr0vity-dev-bot
Copy link
Copy Markdown

gr0vity-dev-bot commented Feb 2, 2026

Test Results for Commit ddf7eb5

Pull Request 5022: Results
Overall Status: ❌

Test Case Results

  • ✅ 5n4pr_conf_10k_bintree: PASS (Duration: 117s)
  • ✅ 5n4pr_conf_10k_change: PASS (Duration: 154s)
  • ✅ 5n4pr_conf_change_dependant: PASS (Duration: 139s)
  • ✅ 5n4pr_conf_change_independant: PASS (Duration: 141s)
  • ✅ 5n4pr_conf_send_dependant: PASS (Duration: 131s)
  • ✅ 5n4pr_conf_send_independant: PASS (Duration: 134s)
  • ✅ 5n4pr_rocks_10k_bintree: PASS (Duration: 117s)
  • ❌ 5n4pr_rocks_10k_change: FAIL (Duration: 275s)
  • Log

Last updated: 2026-02-02 16:16:51 UTC

@pwojcikdev pwojcikdev requested a review from Copilot February 2, 2026 16:25
Copilot AI reviewed Feb 2, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Addresses a ThreadSanitizer-reported data race in nano::request_aggregator::request() by avoiding unsynchronized access to the threads vector during shutdown.

Changes:

  • Introduces an std::atomic<bool> enabled flag to gate request acceptance without reading threads.empty().
  • Switches start() and request() to use the new flag.
  • Removes an unused lambda capture in queue.priority_query.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
nano/node/request_aggregator.hpp Adds an atomic enabled flag member to control request acceptance.
nano/node/request_aggregator.cpp Uses the atomic flag to avoid the threads.empty() access that triggered TSAN.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

nano/node/request_aggregator.hpp
using value_type = std::pair<request_type, std::shared_ptr<nano::transport::channel>>;
nano::fair_queue<value_type, nano::no_value> queue;

std::atomic<bool> enabled{ true };
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

std::atomic<bool> is used here but this header doesn't include <atomic>. Please add the missing include to avoid relying on transitive includes / include-order (this can fail to compile in some translation units).

Copilot uses AI. Check for mistakes.
@pwojcikdev pwojcikdev force-pushed the fix-req-aggr-shutdown branch from ddf7eb5 to 46dd17e Compare February 2, 2026 16:35
@pwojcikdev pwojcikdev merged commit 0274cf7 into nanocurrency:develop Feb 2, 2026
27 of 28 checks passed
@pwojcikdev pwojcikdev deleted the fix-req-aggr-shutdown branch February 2, 2026 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@pwojcikdev @gr0vity-dev-bot