From fc74c032f0c54c7c900ccd879c76b9135bce98af Mon Sep 17 00:00:00 2001 From: arvidn Date: Wed, 8 Nov 2017 11:44:56 +0100 Subject: [PATCH] minor cleanup and modernization of torrent_peer --- include/libtorrent/aux_/session_impl.hpp | 8 +++- include/libtorrent/peer_list.hpp | 37 ++++++---------- include/libtorrent/torrent_peer.hpp | 7 +++- include/libtorrent/torrent_peer_allocator.hpp | 23 ++++++---- src/peer_list.cpp | 42 +++++-------------- src/torrent_peer.cpp | 7 ++-- src/torrent_peer_allocator.cpp | 18 ++------ 7 files changed, 58 insertions(+), 84 deletions(-) diff --git a/include/libtorrent/aux_/session_impl.hpp b/include/libtorrent/aux_/session_impl.hpp index 413befd03..1808d69f4 100644 --- a/include/libtorrent/aux_/session_impl.hpp +++ b/include/libtorrent/aux_/session_impl.hpp @@ -249,8 +249,8 @@ namespace aux { #if TORRENT_USE_INVARIANT_CHECKS friend class libtorrent::invariant_access; #endif - typedef std::set> connection_map; - typedef std::unordered_map> torrent_map; + using connection_map = std::set>; + using torrent_map = std::unordered_map>; explicit session_impl(io_service& ios); ~session_impl() override; @@ -834,6 +834,10 @@ namespace aux { resolver m_host_resolver; tracker_manager m_tracker_manager; + + // the torrents must be destructed after the torrent_peer_allocator, + // since the torrents hold the peer lists that own the torrent_peers + // (which are allocated in the torrent_peer_allocator) torrent_map m_torrents; // all torrents that are downloading or queued, diff --git a/include/libtorrent/peer_list.hpp b/include/libtorrent/peer_list.hpp index e7d1d92c5..0a79d1104 100644 --- a/include/libtorrent/peer_list.hpp +++ b/include/libtorrent/peer_list.hpp @@ -61,41 +61,30 @@ namespace libtorrent { // the peer_list type not depend on the torrent type directly. struct torrent_state { - torrent_state() - : is_paused(false) - , is_finished(false) - , allow_multiple_connections_per_ip(false) - , first_time_seen(false) - , max_peerlist_size(1000) - , min_reconnect_time(60) - , loop_counter(0) - , port(0) - , max_failcount(3) - {} - bool is_paused; - bool is_finished; - bool allow_multiple_connections_per_ip; + bool is_paused = false; + bool is_finished = false; + bool allow_multiple_connections_per_ip = false; // this is set by peer_list::add_peer to either true or false // true means the peer we just added was new, false means // we already knew about the peer - bool first_time_seen; + bool first_time_seen = false; - int max_peerlist_size; - int min_reconnect_time; + int max_peerlist_size = 1000; + int min_reconnect_time = 60; // the number of iterations over the peer list for this operation - int loop_counter; + int loop_counter = 0; // these are used only by find_connect_candidates in order // to implement peer ranking. See: // http://blog.libtorrent.org/2012/12/swarm-connectivity/ external_ip ip; - int port; + int port = 0; // the number of times a peer must fail before it's no longer considered // a connect candidate - int max_failcount; + int max_failcount = 3; // if any peer were removed during this call, they are returned in // this vector. The caller would want to make sure there are no @@ -110,6 +99,10 @@ namespace libtorrent { explicit peer_list(torrent_peer_allocator_interface& alloc); ~peer_list(); + // not copyable + peer_list(peer_list const&) = delete; + peer_list& operator=(peer_list const&) = delete; + #if TORRENT_USE_I2P torrent_peer* add_i2p_peer(string_view destination , peer_source_flags_t src, char flags @@ -206,10 +199,6 @@ namespace libtorrent { private: - // not copyable - peer_list(peer_list const&); - peer_list& operator=(peer_list const&); - void recalculate_connect_candidates(torrent_state* state); void update_connect_candidates(int delta); diff --git a/include/libtorrent/torrent_peer.hpp b/include/libtorrent/torrent_peer.hpp index 348cef666..99cf9d12c 100644 --- a/include/libtorrent/torrent_peer.hpp +++ b/include/libtorrent/torrent_peer.hpp @@ -54,6 +54,11 @@ namespace libtorrent { struct TORRENT_EXTRA_EXPORT torrent_peer { torrent_peer(std::uint16_t port, bool connectable, peer_source_flags_t src); +#if TORRENT_USE_ASSERTS + torrent_peer(torrent_peer const&) = default; + torrent_peer& operator=(torrent_peer const&) = default; + ~torrent_peer() { in_use = false; } +#endif std::int64_t total_download() const; std::int64_t total_upload() const; @@ -199,7 +204,7 @@ namespace libtorrent { // never considered a connect candidate bool web_seed:1; #if TORRENT_USE_ASSERTS - bool in_use:1; + bool in_use = true; #endif }; diff --git a/include/libtorrent/torrent_peer_allocator.hpp b/include/libtorrent/torrent_peer_allocator.hpp index fcf99a253..dd8dfa34c 100644 --- a/include/libtorrent/torrent_peer_allocator.hpp +++ b/include/libtorrent/torrent_peer_allocator.hpp @@ -62,7 +62,11 @@ namespace libtorrent { struct TORRENT_EXTRA_EXPORT torrent_peer_allocator final : torrent_peer_allocator_interface { - torrent_peer_allocator(); +#if TORRENT_USE_ASSERTS + ~torrent_peer_allocator() { + m_in_use = false; + } +#endif torrent_peer* allocate_peer_entry(int type) override; void free_peer_entry(torrent_peer* p) override; @@ -79,22 +83,25 @@ namespace libtorrent { // to have tens of thousands of peers, and a pool // saves significant overhead - boost::pool<> m_ipv4_peer_pool; + boost::pool<> m_ipv4_peer_pool{sizeof(libtorrent::ipv4_peer), 500}; #if TORRENT_USE_IPV6 - boost::pool<> m_ipv6_peer_pool; + boost::pool<> m_ipv6_peer_pool{sizeof(libtorrent::ipv6_peer), 500}; #endif #if TORRENT_USE_I2P - boost::pool<> m_i2p_peer_pool; + boost::pool<> m_i2p_peer_pool{sizeof(libtorrent::i2p_peer), 500}; #endif // the total number of bytes allocated (cumulative) - std::uint64_t m_total_bytes; + std::uint64_t m_total_bytes = 0; // the total number of allocations (cumulative) - std::uint64_t m_total_allocations; + std::uint64_t m_total_allocations = 0; // the number of currently live bytes - int m_live_bytes; + int m_live_bytes = 0; // the number of currently live allocations - int m_live_allocations; + int m_live_allocations = 0; +#if TORRENT_USE_ASSERTS + bool m_in_use = true; +#endif }; } diff --git a/src/peer_list.cpp b/src/peer_list.cpp index b4ba6988c..7c11ebac5 100644 --- a/src/peer_list.cpp +++ b/src/peer_list.cpp @@ -125,11 +125,8 @@ namespace libtorrent { peer_list::~peer_list() { - for (peers_t::iterator i = m_peers.begin() - , end(m_peers.end()); i != end; ++i) - { - m_peer_allocator.free_peer_entry(*i); - } + for (auto const p : m_peers) + m_peer_allocator.free_peer_entry(p); } void peer_list::set_max_failcount(torrent_state* state) @@ -295,11 +292,6 @@ namespace libtorrent { std::vector::iterator ci = std::find(m_candidate_cache.begin(), m_candidate_cache.end(), *i); if (ci != m_candidate_cache.end()) m_candidate_cache.erase(ci); -#if TORRENT_USE_ASSERTS - TORRENT_ASSERT((*i)->in_use); - (*i)->in_use = false; -#endif - m_peer_allocator.free_peer_entry(*i); m_peers.erase(i); } @@ -765,10 +757,6 @@ namespace libtorrent { #endif new (p) ipv4_peer(c.remote(), false, {}); -#if TORRENT_USE_ASSERTS - p->in_use = true; -#endif - iter = m_peers.insert(iter, p); if (m_round_robin >= iter - m_peers.begin()) ++m_round_robin; @@ -1036,16 +1024,8 @@ namespace libtorrent { if (p == nullptr) return nullptr; new (p) i2p_peer(destination, true, src); -#if TORRENT_USE_ASSERTS - p->in_use = true; -#endif - if (!insert_peer(p, iter, flags, state)) { -#if TORRENT_USE_ASSERTS - p->in_use = false; -#endif - m_peer_allocator.free_peer_entry(p); return nullptr; } @@ -1113,16 +1093,16 @@ namespace libtorrent { #endif new (p) ipv4_peer(remote, true, src); -#if TORRENT_USE_ASSERTS - p->in_use = true; -#endif - - if (!insert_peer(p, iter, flags, state)) + try + { + if (!insert_peer(p, iter, flags, state)) + { + m_peer_allocator.free_peer_entry(p); + return nullptr; + } + } + catch (std::exception const&) { -#if TORRENT_USE_ASSERTS - p->in_use = false; -#endif -// TODO: 3 this is not exception safe! m_peer_allocator.free_peer_entry(p); return nullptr; } diff --git a/src/torrent_peer.cpp b/src/torrent_peer.cpp index b5eba2b01..222964acb 100644 --- a/src/torrent_peer.cpp +++ b/src/torrent_peer.cpp @@ -177,13 +177,11 @@ namespace libtorrent { , confirmed_supports_utp(false) , supports_holepunch(false) , web_seed(false) -#if TORRENT_USE_ASSERTS - , in_use(false) -#endif {} std::uint32_t torrent_peer::rank(external_ip const& external, int external_port) const { + TORRENT_ASSERT(in_use); //TODO: how do we deal with our external address changing? if (peer_rank == 0) peer_rank = peer_priority( @@ -195,6 +193,7 @@ namespace libtorrent { #ifndef TORRENT_DISABLE_LOGGING std::string torrent_peer::to_string() const { + TORRENT_ASSERT(in_use); #if TORRENT_USE_I2P if (is_i2p_addr) return dest().to_string(); #endif // TORRENT_USE_I2P @@ -205,6 +204,7 @@ namespace libtorrent { std::int64_t torrent_peer::total_download() const { + TORRENT_ASSERT(in_use); if (connection != nullptr) { TORRENT_ASSERT(prev_amount_download == 0); @@ -218,6 +218,7 @@ namespace libtorrent { std::int64_t torrent_peer::total_upload() const { + TORRENT_ASSERT(in_use); if (connection != nullptr) { TORRENT_ASSERT(prev_amount_upload == 0); diff --git a/src/torrent_peer_allocator.cpp b/src/torrent_peer_allocator.cpp index 17e5cb16f..f71ded2db 100644 --- a/src/torrent_peer_allocator.cpp +++ b/src/torrent_peer_allocator.cpp @@ -37,23 +37,9 @@ POSSIBILITY OF SUCH DAMAGE. namespace libtorrent { - torrent_peer_allocator::torrent_peer_allocator() - : m_ipv4_peer_pool(sizeof(libtorrent::ipv4_peer), 500) -#if TORRENT_USE_IPV6 - , m_ipv6_peer_pool(sizeof(libtorrent::ipv6_peer), 500) -#endif -#if TORRENT_USE_I2P - , m_i2p_peer_pool(sizeof(libtorrent::i2p_peer), 500) -#endif - , m_total_bytes(0) - , m_total_allocations(0) - , m_live_bytes(0) - , m_live_allocations(0) - { - } - torrent_peer* torrent_peer_allocator::allocate_peer_entry(int type) { + TORRENT_ASSERT(m_in_use); torrent_peer* p = nullptr; switch(type) { @@ -94,6 +80,8 @@ namespace libtorrent { void torrent_peer_allocator::free_peer_entry(torrent_peer* p) { + TORRENT_ASSERT(m_in_use); + TORRENT_ASSERT(p->in_use); #if TORRENT_USE_IPV6 if (p->is_v6_addr) {