diff --git a/ChangeLog b/ChangeLog index 65b1f357e..89ca5a3c4 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,6 +1,7 @@ 1.1.5 release + * fix leak of torrent_peer objecs (entries in peer_list) * fix leak of peer_class objects (when setting per-torrent rate limits) * expose peer_class API to python binding * fix integer overflow in whole_pieces_threshold logic diff --git a/include/libtorrent/aux_/session_impl.hpp b/include/libtorrent/aux_/session_impl.hpp index c923dbbb5..e912963bc 100644 --- a/include/libtorrent/aux_/session_impl.hpp +++ b/include/libtorrent/aux_/session_impl.hpp @@ -207,8 +207,8 @@ namespace libtorrent void open_listen_port(); - torrent_peer_allocator_interface* get_peer_allocator() TORRENT_OVERRIDE - { return &m_peer_allocator; } + torrent_peer_allocator_interface& get_peer_allocator() TORRENT_OVERRIDE + { return m_peer_allocator; } io_service& get_io_service() TORRENT_OVERRIDE { return m_io_service; } resolver_interface& get_resolver() TORRENT_OVERRIDE { return m_host_resolver; } diff --git a/include/libtorrent/aux_/session_interface.hpp b/include/libtorrent/aux_/session_interface.hpp index fcd2c4317..13099336e 100644 --- a/include/libtorrent/aux_/session_interface.hpp +++ b/include/libtorrent/aux_/session_interface.hpp @@ -152,7 +152,7 @@ namespace libtorrent { namespace aux virtual alert_manager& alerts() = 0; - virtual torrent_peer_allocator_interface* get_peer_allocator() = 0; + virtual torrent_peer_allocator_interface& get_peer_allocator() = 0; virtual io_service& get_io_service() = 0; virtual resolver_interface& get_resolver() = 0; diff --git a/include/libtorrent/peer_list.hpp b/include/libtorrent/peer_list.hpp index ae7d9d010..2892d1b4c 100644 --- a/include/libtorrent/peer_list.hpp +++ b/include/libtorrent/peer_list.hpp @@ -70,7 +70,6 @@ namespace libtorrent , loop_counter(0) , ip(NULL), port(0) , max_failcount(3) - , peer_allocator(NULL) {} bool is_paused; bool is_finished; @@ -97,9 +96,6 @@ namespace libtorrent // a connect candidate int max_failcount; - // this must be set to a torrent_peer allocator - torrent_peer_allocator_interface* peer_allocator; - // if any peer were removed during this call, they are returned in // this vector. The caller would want to make sure there are no // references to these torrent_peers anywhere @@ -110,7 +106,8 @@ namespace libtorrent { public: - peer_list(); + peer_list(torrent_peer_allocator_interface& alloc); + ~peer_list(); #if TORRENT_USE_I2P torrent_peer* add_i2p_peer(char const* destination, int src, char flags @@ -211,6 +208,10 @@ 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); @@ -244,6 +245,10 @@ namespace libtorrent // if so, don't delete it. torrent_peer* m_locked_peer; + // the peer allocator, as stored from the constructor + // this must be available in the destructor to free all peers + torrent_peer_allocator_interface& m_peer_allocator; + // the number of seeds in the torrent_peer list boost::uint32_t m_num_seeds:31; diff --git a/src/peer_list.cpp b/src/peer_list.cpp index 12adb44a1..4c4167107 100644 --- a/src/peer_list.cpp +++ b/src/peer_list.cpp @@ -121,8 +121,9 @@ namespace namespace libtorrent { - peer_list::peer_list() + peer_list::peer_list(torrent_peer_allocator_interface& alloc) : m_locked_peer(NULL) + , m_peer_allocator(alloc) , m_num_seeds(0) , m_finished(0) , m_round_robin(0) @@ -132,6 +133,15 @@ namespace libtorrent thread_started(); } + 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); + } + } + void peer_list::set_max_failcount(torrent_state* state) { if (state->max_failcount == m_max_failcount) return; @@ -301,7 +311,7 @@ namespace libtorrent (*i)->in_use = false; #endif - state->peer_allocator->free_peer_entry(*i); + m_peer_allocator.free_peer_entry(*i); m_peers.erase(i); } @@ -745,7 +755,7 @@ namespace libtorrent #else bool is_v6 = false; #endif - torrent_peer* p = state->peer_allocator->allocate_peer_entry( + torrent_peer* p = m_peer_allocator.allocate_peer_entry( is_v6 ? torrent_peer_allocator_interface::ipv6_peer_type : torrent_peer_allocator_interface::ipv4_peer_type); if (p == 0) return false; @@ -1030,7 +1040,7 @@ namespace libtorrent { // we don't have any info about this peer. // add a new entry - p = state->peer_allocator->allocate_peer_entry(torrent_peer_allocator_interface::i2p_peer_type); + p = m_peer_allocator.allocate_peer_entry(torrent_peer_allocator_interface::i2p_peer_type); if (p == NULL) return NULL; new (p) i2p_peer(destination, true, src); @@ -1044,7 +1054,7 @@ namespace libtorrent p->in_use = false; #endif - state->peer_allocator->free_peer_entry(p); + m_peer_allocator.free_peer_entry(p); return 0; } } @@ -1106,7 +1116,7 @@ namespace libtorrent #else bool is_v6 = false; #endif - p = state->peer_allocator->allocate_peer_entry( + p = m_peer_allocator.allocate_peer_entry( is_v6 ? torrent_peer_allocator_interface::ipv6_peer_type : torrent_peer_allocator_interface::ipv4_peer_type); if (p == NULL) return NULL; @@ -1127,7 +1137,8 @@ namespace libtorrent #if TORRENT_USE_ASSERTS p->in_use = false; #endif - state->peer_allocator->free_peer_entry(p); + // TODO: 3 this is not exception safe! + m_peer_allocator.free_peer_entry(p); return 0; } state->first_time_seen = true; diff --git a/src/torrent.cpp b/src/torrent.cpp index 3d09f9162..63f8bec49 100644 --- a/src/torrent.cpp +++ b/src/torrent.cpp @@ -1172,7 +1172,7 @@ namespace libtorrent void torrent::need_peer_list() { if (m_peer_list) return; - m_peer_list.reset(new peer_list); + m_peer_list.reset(new peer_list(m_ses.get_peer_allocator())); } void torrent::handle_disk_error(disk_io_job const* j, peer_connection* c) @@ -11503,7 +11503,6 @@ namespace { : settings().get_int(settings_pack::max_peerlist_size); ret.min_reconnect_time = settings().get_int(settings_pack::min_reconnect_time); - ret.peer_allocator = m_ses.get_peer_allocator(); ret.ip = &m_ses.external_address(); ret.port = m_ses.listen_port(); ret.max_failcount = settings().get_int(settings_pack::max_failcount); diff --git a/test/test_peer_list.cpp b/test/test_peer_list.cpp index 5a051c4d4..ba3a36aeb 100644 --- a/test/test_peer_list.cpp +++ b/test/test_peer_list.cpp @@ -162,15 +162,13 @@ bool has_peer(peer_list const& p, tcp::endpoint const& ep) return its.first != its.second; } -torrent_state init_state(torrent_peer_allocator& allocator - , external_ip& ext_ip) +torrent_state init_state(external_ip& ext_ip) { torrent_state st; st.is_finished = false; st.is_paused = false; st.max_peerlist_size = 1000; st.allow_multiple_connections_per_ip = false; - st.peer_allocator = &allocator; st.ip = &ext_ip; st.port = 9999; return st; @@ -206,9 +204,9 @@ static external_ip ext_ip; // when disallowing it TORRENT_TEST(multiple_ips_disallowed) { - torrent_state st = init_state(allocator, ext_ip); + torrent_state st = init_state(ext_ip); mock_torrent t(&st); - peer_list p; + peer_list p(allocator); t.m_p = &p; TEST_EQUAL(p.num_connect_candidates(), 0); torrent_peer* peer1 = p.add_peer(ep("10.0.0.2", 3000), 0, 0, &st); @@ -228,10 +226,10 @@ TORRENT_TEST(multiple_ips_disallowed) // when allowing it TORRENT_TEST(multiple_ips_allowed) { - torrent_state st = init_state(allocator, ext_ip); + torrent_state st = init_state(ext_ip); mock_torrent t(&st); st.allow_multiple_connections_per_ip = true; - peer_list p; + peer_list p(allocator); t.m_p = &p; torrent_peer* peer1 = p.add_peer(ep("10.0.0.2", 3000), 0, 0, &st); TEST_EQUAL(p.num_connect_candidates(), 1); @@ -250,10 +248,10 @@ TORRENT_TEST(multiple_ips_allowed) // with allow_multiple_connections_per_ip enabled TORRENT_TEST(multiple_ips_allowed2) { - torrent_state st = init_state(allocator, ext_ip); + torrent_state st = init_state(ext_ip); mock_torrent t(&st); st.allow_multiple_connections_per_ip = true; - peer_list p; + peer_list p(allocator); t.m_p = &p; torrent_peer* peer1 = p.add_peer(ep("10.0.0.2", 3000), 0, 0, &st); TEST_EQUAL(p.num_connect_candidates(), 1); @@ -289,10 +287,10 @@ TORRENT_TEST(multiple_ips_allowed2) // with allow_multiple_connections_per_ip disabled TORRENT_TEST(multiple_ips_disallowed2) { - torrent_state st = init_state(allocator, ext_ip); + torrent_state st = init_state(ext_ip); mock_torrent t(&st); st.allow_multiple_connections_per_ip = false; - peer_list p; + peer_list p(allocator); t.m_p = &p; torrent_peer* peer1 = p.add_peer(ep("10.0.0.2", 3000), 0, 0, &st); TEST_EQUAL(p.num_connect_candidates(), 1); @@ -323,10 +321,10 @@ TORRENT_TEST(multiple_ips_disallowed2) // and update_peer_port TORRENT_TEST(update_peer_port) { - torrent_state st = init_state(allocator, ext_ip); + torrent_state st = init_state(ext_ip); mock_torrent t(&st); st.allow_multiple_connections_per_ip = false; - peer_list p; + peer_list p(allocator); t.m_p = &p; TEST_EQUAL(p.num_connect_candidates(), 0); boost::shared_ptr c @@ -347,10 +345,10 @@ TORRENT_TEST(update_peer_port) // and update_peer_port, causing collission TORRENT_TEST(update_peer_port_collide) { - torrent_state st = init_state(allocator, ext_ip); + torrent_state st = init_state(ext_ip); mock_torrent t(&st); st.allow_multiple_connections_per_ip = true; - peer_list p; + peer_list p(allocator); t.m_p = &p; torrent_peer* peer2 = p.add_peer(ep("10.0.0.1", 4000), 0, 0, &st); @@ -378,10 +376,10 @@ TORRENT_TEST(update_peer_port_collide) // test ip filter TORRENT_TEST(ip_filter) { - torrent_state st = init_state(allocator, ext_ip); + torrent_state st = init_state(ext_ip); mock_torrent t(&st); st.allow_multiple_connections_per_ip = false; - peer_list p; + peer_list p(allocator); t.m_p = &p; // add peer 1 @@ -418,10 +416,10 @@ TORRENT_TEST(ip_filter) // test port filter TORRENT_TEST(port_filter) { - torrent_state st = init_state(allocator, ext_ip); + torrent_state st = init_state(ext_ip); mock_torrent t(&st); st.allow_multiple_connections_per_ip = false; - peer_list p; + peer_list p(allocator); t.m_p = &p; // add peer 1 @@ -458,10 +456,10 @@ TORRENT_TEST(port_filter) // test banning peers TORRENT_TEST(ban_peers) { - torrent_state st = init_state(allocator, ext_ip); + torrent_state st = init_state(ext_ip); mock_torrent t(&st); st.allow_multiple_connections_per_ip = false; - peer_list p; + peer_list p(allocator); t.m_p = &p; torrent_peer* peer1 = add_peer(p, st, ep("10.0.0.1", 4000)); @@ -499,11 +497,11 @@ TORRENT_TEST(ban_peers) // test erase_peers when we fill up the peer list TORRENT_TEST(erase_peers) { - torrent_state st = init_state(allocator, ext_ip); + torrent_state st = init_state(ext_ip); mock_torrent t(&st); st.max_peerlist_size = 100; st.allow_multiple_connections_per_ip = true; - peer_list p; + peer_list p(allocator); t.m_p = &p; for (int i = 0; i < 100; ++i) @@ -532,11 +530,11 @@ TORRENT_TEST(erase_peers) // test set_ip_filter TORRENT_TEST(set_ip_filter) { - torrent_state st = init_state(allocator, ext_ip); + torrent_state st = init_state(ext_ip); std::vector
banned; mock_torrent t(&st); - peer_list p; + peer_list p(allocator); t.m_p = &p; for (int i = 0; i < 100; ++i) @@ -563,11 +561,11 @@ TORRENT_TEST(set_ip_filter) // test set_port_filter TORRENT_TEST(set_port_filter) { - torrent_state st = init_state(allocator, ext_ip); + torrent_state st = init_state(ext_ip); std::vector
banned; mock_torrent t(&st); - peer_list p; + peer_list p(allocator); t.m_p = &p; for (int i = 0; i < 100; ++i) @@ -594,10 +592,10 @@ TORRENT_TEST(set_port_filter) // test set_max_failcount TORRENT_TEST(set_max_failcount) { - torrent_state st = init_state(allocator, ext_ip); + torrent_state st = init_state(ext_ip); mock_torrent t(&st); - peer_list p; + peer_list p(allocator); t.m_p = &p; for (int i = 0; i < 100; ++i) @@ -624,10 +622,10 @@ TORRENT_TEST(set_max_failcount) // test set_seed TORRENT_TEST(set_seed) { - torrent_state st = init_state(allocator, ext_ip); + torrent_state st = init_state(ext_ip); mock_torrent t(&st); - peer_list p; + peer_list p(allocator); t.m_p = &p; for (int i = 0; i < 100; ++i) @@ -658,11 +656,11 @@ TORRENT_TEST(set_seed) // test has_peer TORRENT_TEST(has_peer) { - torrent_state st = init_state(allocator, ext_ip); + torrent_state st = init_state(ext_ip); std::vector
banned; mock_torrent t(&st); - peer_list p; + peer_list p(allocator); t.m_p = &p; torrent_peer* peer1 = add_peer(p, st, ep("10.10.0.1", 10)); @@ -691,11 +689,11 @@ TORRENT_TEST(has_peer) // test connect_candidates torrent_finish TORRENT_TEST(connect_candidates_finish) { - torrent_state st = init_state(allocator, ext_ip); + torrent_state st = init_state(ext_ip); std::vector
banned; mock_torrent t(&st); - peer_list p; + peer_list p(allocator); t.m_p = &p; torrent_peer* peer1 = add_peer(p, st, ep("10.10.0.1", 10)); @@ -730,10 +728,10 @@ TORRENT_TEST(connect_candidates_finish) // test self-connection TORRENT_TEST(self_connection) { - torrent_state st = init_state(allocator, ext_ip); + torrent_state st = init_state(ext_ip); mock_torrent t(&st); st.allow_multiple_connections_per_ip = false; - peer_list p; + peer_list p(allocator); t.m_p = &p; // add and connect peer @@ -762,10 +760,10 @@ TORRENT_TEST(self_connection) // test double connection (both incoming) TORRENT_TEST(double_connection) { - torrent_state st = init_state(allocator, ext_ip); + torrent_state st = init_state(ext_ip); mock_torrent t(&st); st.allow_multiple_connections_per_ip = false; - peer_list p; + peer_list p(allocator); t.m_p = &p; // we are 10.0.0.1 and the other peer is 10.0.0.2 @@ -792,10 +790,10 @@ TORRENT_TEST(double_connection) // test double connection (we loose) TORRENT_TEST(double_connection_loose) { - torrent_state st = init_state(allocator, ext_ip); + torrent_state st = init_state(ext_ip); mock_torrent t(&st); st.allow_multiple_connections_per_ip = false; - peer_list p; + peer_list p(allocator); t.m_p = &p; // we are 10.0.0.1 and the other peer is 10.0.0.2 @@ -823,10 +821,10 @@ TORRENT_TEST(double_connection_loose) // test double connection (we win) TORRENT_TEST(double_connection_win) { - torrent_state st = init_state(allocator, ext_ip); + torrent_state st = init_state(ext_ip); mock_torrent t(&st); st.allow_multiple_connections_per_ip = false; - peer_list p; + peer_list p(allocator); t.m_p = &p; // we are 10.0.0.1 and the other peer is 10.0.0.2 @@ -854,11 +852,11 @@ TORRENT_TEST(double_connection_win) // test incoming connection when we are at the list size limit TORRENT_TEST(incoming_size_limit) { - torrent_state st = init_state(allocator, ext_ip); + torrent_state st = init_state(ext_ip); st.max_peerlist_size = 5; mock_torrent t(&st); st.allow_multiple_connections_per_ip = false; - peer_list p; + peer_list p(allocator); t.m_p = &p; torrent_peer* peer1 = add_peer(p, st, ep("10.0.0.1", 8080)); @@ -900,11 +898,11 @@ TORRENT_TEST(incoming_size_limit) // test new peer when we are at the list size limit TORRENT_TEST(new_peer_size_limit) { - torrent_state st = init_state(allocator, ext_ip); + torrent_state st = init_state(ext_ip); st.max_peerlist_size = 5; mock_torrent t(&st); st.allow_multiple_connections_per_ip = false; - peer_list p; + peer_list p(allocator); t.m_p = &p; torrent_peer* peer1 = add_peer(p, st, ep("10.0.0.1", 8080));