Remove atomic enum election::state_m and instead require locking elec…

…tion::mutex.
nanocurrency · Oct 31, 2023 · f83b4f7 · f83b4f7
1 parent d7f2b8e
commit f83b4f7
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 15 deletions.
diff --git a/nano/node/election.cpp b/nano/node/election.cpp
@@ -37,7 +37,9 @@ void nano::election::confirm_once (nano::unique_lock<nano::mutex> & lock_a, nano
 	debug_assert (lock_a.owns_lock ());
 	// This must be kept above the setting of election state, as dependent confirmed elections require up to date changes to election_winner_details
 	nano::unique_lock<nano::mutex> election_winners_lk{ node.active.election_winner_details_mutex };
-	if (state_m.exchange (nano::election::state_t::confirmed) != nano::election::state_t::confirmed && (node.active.election_winner_details.count (status.winner->hash ()) == 0))
+	auto just_confirmed = state_m != nano::election::state_t::confirmed;
+	state_m = nano::election::state_t::confirmed;
+	if (just_confirmed && (node.active.election_winner_details.count (status.winner->hash ()) == 0))
 	{
 		node.active.election_winner_details.emplace (status.winner->hash (), shared_from_this ());
 		election_winners_lk.unlock ();
@@ -115,8 +117,9 @@ bool nano::election::state_change (nano::election::state_t expected_a, nano::ele
 	bool result = true;
 	if (valid_change (expected_a, desired_a))
 	{
-		if (state_m.compare_exchange_strong (expected_a, desired_a))
+		if (state_m == expected_a)
 		{
+			state_m = desired_a;
 			state_start = std::chrono::steady_clock::now ().time_since_epoch ();
 			result = false;
 		}
@@ -142,7 +145,6 @@ void nano::election::send_confirm_req (nano::confirmation_solicitor & solicitor_
 {
 	if (confirm_req_time () < (std::chrono::steady_clock::now () - last_req))
 	{
-		nano::lock_guard<nano::mutex> guard{ mutex };
 		if (!solicitor_a.add (*this))
 		{
 			last_req = std::chrono::steady_clock::now ();
@@ -170,7 +172,6 @@ void nano::election::broadcast_block (nano::confirmation_solicitor & solicitor_a
 {
 	if (base_latency () * 15 < std::chrono::steady_clock::now () - last_block)
 	{
-		nano::lock_guard<nano::mutex> guard{ mutex };
 		if (!solicitor_a.broadcast (*this))
 		{
 			last_block = std::chrono::steady_clock::now ();
@@ -181,11 +182,7 @@ void nano::election::broadcast_block (nano::confirmation_solicitor & solicitor_a
 void nano::election::broadcast_vote ()
 {
 	nano::unique_lock<nano::mutex> lock{ mutex };
-	if (last_vote + std::chrono::milliseconds (node.config.network_params.network.vote_broadcast_interval) < std::chrono::steady_clock::now ())
-	{
-		broadcast_vote_impl ();
-		last_vote = std::chrono::steady_clock::now ();
-	}
+	broadcast_vote_impl ();
 }
 
 nano::vote_info nano::election::get_last_vote (nano::account const & account)
@@ -208,17 +205,18 @@ nano::election_status nano::election::get_status () const
 
 bool nano::election::transition_time (nano::confirmation_solicitor & solicitor_a)
 {
+	nano::lock_guard<nano::mutex> guard{ mutex };
 	bool result = false;
 	switch (state_m)
 	{
 		case nano::election::state_t::passive:
-			if (base_latency () * passive_duration_factor < std::chrono::steady_clock::now ().time_since_epoch () - state_start.load ())
+			if (base_latency () * passive_duration_factor < std::chrono::steady_clock::now ().time_since_epoch () - state_start)
 			{
 				state_change (nano::election::state_t::passive, nano::election::state_t::active);
 			}
 			break;
 		case nano::election::state_t::active:
-			broadcast_vote ();
+			broadcast_vote_impl ();
 			broadcast_block (solicitor_a);
 			send_confirm_req (solicitor_a);
 			break;
@@ -237,7 +235,7 @@ bool nano::election::transition_time (nano::confirmation_solicitor & solicitor_a
 		nano::lock_guard<nano::mutex> guard{ mutex };
 		// It is possible the election confirmed while acquiring the mutex
 		// state_change returning true would indicate it
-		if (!state_change (state_m.load (), nano::election::state_t::expired_unconfirmed))
+		if (!state_change (state_m, nano::election::state_t::expired_unconfirmed))
 		{
 			result = true; // Return true to indicate this election should be cleaned up
 			if (node.config.logging.election_expiration_tally_logging ())
@@ -553,6 +551,11 @@ void nano::election::broadcast_vote_impl ()
 {
 	debug_assert (!mutex.try_lock ());
 
+	if (std::chrono::steady_clock::now () < last_vote + std::chrono::milliseconds (node.config.network_params.network.vote_broadcast_interval))
+	{
+		return;
+	}
+	last_vote = std::chrono::steady_clock::now ();
 	if (node.config.enable_voting && node.wallets.reps ().voting > 0)
 	{
 		node.stats.inc (nano::stat::type::election, nano::stat::detail::generate_vote);

diff --git a/nano/node/election.hpp b/nano/node/election.hpp
@@ -97,10 +97,9 @@ class election final : public std::enable_shared_from_this<nano::election>
 
 	static unsigned constexpr passive_duration_factor = 5;
 	static unsigned constexpr active_request_count_min = 2;
-	std::atomic<nano::election::state_t> state_m = { state_t::passive };
+	nano::election::state_t state_m = { state_t::passive };
 
-	static_assert (std::is_trivial<std::chrono::steady_clock::duration> ());
-	std::atomic<std::chrono::steady_clock::duration> state_start{ std::chrono::steady_clock::now ().time_since_epoch () };
+	std::chrono::steady_clock::duration state_start{ std::chrono::steady_clock::now ().time_since_epoch () };
 
 	// These are modified while not holding the mutex from transition_time only
 	std::chrono::steady_clock::time_point last_block = { std::chrono::steady_clock::now () };