diff --git a/nano/core_test/wallet.cpp b/nano/core_test/wallet.cpp index a3cdc14f4..acf755c30 100644 --- a/nano/core_test/wallet.cpp +++ b/nano/core_test/wallet.cpp @@ -1556,3 +1556,22 @@ TEST (wallet, epoch_2_receive_unopened) } ASSERT_LT (tries, max_tries); } + +TEST (wallet, foreach_representative_deadlock) +{ + nano::system system (1); + auto & node (*system.nodes[0]); + system.wallet (0)->insert_adhoc (nano::test_genesis_key.prv); + node.wallets.compute_reps (); + ASSERT_EQ (1, node.wallets.rep_counts ().voting); + node.wallets.foreach_representative ([&node](nano::public_key const & pub, nano::raw_key const & prv) { + if (node.wallets.mutex.try_lock ()) + { + node.wallets.mutex.unlock (); + } + else + { + ASSERT_FALSE (true); + } + }); +} diff --git a/nano/node/voting.cpp b/nano/node/voting.cpp index a0cb8026f..ab2878902 100644 --- a/nano/node/voting.cpp +++ b/nano/node/voting.cpp @@ -137,9 +137,12 @@ wallets (wallets_a) void nano::votes_cache::add (std::shared_ptr const & vote_a) { - nano::lock_guard lock (cache_mutex); auto voting (wallets.rep_counts ().voting); - debug_assert (voting > 0); + if (voting == 0) + { + return; + } + nano::lock_guard lock (cache_mutex); auto const max_cache_size (network_params.voting.max_cache / std::max (voting, static_cast (1))); for (auto & block : vote_a->blocks) { diff --git a/nano/node/wallet.cpp b/nano/node/wallet.cpp index 3413210e6..a0ad37e65 100644 --- a/nano/node/wallet.cpp +++ b/nano/node/wallet.cpp @@ -1750,44 +1750,51 @@ void nano::wallets::foreach_representative (std::function lock (mutex); - auto transaction_l (tx_begin_read ()); - for (auto i (items.begin ()), n (items.end ()); i != n; ++i) + std::vector> action_accounts_l; { - auto & wallet (*i->second); - nano::lock_guard store_lock (wallet.store.mutex); - decltype (wallet.representatives) representatives_l; + auto transaction_l (tx_begin_read ()); + nano::lock_guard lock (mutex); + for (auto i (items.begin ()), n (items.end ()); i != n; ++i) { - nano::lock_guard representatives_lock (wallet.representatives_mutex); - representatives_l = wallet.representatives; - } - for (auto const & account : representatives_l) - { - if (wallet.store.exists (transaction_l, account)) + auto & wallet (*i->second); + nano::lock_guard store_lock (wallet.store.mutex); + decltype (wallet.representatives) representatives_l; { - if (!node.ledger.weight (account).is_zero ()) + nano::lock_guard representatives_lock (wallet.representatives_mutex); + representatives_l = wallet.representatives; + } + for (auto const & account : representatives_l) + { + if (wallet.store.exists (transaction_l, account)) { - if (wallet.store.valid_password (transaction_l)) + if (!node.ledger.weight (account).is_zero ()) { - nano::raw_key prv; - auto error (wallet.store.fetch (transaction_l, account, prv)); - (void)error; - debug_assert (!error); - action_a (account, prv); - } - else - { - static auto last_log = std::chrono::steady_clock::time_point (); - if (last_log < std::chrono::steady_clock::now () - std::chrono::seconds (60)) + if (wallet.store.valid_password (transaction_l)) { - last_log = std::chrono::steady_clock::now (); - node.logger.always_log (boost::str (boost::format ("Representative locked inside wallet %1%") % i->first.to_string ())); + nano::raw_key prv; + auto error (wallet.store.fetch (transaction_l, account, prv)); + (void)error; + debug_assert (!error); + action_accounts_l.emplace_back (account, prv); + } + else + { + static auto last_log = std::chrono::steady_clock::time_point (); + if (last_log < std::chrono::steady_clock::now () - std::chrono::seconds (60)) + { + last_log = std::chrono::steady_clock::now (); + node.logger.always_log (boost::str (boost::format ("Representative locked inside wallet %1%") % i->first.to_string ())); + } } } } } } } + for (auto const & representative : action_accounts_l) + { + action_a (representative.first, representative.second); + } } }