From f7cef4cca5f6d28fc3171792039a4b9872c3acd6 Mon Sep 17 00:00:00 2001 From: Wesley Shillingford Date: Thu, 5 Sep 2019 12:44:41 +0100 Subject: [PATCH] Improve bootstrap attempt locking (#2286) --- ci/build-travis.sh | 4 ++-- nano/lib/locks.cpp | 3 +++ nano/node/bootstrap/bootstrap.cpp | 2 +- nano/node/bootstrap/bootstrap.hpp | 5 +++-- nano/node/bootstrap/bootstrap_bulk_pull.cpp | 1 - 5 files changed, 9 insertions(+), 6 deletions(-) diff --git a/ci/build-travis.sh b/ci/build-travis.sh index 2b58e8d5..6f9d2c89 100755 --- a/ci/build-travis.sh +++ b/ci/build-travis.sh @@ -15,8 +15,8 @@ if [[ $(grep -rl --exclude="*asio.hpp" "asio::async_write" ./nano) ]]; then fi # prevent unsolicited use of std::lock_guard & std::unique_lock outside of allowed areas -if [[ $(grep -rl --exclude={"*random_pool.cpp","*random_pool.hpp","*locks.hpp"} "std::unique_lock\|std::lock_guard" ./nano) ]]; then - echo "using std::unique_lock or std::lock_guard is not permitted (except in nano/lib/locks.hpp and non-nano dependent libraries). Use the nano::* versions instead" +if [[ $(grep -rl --exclude={"*random_pool.cpp","*random_pool.hpp","*locks.hpp","*locks.cpp"} "std::unique_lock\|std::lock_guard\|std::condition_variable" ./nano) ]]; then + echo "using std::unique_lock, std::lock_guard or std::condition_variable is not permitted (except in nano/lib/locks.hpp and non-nano dependent libraries). Use the nano::* versions instead" exit 1 fi diff --git a/nano/lib/locks.cpp b/nano/lib/locks.cpp index 733a9911..4f85560d 100644 --- a/nano/lib/locks.cpp +++ b/nano/lib/locks.cpp @@ -7,7 +7,10 @@ namespace template void output (const char * str, std::chrono::milliseconds time, Mutex & mutex) { + static std::mutex cout_mutex; auto stacktrace = nano::generate_stacktrace (); + // Guard standard out to keep the output from being interleaved + std::lock_guard guard (cout_mutex); std::cout << std::addressof (mutex) << " Mutex " << str << " for: " << time.count () << "ms\n" << stacktrace << std::endl; } diff --git a/nano/node/bootstrap/bootstrap.cpp b/nano/node/bootstrap/bootstrap.cpp index ea736351..bbfca3d6 100644 --- a/nano/node/bootstrap/bootstrap.cpp +++ b/nano/node/bootstrap/bootstrap.cpp @@ -85,7 +85,7 @@ nano::bootstrap_attempt::~bootstrap_attempt () bool nano::bootstrap_attempt::should_log () { - nano::lock_guard lock (mutex); + nano::lock_guard guard (next_log_mutex); auto result (false); auto now (std::chrono::steady_clock::now ()); if (next_log < now) diff --git a/nano/node/bootstrap/bootstrap.hpp b/nano/node/bootstrap/bootstrap.hpp index 177e232b..1e447cba 100644 --- a/nano/node/bootstrap/bootstrap.hpp +++ b/nano/node/bootstrap/bootstrap.hpp @@ -80,6 +80,7 @@ public: void wallet_run (); void wallet_start (std::deque &); bool wallet_finished (); + std::mutex next_log_mutex; std::chrono::steady_clock::time_point next_log; std::deque> clients; std::weak_ptr connection_frontier_request; @@ -97,7 +98,7 @@ public: std::atomic stopped; nano::bootstrap_mode mode; std::mutex mutex; - std::condition_variable condition; + nano::condition_variable condition; // Lazy bootstrap std::unordered_set lazy_blocks; std::unordered_map> lazy_state_unknown; @@ -176,7 +177,7 @@ private: std::shared_ptr attempt; std::atomic stopped; std::mutex mutex; - std::condition_variable condition; + nano::condition_variable condition; std::mutex observers_mutex; std::vector> observers; boost::thread thread; diff --git a/nano/node/bootstrap/bootstrap_bulk_pull.cpp b/nano/node/bootstrap/bootstrap_bulk_pull.cpp index a479d3db..54122be2 100644 --- a/nano/node/bootstrap/bootstrap_bulk_pull.cpp +++ b/nano/node/bootstrap/bootstrap_bulk_pull.cpp @@ -19,7 +19,6 @@ pull (pull_a), pull_blocks (0), unexpected_count (0) { - nano::lock_guard mutex (connection->attempt->mutex); connection->attempt->condition.notify_all (); }