From 6e1cfda316e5649488d4c59dc89704a572946b8a Mon Sep 17 00:00:00 2001 From: Sergey Kroshnin Date: Fri, 15 Mar 2019 13:55:45 +0300 Subject: [PATCH] Valgrind fixes & suppressions (#1814) * Fix detelcted by Valgrind compute_reps () issue * Improve logging headers * Various minor tests improvements * Valgrind suppression file * Remove default value for min_log_delta --- nano/core_test/logger.cpp | 6 +++--- nano/core_test/network.cpp | 22 +++++++++++----------- nano/core_test/rpc.cpp | 7 +++---- nano/core_test/wallets.cpp | 6 ++++-- nano/node/logging.hpp | 11 ++++++----- nano/node/node.cpp | 8 ++++---- nano/secure/ledger.hpp | 2 +- valgrind.supp | 26 ++++++++++++++++++++++++++ 8 files changed, 58 insertions(+), 30 deletions(-) create mode 100644 valgrind.supp diff --git a/nano/core_test/logger.cpp b/nano/core_test/logger.cpp index 88b56e02..09e86d27 100644 --- a/nano/core_test/logger.cpp +++ b/nano/core_test/logger.cpp @@ -122,7 +122,7 @@ TEST (logger, changing_time_interval) nano::logger_mt my_logger (logging.min_time_between_log_output); auto error (my_logger.try_log ("logger.changing_time_interval1")); ASSERT_FALSE (error); - logging.min_time_between_log_output = 20s; + my_logger.min_log_delta = 20s; error = my_logger.try_log ("logger, changing_time_interval2"); ASSERT_TRUE (error); } @@ -132,7 +132,7 @@ TEST (logger, try_log) auto path1 (nano::unique_path ()); std::stringstream ss; boost_log_cerr_redirect redirect_cerr (ss.rdbuf ()); - nano::logger_mt my_logger (3ms); + nano::logger_mt my_logger (100ms); auto output1 = "logger.try_log1"; auto error (my_logger.try_log (output1)); ASSERT_FALSE (error); @@ -141,7 +141,7 @@ TEST (logger, try_log) ASSERT_TRUE (error); // Fails as it is occuring too soon // Sleep for a bit and then confirm - std::this_thread::sleep_for (3ms); + std::this_thread::sleep_for (100ms); error = my_logger.try_log (output2); ASSERT_FALSE (error); diff --git a/nano/core_test/network.cpp b/nano/core_test/network.cpp index 3d520d64..0026986d 100644 --- a/nano/core_test/network.cpp +++ b/nano/core_test/network.cpp @@ -1486,22 +1486,22 @@ TEST (confirmation_height, multiple) system.wallet (1)->insert_adhoc (key3.prv); // Send to all accounts - nano::send_block send1 (latest1, key1.pub, 300, nano::test_genesis_key.prv, nano::test_genesis_key.pub, 0); - nano::send_block send2 (send1.hash (), key2.pub, 1, nano::test_genesis_key.prv, nano::test_genesis_key.pub, 0); - nano::send_block send3 (send2.hash (), key3.pub, 1, nano::test_genesis_key.prv, nano::test_genesis_key.pub, 0); + nano::send_block send1 (latest1, key1.pub, 300, nano::test_genesis_key.prv, nano::test_genesis_key.pub, system.work.generate (latest1)); + nano::send_block send2 (send1.hash (), key2.pub, 1, nano::test_genesis_key.prv, nano::test_genesis_key.pub, system.work.generate (send1.hash ())); + nano::send_block send3 (send2.hash (), key3.pub, 1, nano::test_genesis_key.prv, nano::test_genesis_key.pub, system.work.generate (send2.hash ())); // Open all accounts - nano::open_block open1 (send1.hash (), nano::genesis_account, key1.pub, key1.prv, key1.pub, 0); - nano::open_block open2 (send2.hash (), nano::genesis_account, key2.pub, key2.prv, key2.pub, 0); - nano::open_block open3 (send3.hash (), nano::genesis_account, key3.pub, key3.prv, key3.pub, 0); + nano::open_block open1 (send1.hash (), nano::genesis_account, key1.pub, key1.prv, key1.pub, system.work.generate (key1.pub)); + nano::open_block open2 (send2.hash (), nano::genesis_account, key2.pub, key2.prv, key2.pub, system.work.generate (key2.pub)); + nano::open_block open3 (send3.hash (), nano::genesis_account, key3.pub, key3.prv, key3.pub, system.work.generate (key3.pub)); // Send and recieve various blocks to these accounts - nano::send_block send4 (open1.hash (), key2.pub, 50, key1.prv, key1.pub, 0); - nano::send_block send5 (send4.hash (), key2.pub, 10, key1.prv, key1.pub, 0); + nano::send_block send4 (open1.hash (), key2.pub, 50, key1.prv, key1.pub, system.work.generate (open1.hash ())); + nano::send_block send5 (send4.hash (), key2.pub, 10, key1.prv, key1.pub, system.work.generate (send4.hash ())); - nano::receive_block receive1 (open2.hash (), send4.hash (), key2.prv, key2.pub, 0); - nano::send_block send6 (receive1.hash (), key3.pub, 10, key2.prv, key2.pub, 0); - nano::receive_block receive2 (send6.hash (), send5.hash (), key2.prv, key2.pub, 0); + nano::receive_block receive1 (open2.hash (), send4.hash (), key2.prv, key2.pub, system.work.generate (open2.hash ())); + nano::send_block send6 (receive1.hash (), key3.pub, 10, key2.prv, key2.pub, system.work.generate (receive1.hash ())); + nano::receive_block receive2 (send6.hash (), send5.hash (), key2.prv, key2.pub, system.work.generate (send6.hash ())); for (auto & node : system.nodes) { diff --git a/nano/core_test/rpc.cpp b/nano/core_test/rpc.cpp index 09656c0b..6a1101e0 100644 --- a/nano/core_test/rpc.cpp +++ b/nano/core_test/rpc.cpp @@ -1991,7 +1991,7 @@ TEST (rpc, work_generate_difficulty) request1.put ("hash", hash1.to_string ()); request1.put ("difficulty", nano::to_string_hex (difficulty1)); test_response response1 (request1, rpc, system.io_ctx); - system.deadline_set (5s); + system.deadline_set (10s); while (response1.status == 0) { ASSERT_NO_ERROR (system.poll ()); @@ -2006,7 +2006,7 @@ TEST (rpc, work_generate_difficulty) uint64_t difficulty2 (0xffff000000000000); request1.put ("difficulty", nano::to_string_hex (difficulty2)); test_response response2 (request1, rpc, system.io_ctx); - system.deadline_set (10s); + system.deadline_set (20s); while (response2.status == 0) { ASSERT_NO_ERROR (system.poll ()); @@ -2034,7 +2034,7 @@ TEST (rpc, work_cancel) boost::property_tree::ptree request1; request1.put ("action", "work_cancel"); request1.put ("hash", hash1.to_string ()); - auto done (false); + std::atomic done (false); system.deadline_set (10s); while (!done) { @@ -4600,7 +4600,6 @@ TEST (rpc, unopened) { nano::system system (24000, 1); system.wallet (0)->insert_adhoc (nano::test_genesis_key.prv); - auto transaction (system.wallet (0)->wallets.tx_begin_write ()); nano::account account1 (1), account2 (account1.number () + 1); auto genesis (system.nodes[0]->latest (nano::test_genesis_key.pub)); ASSERT_FALSE (genesis.is_zero ()); diff --git a/nano/core_test/wallets.cpp b/nano/core_test/wallets.cpp index 60d7701a..ee585855 100644 --- a/nano/core_test/wallets.cpp +++ b/nano/core_test/wallets.cpp @@ -16,7 +16,8 @@ TEST (wallets, open_create) nano::wallets wallets (error, *system.nodes[0]); ASSERT_FALSE (error); ASSERT_EQ (1, wallets.items.size ()); // it starts out with a default wallet - nano::uint256_union id; + nano::keypair random_key; + nano::uint256_union id (random_key.pub); ASSERT_EQ (nullptr, wallets.open (id)); auto wallet (wallets.create (id)); ASSERT_NE (nullptr, wallet); @@ -26,7 +27,8 @@ TEST (wallets, open_create) TEST (wallets, open_existing) { nano::system system (24000, 1); - nano::uint256_union id; + nano::keypair random_key; + nano::uint256_union id (random_key.pub); { bool error (false); nano::wallets wallets (error, *system.nodes[0]); diff --git a/nano/node/logging.hpp b/nano/node/logging.hpp index fb2db289..0b2f5cf8 100644 --- a/nano/node/logging.hpp +++ b/nano/node/logging.hpp @@ -69,8 +69,8 @@ public: bool try_log (LogItems &&... log_items) { auto error (true); - auto time_now = steady_clock::now (); - if (((time_now - last_log_time) > min_log_delta) || last_log_time == steady_clock::time_point{}) + auto time_now = std::chrono::steady_clock::now (); + if (((time_now - last_log_time) > min_log_delta) || last_log_time == std::chrono::steady_clock::time_point{}) { output (std::forward (log_items)...); last_log_time = time_now; @@ -79,9 +79,10 @@ public: return error; } + std::chrono::milliseconds min_log_delta; + private: - milliseconds const & min_log_delta; - steady_clock::time_point last_log_time; + std::chrono::steady_clock::time_point last_log_time; boost::log::sources::logger_mt boost_logger_mt; }; @@ -133,7 +134,7 @@ public: bool flush{ true }; uintmax_t max_size{ 128 * 1024 * 1024 }; uintmax_t rotation_size{ 4 * 1024 * 1024 }; - milliseconds min_time_between_log_output{ 5 }; + std::chrono::milliseconds min_time_between_log_output{ 5 }; nano::logger_mt logger{ min_time_between_log_output }; int json_version () const { diff --git a/nano/node/node.cpp b/nano/node/node.cpp index f28c77cf..3361bc4d 100644 --- a/nano/node/node.cpp +++ b/nano/node/node.cpp @@ -3723,8 +3723,8 @@ void nano::thread_runner::join () } } -nano::inactive_node::inactive_node (boost::filesystem::path const & path, uint16_t peering_port_a) : -path (path), +nano::inactive_node::inactive_node (boost::filesystem::path const & path_a, uint16_t peering_port_a) : +path (path_a), io_context (std::make_shared ()), alarm (*io_context), work (1, nullptr), @@ -3747,8 +3747,8 @@ nano::inactive_node::~inactive_node () node->stop (); } -nano::udp_buffer::udp_buffer (nano::stat & stats, size_t size, size_t count) : -stats (stats), +nano::udp_buffer::udp_buffer (nano::stat & stats_a, size_t size, size_t count) : +stats (stats_a), free (count), full (count), slab (size * count), diff --git a/nano/secure/ledger.hpp b/nano/secure/ledger.hpp index d02114f3..07f5eaec 100644 --- a/nano/secure/ledger.hpp +++ b/nano/secure/ledger.hpp @@ -51,7 +51,7 @@ public: nano::block_store & store; nano::stat & stats; std::unordered_map bootstrap_weights; - uint64_t bootstrap_weight_max_blocks; + uint64_t bootstrap_weight_max_blocks{ 1 }; std::atomic check_bootstrap_weights; nano::uint256_union epoch_link; nano::account epoch_signer; diff --git a/valgrind.supp b/valgrind.supp new file mode 100644 index 00000000..6ef9a482 --- /dev/null +++ b/valgrind.supp @@ -0,0 +1,26 @@ +{ + Boost_Log_Initialization + Memcheck:Cond + fun:_ZN5boost10filesystem6detail28directory_iterator_incrementERNS0_18directory_iteratorEPNS_6system10error_codeE +} + +{ + LMDB_block_exists + Memcheck:Cond + ... + fun:mdb_node_search +} + +{ + wallets_entry_get + Memcheck:Cond + ... + fun:_ZN4nano12wallet_store13entry_get_rawERKNS_11transactionERKNS_13uint256_unionE +} + +{ + wallets_entry_get_size8 + Memcheck:Value8 + ... + fun:_ZN4nano12wallet_store13entry_get_rawERKNS_11transactionERKNS_13uint256_unionE +}