From ddf07f156b31d18a599de942ec79e0e30b72d9c5 Mon Sep 17 00:00:00 2001 From: Dimitrios Siganos Date: Tue, 8 Feb 2022 12:46:36 +0000 Subject: [PATCH] fix race condition between bootstrap pull and push (#3721) The unit test bootstrap_processor.push_one fails intermittently and it exposes a race condition in the bootstrap code of the node. #3533 The race condition is that after finishing with bootstrap frontiers pull, the code is supposed to put the socket back into the idle bootstrap connections queue. However it does that in a racy way. The problem is in the function nano::frontier_req_client::received_frontier. Near the bottom of that function, we should call: connection->connections.pool_connection (connection); before calling promise.set_value (false); Once the promise is given a value then nano::bootstrap_attempt_legacy::run() continues to call push_request in parallel with pool_connection and it is a race condition. Although this is a bug, this is likely not a major nor catastrophic bug because the bootstrap push functionality is only an efficiency. resolves #3533 resolves #3720 --- nano/core_test/bootstrap.cpp | 5 +---- nano/node/bootstrap/bootstrap_frontier.cpp | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/nano/core_test/bootstrap.cpp b/nano/core_test/bootstrap.cpp index 7a752d0d..8ef2d944 100644 --- a/nano/core_test/bootstrap.cpp +++ b/nano/core_test/bootstrap.cpp @@ -459,10 +459,7 @@ TEST (bootstrap_processor, DISABLED_push_diamond_pruning) node1->stop (); } -// Test disabled because it's failing intermittently. -// PR in which it got disabled: https://github.com/nanocurrency/nano-node/pull/3532 -// Issue for investigating it: https://github.com/nanocurrency/nano-node/issues/3533 -TEST (bootstrap_processor, DISABLED_push_one) +TEST (bootstrap_processor, push_one) { nano::system system; nano::node_config config (nano::get_available_port (), system.logging); diff --git a/nano/node/bootstrap/bootstrap_frontier.cpp b/nano/node/bootstrap/bootstrap_frontier.cpp index 935c70eb..219a4e24 100644 --- a/nano/node/bootstrap/bootstrap_frontier.cpp +++ b/nano/node/bootstrap/bootstrap_frontier.cpp @@ -192,6 +192,7 @@ void nano::frontier_req_client::received_frontier (boost::system::error_code con // Set last processed account as new start target attempt->set_start_account (last_account); } + connection->connections.pool_connection (connection); try { promise.set_value (false); @@ -199,7 +200,6 @@ void nano::frontier_req_client::received_frontier (boost::system::error_code con catch (std::future_error &) { } - connection->connections.pool_connection (connection); } } else