From 7450656bf3a7aa3b4cb2732cc44f293ebc5080ef Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sun, 13 Mar 2016 17:53:27 +0000 Subject: [PATCH 1/3] net_node: fix connection leak when ping fails with bad response If there is no comms error, but the response is not as expected, close would not be called. --- src/p2p/net_node.inl | 1 + 1 file changed, 1 insertion(+) diff --git a/src/p2p/net_node.inl b/src/p2p/net_node.inl index 6278db891..0fab40322 100644 --- a/src/p2p/net_node.inl +++ b/src/p2p/net_node.inl @@ -1319,6 +1319,7 @@ namespace nodetool if(rsp.status != PING_OK_RESPONSE_STATUS_TEXT || pr != rsp.peer_id) { LOG_PRINT_CC_L2(ping_context, "back ping invoke wrong response \"" << rsp.status << "\" from" << ip << ":" << port << ", hsh_peer_id=" << pr_ << ", rsp.peer_id=" << rsp.peer_id); + m_net_server.get_config_object().close(ping_context.m_connection_id); return; } m_net_server.get_config_object().close(ping_context.m_connection_id); From 6bca9a8ef435b6b8a5dca917ec848c076c3a971e Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sun, 17 Apr 2016 11:45:38 +0100 Subject: [PATCH 2/3] abstract_tcp_server2: avoid deadlock waiting for send queue to drain If we reach the send queue size limit, we need to release the lock, or we will deadlock and it will never drain. If we reach that limit, it's likely there's another problem in the first place though, so it will probably not drain in practice either, unless some kind of transient network timeout. --- contrib/epee/include/net/abstract_tcp_server2.inl | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/contrib/epee/include/net/abstract_tcp_server2.inl b/contrib/epee/include/net/abstract_tcp_server2.inl index b3d4e5fdb..416a24739 100644 --- a/contrib/epee/include/net/abstract_tcp_server2.inl +++ b/contrib/epee/include/net/abstract_tcp_server2.inl @@ -490,7 +490,9 @@ PRAGMA_WARNING_DISABLE_VS(4355) sleep_before_packet(cb, 1, 1); } - epee::critical_region_t send_guard(m_send_que_lock); // *** critical *** + m_send_que_lock.lock(); // *** critical *** + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = epee::misc_utils::create_scope_leave_handler([&](){m_send_que_lock.unlock();}); + long int retry=0; const long int retry_limit = 5*4; while (m_send_que.size() > ABSTRACT_SERVER_SEND_QUE_MAX_COUNT) @@ -504,11 +506,12 @@ PRAGMA_WARNING_DISABLE_VS(4355) long int ms = 250 + (rand()%50); _info_c("net/sleep", "Sleeping because QUEUE is FULL, in " << __FUNCTION__ << " for " << ms << " ms before packet_size="< retry_limit) { - send_guard.unlock(); _erro("send que size is more than ABSTRACT_SERVER_SEND_QUE_MAX_COUNT(" << ABSTRACT_SERVER_SEND_QUE_MAX_COUNT << "), shutting down connection"); // _dbg1_c("net/sleep", "send que size is more than ABSTRACT_SERVER_SEND_QUE_MAX_COUNT(" << ABSTRACT_SERVER_SEND_QUE_MAX_COUNT << "), shutting down connection"); close(); From 3102feb56c2274313e40d1d13cb2761f7796bb9b Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sun, 17 Apr 2016 11:47:22 +0100 Subject: [PATCH 3/3] abstract_tcp_server2: fix send queue limit warning spam When the send queue limit is reached, it is likely to not drain any time soon. If we call close on the connection, it will stay alive, waiting for the queue to drain before actually closing, and will hit that check again and again. Since the queue size limit is the reason we're closing in the first place, we call shutdown directly. --- contrib/epee/include/net/abstract_tcp_server2.inl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/epee/include/net/abstract_tcp_server2.inl b/contrib/epee/include/net/abstract_tcp_server2.inl index 416a24739..ca9429c91 100644 --- a/contrib/epee/include/net/abstract_tcp_server2.inl +++ b/contrib/epee/include/net/abstract_tcp_server2.inl @@ -514,7 +514,7 @@ PRAGMA_WARNING_DISABLE_VS(4355) if (retry > retry_limit) { _erro("send que size is more than ABSTRACT_SERVER_SEND_QUE_MAX_COUNT(" << ABSTRACT_SERVER_SEND_QUE_MAX_COUNT << "), shutting down connection"); // _dbg1_c("net/sleep", "send que size is more than ABSTRACT_SERVER_SEND_QUE_MAX_COUNT(" << ABSTRACT_SERVER_SEND_QUE_MAX_COUNT << "), shutting down connection"); - close(); + shutdown(); return false; } }