From e5108a294a7d4fd923665e059a6eb90b0ab5a337 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Tue, 2 Oct 2018 08:39:47 +0000 Subject: [PATCH 1/4] Catch more exceptions in dtors Misc coverity reports --- contrib/epee/include/misc_language.h | 3 ++- src/common/threadpool.cpp | 10 ++++++++++ tests/crypto/main.cpp | 3 +++ tests/fuzz/base58.cpp | 2 ++ tests/fuzz/block.cpp | 2 ++ tests/fuzz/bulletproof.cpp | 2 ++ tests/fuzz/cold-outputs.cpp | 2 ++ tests/fuzz/cold-transaction.cpp | 2 ++ tests/fuzz/http-client.cpp | 2 ++ tests/fuzz/levin.cpp | 2 ++ tests/fuzz/load_from_binary.cpp | 2 ++ tests/fuzz/load_from_json.cpp | 2 ++ tests/fuzz/parse_url.cpp | 2 ++ tests/fuzz/signature.cpp | 2 ++ tests/fuzz/transaction.cpp | 2 ++ tests/net_load_tests/clt.cpp | 2 ++ tests/net_load_tests/srv.cpp | 2 ++ 17 files changed, 43 insertions(+), 1 deletion(-) diff --git a/contrib/epee/include/misc_language.h b/contrib/epee/include/misc_language.h index d5157365c..7e4eb337d 100644 --- a/contrib/epee/include/misc_language.h +++ b/contrib/epee/include/misc_language.h @@ -147,7 +147,8 @@ namespace misc_utils {} ~call_befor_die() { - m_func(); + try { m_func(); } + catch (...) { /* ignore */ } } }; diff --git a/src/common/threadpool.cpp b/src/common/threadpool.cpp index 5ea04a353..37825e31d 100644 --- a/src/common/threadpool.cpp +++ b/src/common/threadpool.cpp @@ -51,11 +51,19 @@ threadpool::threadpool(unsigned int max_threads) : running(true), active(0) { } threadpool::~threadpool() { + try { const boost::unique_lock lock(mutex); running = false; has_work.notify_all(); } + catch (...) + { + // if the lock throws, we're just do it without a lock and hope, + // since the alternative is terminate + running = false; + has_work.notify_all(); + } for (size_t i = 0; i lock(mt); if (num) MERROR("wait should have been called before waiter dtor - waiting now"); } + catch (...) { /* ignore */ } try { wait(NULL); diff --git a/tests/crypto/main.cpp b/tests/crypto/main.cpp index e5406f771..cc3b53b83 100644 --- a/tests/crypto/main.cpp +++ b/tests/crypto/main.cpp @@ -35,6 +35,7 @@ #include #include "warnings.h" +#include "misc_log_ex.h" #include "crypto/crypto.h" #include "crypto/hash.h" #include "crypto-tests.h" @@ -59,6 +60,7 @@ bool operator !=(const key_derivation &a, const key_derivation &b) { DISABLE_GCC_WARNING(maybe-uninitialized) int main(int argc, char *argv[]) { + TRY_ENTRY(); fstream input; string cmd; size_t test = 0; @@ -266,4 +268,5 @@ error: error = true; } return error ? 1 : 0; + CATCH_ENTRY_L0("main", 1); } diff --git a/tests/fuzz/base58.cpp b/tests/fuzz/base58.cpp index 49516dd83..a4857bdd1 100644 --- a/tests/fuzz/base58.cpp +++ b/tests/fuzz/base58.cpp @@ -68,7 +68,9 @@ int Base58Fuzzer::run(const std::string &filename) int main(int argc, const char **argv) { + TRY_ENTRY(); Base58Fuzzer fuzzer; return run_fuzzer(argc, argv, fuzzer); + CATCH_ENTRY_L0("main", 1); } diff --git a/tests/fuzz/block.cpp b/tests/fuzz/block.cpp index 2df77b046..eed3b94b2 100644 --- a/tests/fuzz/block.cpp +++ b/tests/fuzz/block.cpp @@ -61,6 +61,8 @@ int BlockFuzzer::run(const std::string &filename) int main(int argc, const char **argv) { + TRY_ENTRY(); BlockFuzzer fuzzer; return run_fuzzer(argc, argv, fuzzer); + CATCH_ENTRY_L0("main", 1); } diff --git a/tests/fuzz/bulletproof.cpp b/tests/fuzz/bulletproof.cpp index 2f4dfd0ea..2f3a2f8d1 100644 --- a/tests/fuzz/bulletproof.cpp +++ b/tests/fuzz/bulletproof.cpp @@ -65,6 +65,8 @@ int BulletproofFuzzer::run(const std::string &filename) int main(int argc, const char **argv) { + TRY_ENTRY(); BulletproofFuzzer fuzzer; return run_fuzzer(argc, argv, fuzzer); + CATCH_ENTRY_L0("main", 1); } diff --git a/tests/fuzz/cold-outputs.cpp b/tests/fuzz/cold-outputs.cpp index 59b59810c..488a3b931 100644 --- a/tests/fuzz/cold-outputs.cpp +++ b/tests/fuzz/cold-outputs.cpp @@ -95,7 +95,9 @@ int ColdOutputsFuzzer::run(const std::string &filename) int main(int argc, const char **argv) { + TRY_ENTRY(); ColdOutputsFuzzer fuzzer; return run_fuzzer(argc, argv, fuzzer); + CATCH_ENTRY_L0("main", 1); } diff --git a/tests/fuzz/cold-transaction.cpp b/tests/fuzz/cold-transaction.cpp index da33dc318..fa3041ba3 100644 --- a/tests/fuzz/cold-transaction.cpp +++ b/tests/fuzz/cold-transaction.cpp @@ -97,6 +97,8 @@ int ColdTransactionFuzzer::run(const std::string &filename) int main(int argc, const char **argv) { + TRY_ENTRY(); ColdTransactionFuzzer fuzzer; return run_fuzzer(argc, argv, fuzzer); + CATCH_ENTRY_L0("main", 1); } diff --git a/tests/fuzz/http-client.cpp b/tests/fuzz/http-client.cpp index cd52643d9..909325832 100644 --- a/tests/fuzz/http-client.cpp +++ b/tests/fuzz/http-client.cpp @@ -92,7 +92,9 @@ int HTTPClientFuzzer::run(const std::string &filename) int main(int argc, const char **argv) { + TRY_ENTRY(); HTTPClientFuzzer fuzzer; return run_fuzzer(argc, argv, fuzzer); + CATCH_ENTRY_L0("main", 1); } diff --git a/tests/fuzz/levin.cpp b/tests/fuzz/levin.cpp index 4ced1837f..d0c5803f5 100644 --- a/tests/fuzz/levin.cpp +++ b/tests/fuzz/levin.cpp @@ -341,7 +341,9 @@ int LevinFuzzer::run(const std::string &filename) int main(int argc, const char **argv) { + TRY_ENTRY(); LevinFuzzer fuzzer; return run_fuzzer(argc, argv, fuzzer); + CATCH_ENTRY_L0("main", 1); } diff --git a/tests/fuzz/load_from_binary.cpp b/tests/fuzz/load_from_binary.cpp index 8f96c454f..89f122902 100644 --- a/tests/fuzz/load_from_binary.cpp +++ b/tests/fuzz/load_from_binary.cpp @@ -70,7 +70,9 @@ int PortableStorageFuzzer::run(const std::string &filename) int main(int argc, const char **argv) { + TRY_ENTRY(); PortableStorageFuzzer fuzzer; return run_fuzzer(argc, argv, fuzzer); + CATCH_ENTRY_L0("main", 1); } diff --git a/tests/fuzz/load_from_json.cpp b/tests/fuzz/load_from_json.cpp index b0c1a9bf3..083555f7e 100644 --- a/tests/fuzz/load_from_json.cpp +++ b/tests/fuzz/load_from_json.cpp @@ -70,7 +70,9 @@ int PortableStorageFuzzer::run(const std::string &filename) int main(int argc, const char **argv) { + TRY_ENTRY(); PortableStorageFuzzer fuzzer; return run_fuzzer(argc, argv, fuzzer); + CATCH_ENTRY_L0("main", 1); } diff --git a/tests/fuzz/parse_url.cpp b/tests/fuzz/parse_url.cpp index 8812cf9c2..fb5754a70 100644 --- a/tests/fuzz/parse_url.cpp +++ b/tests/fuzz/parse_url.cpp @@ -68,7 +68,9 @@ int ParseURLFuzzer::run(const std::string &filename) int main(int argc, const char **argv) { + TRY_ENTRY(); ParseURLFuzzer fuzzer; return run_fuzzer(argc, argv, fuzzer); + CATCH_ENTRY_L0("main", 1); } diff --git a/tests/fuzz/signature.cpp b/tests/fuzz/signature.cpp index 6dadf960d..f82ada8b4 100644 --- a/tests/fuzz/signature.cpp +++ b/tests/fuzz/signature.cpp @@ -92,6 +92,8 @@ int SignatureFuzzer::run(const std::string &filename) int main(int argc, const char **argv) { + TRY_ENTRY(); SignatureFuzzer fuzzer; return run_fuzzer(argc, argv, fuzzer); + CATCH_ENTRY_L0("main", 1); } diff --git a/tests/fuzz/transaction.cpp b/tests/fuzz/transaction.cpp index b3349c383..934bd4685 100644 --- a/tests/fuzz/transaction.cpp +++ b/tests/fuzz/transaction.cpp @@ -61,6 +61,8 @@ int TransactionFuzzer::run(const std::string &filename) int main(int argc, const char **argv) { + TRY_ENTRY(); TransactionFuzzer fuzzer; return run_fuzzer(argc, argv, fuzzer); + CATCH_ENTRY_L0("main", 1); } diff --git a/tests/net_load_tests/clt.cpp b/tests/net_load_tests/clt.cpp index a65e02cab..4ae50d064 100644 --- a/tests/net_load_tests/clt.cpp +++ b/tests/net_load_tests/clt.cpp @@ -628,6 +628,7 @@ TEST_F(net_load_test_clt, permament_open_and_close_and_connections_closed_by_ser int main(int argc, char** argv) { + TRY_ENTRY(); tools::on_startup(); epee::debug::get_set_enable_assert(true, false); //set up logging options @@ -635,4 +636,5 @@ int main(int argc, char** argv) ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); + CATCH_ENTRY_L0("main", 1); } diff --git a/tests/net_load_tests/srv.cpp b/tests/net_load_tests/srv.cpp index 6518d9117..dc3772127 100644 --- a/tests/net_load_tests/srv.cpp +++ b/tests/net_load_tests/srv.cpp @@ -215,6 +215,7 @@ namespace int main(int argc, char** argv) { + TRY_ENTRY(); tools::on_startup(); //set up logging options mlog_configure(mlog_get_default_log_path("net_load_tests_srv.log"), true); @@ -233,4 +234,5 @@ int main(int argc, char** argv) if (!tcp_server.run_server(thread_count, true)) return 2; return 0; + CATCH_ENTRY_L0("main", 1); } From 758d7684863b8ff001758c4360ed7087245eac03 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Tue, 2 Oct 2018 09:54:38 +0000 Subject: [PATCH 2/4] connection_basic: remove unused floating time start time --- contrib/epee/include/net/connection_basic.hpp | 3 --- contrib/epee/src/connection_basic.cpp | 7 ------- 2 files changed, 10 deletions(-) diff --git a/contrib/epee/include/net/connection_basic.hpp b/contrib/epee/include/net/connection_basic.hpp index 095e747a5..7e8750047 100644 --- a/contrib/epee/include/net/connection_basic.hpp +++ b/contrib/epee/include/net/connection_basic.hpp @@ -92,7 +92,6 @@ class connection_basic { // not-templated base class for rapid developmet of som critical_section m_send_que_lock; std::list m_send_que; volatile bool m_is_multithreaded; - double m_start_time; /// Strand to ensure the connection's handlers are not called concurrently. boost::asio::io_service::strand strand_; /// Socket for the connection. @@ -112,8 +111,6 @@ class connection_basic { // not-templated base class for rapid developmet of som void logger_handle_net_write(size_t size); // network data written void logger_handle_net_read(size_t size); // network data read - void set_start_time(); - // config for rate limit static void set_rate_up_limit(uint64_t limit); diff --git a/contrib/epee/src/connection_basic.cpp b/contrib/epee/src/connection_basic.cpp index dea1928a7..9ab485839 100644 --- a/contrib/epee/src/connection_basic.cpp +++ b/contrib/epee/src/connection_basic.cpp @@ -250,22 +250,15 @@ void connection_basic::sleep_before_packet(size_t packet_size, int phase, int q } } -void connection_basic::set_start_time() { - CRITICAL_REGION_LOCAL( network_throttle_manager::m_lock_get_global_throttle_out ); - m_start_time = network_throttle_manager::get_global_throttle_out().get_time_seconds(); -} void connection_basic::do_send_handler_write(const void* ptr , size_t cb ) { // No sleeping here; sleeping is done once and for all in connection::handle_write MTRACE("handler_write (direct) - before ASIO write, for packet="<::handle_write MTRACE("handler_write (after write, from queue="< Date: Tue, 2 Oct 2018 12:13:57 +0000 Subject: [PATCH 3/4] abstract_tcp_server2: move m_period to subclass This is where it is actually used, and initialized --- contrib/epee/include/net/abstract_tcp_server2.h | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/contrib/epee/include/net/abstract_tcp_server2.h b/contrib/epee/include/net/abstract_tcp_server2.h index b2c05ebb0..3f726a352 100644 --- a/contrib/epee/include/net/abstract_tcp_server2.h +++ b/contrib/epee/include/net/abstract_tcp_server2.h @@ -246,7 +246,6 @@ namespace net_utils m_timer(io_serice) {} boost::asio::deadline_timer m_timer; - uint64_t m_period; }; template @@ -262,25 +261,27 @@ namespace net_utils { return m_handler(); } + uint64_t m_period; }; template bool add_idle_handler(t_handler t_callback, uint64_t timeout_ms) { - boost::shared_ptr ptr(new idle_callback_conext(io_service_, t_callback, timeout_ms)); + boost::shared_ptr> ptr(new idle_callback_conext(io_service_, t_callback, timeout_ms)); //needed call handler here ?... ptr->m_timer.expires_from_now(boost::posix_time::milliseconds(ptr->m_period)); - ptr->m_timer.async_wait(boost::bind(&boosted_tcp_server::global_timer_handler, this, ptr)); + ptr->m_timer.async_wait(boost::bind(&boosted_tcp_server::global_timer_handler, this, ptr)); return true; } - bool global_timer_handler(/*const boost::system::error_code& err, */boost::shared_ptr ptr) + template + bool global_timer_handler(/*const boost::system::error_code& err, */boost::shared_ptr> ptr) { //if handler return false - he don't want to be called anymore if(!ptr->call_handler()) return true; ptr->m_timer.expires_from_now(boost::posix_time::milliseconds(ptr->m_period)); - ptr->m_timer.async_wait(boost::bind(&boosted_tcp_server::global_timer_handler, this, ptr)); + ptr->m_timer.async_wait(boost::bind(&boosted_tcp_server::global_timer_handler, this, ptr)); return true; } From 00901e9c938dc801e963f98825c2ab5976fa8cc5 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Tue, 2 Oct 2018 12:14:44 +0000 Subject: [PATCH 4/4] epee: initialize a few data members where it seems to be appropriate --- contrib/epee/include/net/levin_protocol_handler_async.h | 4 +++- contrib/epee/src/network_throttle-detail.cpp | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/contrib/epee/include/net/levin_protocol_handler_async.h b/contrib/epee/include/net/levin_protocol_handler_async.h index e9853ee26..08aa1d468 100644 --- a/contrib/epee/include/net/levin_protocol_handler_async.h +++ b/contrib/epee/include/net/levin_protocol_handler_async.h @@ -99,7 +99,7 @@ public: size_t get_connections_count(); void set_handler(levin_commands_handler* handler, void (*destroy)(levin_commands_handler*) = NULL); - async_protocol_handler_config():m_pcommands_handler(NULL), m_pcommands_handler_destroy(NULL), m_max_packet_size(LEVIN_DEFAULT_MAX_PACKET_SIZE) + async_protocol_handler_config():m_pcommands_handler(NULL), m_pcommands_handler_destroy(NULL), m_max_packet_size(LEVIN_DEFAULT_MAX_PACKET_SIZE), m_invoke_timeout(LEVIN_DEFAULT_TIMEOUT_PRECONFIGURED) {} ~async_protocol_handler_config() { set_handler(NULL, NULL); } void del_out_connections(size_t count); @@ -272,6 +272,8 @@ public: m_wait_count = 0; m_oponent_protocol_ver = 0; m_connection_initialized = false; + m_invoke_buf_ready = 0; + m_invoke_result_code = LEVIN_ERROR_CONNECTION; } virtual ~async_protocol_handler() { diff --git a/contrib/epee/src/network_throttle-detail.cpp b/contrib/epee/src/network_throttle-detail.cpp index 7eeade3a1..28c85bb78 100644 --- a/contrib/epee/src/network_throttle-detail.cpp +++ b/contrib/epee/src/network_throttle-detail.cpp @@ -146,6 +146,7 @@ network_throttle::network_throttle(const std::string &nameshort, const std::stri m_network_add_cost = 128; m_network_minimal_segment = 256; m_network_max_segment = 1024*1024; + m_start_time = 0; m_any_packet_yet = false; m_slot_size = 1.0; // hard coded in few places m_target_speed = 16 * 1024; // other defaults are probably defined in the command-line parsing code when this class is used e.g. as main global throttle