From 7f90a241b668830087f3e9e681081a9dd05fd683 Mon Sep 17 00:00:00 2001 From: Arvid Norberg Date: Sat, 21 Jan 2012 09:05:34 +0000 Subject: [PATCH] don't let hung outgoing connection attempts block incoming connections --- ChangeLog | 1 + include/libtorrent/peer_connection.hpp | 2 +- include/libtorrent/torrent.hpp | 6 +++ src/peer_connection.cpp | 74 +++++++++++++++++++++++--- src/torrent.cpp | 24 ++++++++- 5 files changed, 96 insertions(+), 11 deletions(-) diff --git a/ChangeLog b/ChangeLog index 1823cb7d7..852516ab3 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,4 @@ + * don't let hung outgoing connection attempts block incoming connections * improve SSL torrent support by using SNI and a single SSL listen socket * improved peer exchange performance by sharing incoming connections which advertize listen port * deprecate set_ratio(), and per-peer rate limits diff --git a/include/libtorrent/peer_connection.hpp b/include/libtorrent/peer_connection.hpp index 00836fb57..f73a081ac 100644 --- a/include/libtorrent/peer_connection.hpp +++ b/include/libtorrent/peer_connection.hpp @@ -1164,7 +1164,7 @@ namespace libtorrent // buffer messages up in the application layer send // buffer, and send it once we're uncorked. bool m_corked:1; - + template struct handler_storage { diff --git a/include/libtorrent/torrent.hpp b/include/libtorrent/torrent.hpp index 560a203cf..1258f5e51 100644 --- a/include/libtorrent/torrent.hpp +++ b/include/libtorrent/torrent.hpp @@ -955,6 +955,12 @@ namespace libtorrent public: #endif std::set m_connections; + // of all peers in m_connections, this is the number + // of peers that are outgoing and still waiting to + // complete the connection. This is used to possibly + // kick out these connections when we get incoming + // connections (if we've reached the connection limit) + int m_num_connecting; #ifdef TORRENT_DEBUG private: #endif diff --git a/src/peer_connection.cpp b/src/peer_connection.cpp index 0d7cab01f..d53ca628a 100644 --- a/src/peer_connection.cpp +++ b/src/peer_connection.cpp @@ -200,6 +200,14 @@ namespace libtorrent , m_received_in_piece(0) #endif { + boost::shared_ptr t = m_torrent.lock(); + // if t is NULL, we better not be connecting, since + // we can't decrement the connecting counter + TORRENT_ASSERT(t || !m_connecting); + if (m_connecting && t) + { + ++t->m_num_connecting; + } m_est_reciprocation_rate = m_ses.m_settings.default_est_reciprocation_rate; #if TORRENT_USE_I2P @@ -347,6 +355,14 @@ namespace libtorrent , m_received_in_piece(0) #endif { + boost::shared_ptr t = m_torrent.lock(); + // if t is NULL, we better not be connecting, since + // we can't decrement the connecting counter + TORRENT_ASSERT(t || !m_connecting); + if (m_connecting && t) + { + ++t->m_num_connecting; + } m_est_reciprocation_rate = m_ses.m_settings.default_est_reciprocation_rate; #if TORRENT_USE_I2P @@ -910,6 +926,22 @@ namespace libtorrent TORRENT_ASSERT(m_disconnecting); TORRENT_ASSERT(m_disconnect_started); + // defensive + + boost::shared_ptr t = m_torrent.lock(); + // if t is NULL, we better not be connecting, since + // we can't decrement the connecting counter + TORRENT_ASSERT(t || !m_connecting); + + // we should really have dealt with this already + TORRENT_ASSERT(!m_connecting); + if (m_connecting && t) + { + TORRENT_ASSERT(t->m_num_connecting > 0); + --t->m_num_connecting; + m_connecting = false; + } + m_disk_recv_buffer_size = 0; #ifndef TORRENT_DISABLE_EXTENSIONS @@ -928,8 +960,6 @@ namespace libtorrent TORRENT_ASSERT(!i->second->has_peer(this)); if (m_peer_info) TORRENT_ASSERT(m_peer_info->connection == 0); - - boost::shared_ptr t = m_torrent.lock(); #endif } @@ -3396,10 +3426,17 @@ namespace libtorrent TORRENT_ASSERT(m_connecting); + boost::shared_ptr t = m_torrent.lock(); + if (m_connecting) + { + TORRENT_ASSERT(t->m_num_connecting > 0); + --t->m_num_connecting; + m_connecting = false; + } + if (m_connection_ticket != -1) { m_ses.m_half_open.done(m_connection_ticket); - m_connecting = false; } // a connection attempt using uTP just failed @@ -3523,13 +3560,19 @@ namespace libtorrent m_channel_state[download_channel] &= ~peer_info::bw_disk; } - if (m_connecting && m_connection_ticket >= 0) + boost::shared_ptr t = m_torrent.lock(); + if (m_connecting) + { + TORRENT_ASSERT(t->m_num_connecting > 0); + --t->m_num_connecting; + m_connecting = false; + } + if (m_connection_ticket >= 0) { m_ses.m_half_open.done(m_connection_ticket); m_connection_ticket = -1; } - boost::shared_ptr t = m_torrent.lock(); torrent_handle handle; if (t) handle = t->get_handle(); @@ -4052,7 +4095,14 @@ namespace libtorrent if (!t || m_disconnecting) { m_ses.m_half_open.done(m_connection_ticket); - m_connecting = false; + if (m_connection_ticket >= -1) m_connection_ticket = -1; + TORRENT_ASSERT(t || !m_connecting); + if (m_connecting && t) + { + TORRENT_ASSERT(t->m_num_connecting > 0); + --t->m_num_connecting; + m_connecting = false; + } disconnect(errors::torrent_aborted); return; } @@ -5427,7 +5477,16 @@ namespace libtorrent return; } - m_connecting = false; + // if t is NULL, we better not be connecting, since + // we can't decrement the connecting counter + boost::shared_ptr t = m_torrent.lock(); + TORRENT_ASSERT(t || !m_connecting); + if (m_connecting && t) + { + TORRENT_ASSERT(t->m_num_connecting > 0); + --t->m_num_connecting; + m_connecting = false; + } m_ses.m_half_open.done(m_connection_ticket); if (m_disconnecting) return; @@ -5469,7 +5528,6 @@ namespace libtorrent { // if the remote endpoint is the same as the local endpoint, we're connected // to ourselves - boost::shared_ptr t = m_torrent.lock(); if (m_peer_info && t) t->get_policy().ban_peer(m_peer_info); disconnect(errors::self_connection, 1); return; diff --git a/src/torrent.cpp b/src/torrent.cpp index 1b5a15b77..fd8a1fb72 100644 --- a/src/torrent.cpp +++ b/src/torrent.cpp @@ -352,6 +352,7 @@ namespace libtorrent , m_total_downloaded(0) , m_started(time_now()) , m_storage(0) + , m_num_connecting(0) , m_tracker_timer(ses.m_io_service) , m_ses(ses) , m_trackerid(p.trackerid) @@ -5649,8 +5650,27 @@ namespace libtorrent if (m_connections.size() >= m_max_connections) { - p->disconnect(errors::too_many_connections); - return false; + // if more than 10% of the connections are outgoing + // connection attempts that haven't completed yet, + // disconnect one of them and let this incoming + // connection through. + if (m_num_connecting < m_max_connections / 10) + { + p->disconnect(errors::too_many_connections); + return false; + } + + // find one of the connecting peers and disconnect it + // TODO: ideally, we would disconnect the oldest connection + // i.e. the one that has waited the longest to connect. + for (std::set::iterator i = m_connections.begin() + , end(m_connections.end()); i != end; ++i) + { + peer_connection* p = *i; + if (!p->is_connecting()) continue; + p->disconnect(errors::too_many_connections); + break; + } } TORRENT_TRY