From 340830de5bdb3dcc57e0767f311f514f13d8690f Mon Sep 17 00:00:00 2001 From: Howard Chu Date: Wed, 31 May 2017 15:34:31 +0100 Subject: [PATCH 1/3] Fix PR#2039 Missed a crypto -> cncrypto rename --- contrib/epee/src/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/epee/src/CMakeLists.txt b/contrib/epee/src/CMakeLists.txt index 1d5fa039..43785183 100644 --- a/contrib/epee/src/CMakeLists.txt +++ b/contrib/epee/src/CMakeLists.txt @@ -40,7 +40,7 @@ endif() target_link_libraries(epee PUBLIC - crypto + cncrypto easylogging ${Boost_FILESYSTEM_LIBRARY} PRIVATE From cf3a376cb54eaa4805ce79828de97b46988b1858 Mon Sep 17 00:00:00 2001 From: Howard Chu Date: Sun, 4 Jun 2017 22:36:09 +0100 Subject: [PATCH 2/3] Don't timeout a slow operation that's making progress If we got at least MIN_BYTES_WANTED (default 512) during any network poll, reset the timeout to allow more time for data to arrive. --- .../net/levin_protocol_handler_async.h | 55 ++++++++++++++++--- 1 file changed, 46 insertions(+), 9 deletions(-) diff --git a/contrib/epee/include/net/levin_protocol_handler_async.h b/contrib/epee/include/net/levin_protocol_handler_async.h index 891089be..5ef78220 100644 --- a/contrib/epee/include/net/levin_protocol_handler_async.h +++ b/contrib/epee/include/net/levin_protocol_handler_async.h @@ -42,6 +42,10 @@ #undef MONERO_DEFAULT_LOG_CATEGORY #define MONERO_DEFAULT_LOG_CATEGORY "net" +#ifndef MIN_BYTES_WANTED +#define MIN_BYTES_WANTED 512 +#endif + namespace epee { namespace levin @@ -139,26 +143,23 @@ public: virtual bool is_timer_started() const=0; virtual void cancel()=0; virtual bool cancel_timer()=0; + virtual void reset_timer()=0; + virtual void timeout_handler(const boost::system::error_code& error)=0; }; template struct anvoke_handler: invoke_response_handler_base { anvoke_handler(const callback_t& cb, uint64_t timeout, async_protocol_handler& con, int command) - :m_cb(cb), m_con(con), m_timer(con.m_pservice_endpoint->get_io_service()), m_timer_started(false), + :m_cb(cb), m_timeout(timeout), m_con(con), m_timer(con.m_pservice_endpoint->get_io_service()), m_timer_started(false), m_cancel_timer_called(false), m_timer_cancelled(false), m_command(command) { if(m_con.start_outer_call()) { + MDEBUG(con.get_context_ref() << "anvoke_handler, timeout: " << timeout); m_timer.expires_from_now(boost::posix_time::milliseconds(timeout)); - m_timer.async_wait([&con, command, cb](const boost::system::error_code& ec) + m_timer.async_wait([this](const boost::system::error_code& ec) { - if(ec == boost::asio::error::operation_aborted) - return; - MINFO(con.get_context_ref() << "Timeout on invoke operation happened, command: " << command); - std::string fake; - cb(LEVIN_ERROR_CONNECTION_TIMEDOUT, fake, con.get_context_ref()); - con.close(); - con.finish_outer_call(); + timeout_handler(ec); }); m_timer_started = true; } @@ -171,7 +172,18 @@ public: bool m_timer_started; bool m_cancel_timer_called; bool m_timer_cancelled; + uint64_t m_timeout; int m_command; + virtual void timeout_handler(const boost::system::error_code& error) + { + if(error == boost::asio::error::operation_aborted) + return; + MINFO(m_con.get_context_ref() << "Timeout on invoke operation happened, command: " << m_command << " timeout: " << m_timeout); + std::string fake; + m_cb(LEVIN_ERROR_CONNECTION_TIMEDOUT, fake, m_con.get_context_ref()); + m_con.close(); + m_con.finish_outer_call(); + } virtual bool handle(int res, const std::string& buff, typename async_protocol_handler::connection_context& context) { if(!cancel_timer()) @@ -203,6 +215,18 @@ public: } return m_timer_cancelled; } + virtual void reset_timer() + { + boost::system::error_code ignored_ec; + if (!m_cancel_timer_called && m_timer.cancel(ignored_ec) > 0) + { + m_timer.expires_from_now(boost::posix_time::milliseconds(m_timeout)); + m_timer.async_wait([this](const boost::system::error_code& ec) + { + timeout_handler(ec); + }); + } + } }; critical_section m_invoke_response_handlers_lock; std::list > m_invoke_response_handlers; @@ -342,6 +366,13 @@ public: if(m_cache_in_buffer.size() < m_current_head.m_cb) { is_continue = false; + if(cb >= MIN_BYTES_WANTED && !m_invoke_response_handlers.empty()) + { + //async call scenario + boost::shared_ptr response_handler = m_invoke_response_handlers.front(); + response_handler->reset_timer(); + MDEBUG(m_connection_context << "LEVIN_PACKET partial msg received. len=" << cb); + } break; } { @@ -595,9 +626,15 @@ public: << ", ver=" << head.m_protocol_version); uint64_t ticks_start = misc_utils::get_tick_count(); + size_t prev_size = 0; while(!boost::interprocess::ipcdetail::atomic_read32(&m_invoke_buf_ready) && !m_deletion_initiated && !m_protocol_released) { + if(m_cache_in_buffer.size() - prev_size >= MIN_BYTES_WANTED) + { + prev_size = m_cache_in_buffer.size(); + ticks_start = misc_utils::get_tick_count(); + } if(misc_utils::get_tick_count() - ticks_start > m_config.m_invoke_timeout) { MWARNING(m_connection_context << "invoke timeout (" << m_config.m_invoke_timeout << "), closing connection "); From 07c4276cbebe453f552dd107a72d7231579dea57 Mon Sep 17 00:00:00 2001 From: Howard Chu Date: Sun, 4 Jun 2017 22:37:53 +0100 Subject: [PATCH 3/3] Don't issue a new timedsync while one is already in progress A timedsync is issued every minute on a connection, but the input tineout is 2 minutes. This means a new sync request could be issued while a slow sync request was already in progress. The additional request will further clog the network on a slow connection, and cause a premature timeout. --- contrib/epee/include/net/net_utils_base.h | 3 +++ src/p2p/net_node.inl | 8 ++++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/contrib/epee/include/net/net_utils_base.h b/contrib/epee/include/net/net_utils_base.h index 4334029f..fd7e2884 100644 --- a/contrib/epee/include/net/net_utils_base.h +++ b/contrib/epee/include/net/net_utils_base.h @@ -56,6 +56,7 @@ namespace net_utils const uint32_t m_remote_port; const bool m_is_income; const time_t m_started; + bool m_in_timedsync; time_t m_last_recv; time_t m_last_send; uint64_t m_recv_cnt; @@ -72,6 +73,7 @@ namespace net_utils m_remote_port(remote_port), m_is_income(is_income), m_started(time(NULL)), + m_in_timedsync(false), m_last_recv(last_recv), m_last_send(last_send), m_recv_cnt(recv_cnt), @@ -85,6 +87,7 @@ namespace net_utils m_remote_port(0), m_is_income(false), m_started(time(NULL)), + m_in_timedsync(false), m_last_recv(0), m_last_send(0), m_recv_cnt(0), diff --git a/src/p2p/net_node.inl b/src/p2p/net_node.inl index 5c903b1f..0d0d86e1 100644 --- a/src/p2p/net_node.inl +++ b/src/p2p/net_node.inl @@ -814,6 +814,7 @@ namespace nodetool bool r = epee::net_utils::async_invoke_remote_command2(context_.m_connection_id, COMMAND_TIMED_SYNC::ID, arg, m_net_server.get_config_object(), [this](int code, const typename COMMAND_TIMED_SYNC::response& rsp, p2p_connection_context& context) { + context.m_in_timedsync = false; if(code < 0) { LOG_ERROR_CC(context, "COMMAND_TIMED_SYNC invoke failed. (" << code << ", " << epee::levin::get_err_descr(code) << ")"); @@ -1300,10 +1301,13 @@ namespace nodetool MDEBUG("STARTED PEERLIST IDLE HANDSHAKE"); typedef std::list > local_connects_type; local_connects_type cncts; - m_net_server.get_config_object().foreach_connection([&](const p2p_connection_context& cntxt) + m_net_server.get_config_object().foreach_connection([&](p2p_connection_context& cntxt) { - if(cntxt.peer_id) + if(cntxt.peer_id && !cntxt.m_in_timedsync) + { + cntxt.m_in_timedsync = true; cncts.push_back(local_connects_type::value_type(cntxt, cntxt.peer_id));//do idle sync only with handshaked connections + } return true; });