Minecon724/dncurrency
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages 1 Wiki Activity Actions 1

fix race condition between bootstrap pull and push (#3721)

Browse source 
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
This commit is contained in:
Dimitrios Siganos 2022-02-08 12:46:36 +00:00 • committed by GitHub
commit ddf07f156b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 2 additions and 5 deletions
Download patch file Download diff file Expand all files Collapse all files

5
nano/core_test/bootstrap.cpp
View file

@ -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);

2
nano/node/bootstrap/bootstrap_frontier.cpp
View file

@ -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