From 3286437a7d9ff2286b4edbf13c76c3c480d946db Mon Sep 17 00:00:00 2001 From: Arvid Norberg Date: Tue, 20 Jan 2015 23:56:45 +0000 Subject: [PATCH] extend peer_list unit test and fix some bugs --- include/libtorrent/peer_list.hpp | 6 +- src/peer_list.cpp | 13 +-- test/test_peer_list.cpp | 136 ++++++++++++++++++++++++++++--- 3 files changed, 138 insertions(+), 17 deletions(-) diff --git a/include/libtorrent/peer_list.hpp b/include/libtorrent/peer_list.hpp index 5b5eaf82b..ec89c1c38 100644 --- a/include/libtorrent/peer_list.hpp +++ b/include/libtorrent/peer_list.hpp @@ -145,8 +145,10 @@ namespace libtorrent void set_failcount(torrent_peer* p, int f); void inc_failcount(torrent_peer* p); - void apply_ip_filter(ip_filter const& filter, torrent_state* state, std::vector
& banned); - void apply_port_filter(port_filter const& filter, torrent_state* state, std::vector
& banned); + void apply_ip_filter(ip_filter const& filter, torrent_state* state + , std::vector
& banned); + void apply_port_filter(port_filter const& filter, torrent_state* state + , std::vector
& banned); void set_seed(torrent_peer* p, bool s); diff --git a/src/peer_list.cpp b/src/peer_list.cpp index 410415eec..239bcb377 100644 --- a/src/peer_list.cpp +++ b/src/peer_list.cpp @@ -138,7 +138,6 @@ namespace libtorrent { if (state->max_failcount == m_max_failcount) return; - m_max_failcount = state->max_failcount; recalculate_connect_candidates(state); } @@ -348,7 +347,7 @@ namespace libtorrent int erase_candidate = -1; int force_erase_candidate = -1; - if (state->is_finished != m_finished) + if (m_finished != state->is_finished) recalculate_connect_candidates(state); int round_robin = random() % m_peers.size(); @@ -1253,18 +1252,22 @@ namespace libtorrent void peer_list::recalculate_connect_candidates(torrent_state* state) { TORRENT_ASSERT(is_single_thread()); - INVARIANT_CHECK; - - if (state->is_finished == m_finished) return; m_num_connect_candidates = 0; m_finished = state->is_finished; + m_max_failcount = state->max_failcount; for (const_iterator i = m_peers.begin(); i != m_peers.end(); ++i) { m_num_connect_candidates += is_connect_candidate(**i); } + +#if TORRENT_USE_INVARIANT_CHECKS + // the invariant is not likely to be upheld at the entry of this function + // but it is likely to have been restored by the end of it + check_invariant(); +#endif } #if TORRENT_USE_ASSERTS diff --git a/test/test_peer_list.cpp b/test/test_peer_list.cpp index 0568b05c2..ef19c5361 100644 --- a/test/test_peer_list.cpp +++ b/test/test_peer_list.cpp @@ -413,27 +413,143 @@ int test_main() // trigger the eviction of one peer torrent_peer* peer = p.add_peer(rand_tcp_ep(), 0, 0, &st); // we either removed an existing peer, or rejected this one + // either is valid behavior when the list is full TEST_CHECK(st.erased.size() == 1 || peer == NULL); } -// TODO: test applying a port_filter + // test set_ip_filter + { + std::vector
banned; + st.erased.clear(); + + mock_torrent t; + peer_list p; + t.m_p = &p; + + for (int i = 0; i < 100; ++i) + { + p.add_peer(tcp::endpoint( + address_v4((10 << 24) + ((i + 10) << 16)), 353), 0, 0, &st); + TEST_EQUAL(st.erased.size(), 0); + st.erased.clear(); + } + TEST_EQUAL(p.num_peers(), 100); + TEST_EQUAL(p.num_connect_candidates(), 100); + + // trigger the removal of one peer + ip_filter filter; + filter.add_rule(address_v4::from_string("10.13.0.0") + , address_v4::from_string("10.13.255.255"), ip_filter::blocked); + p.apply_ip_filter(filter, &st, banned); + TEST_EQUAL(st.erased.size(), 1); + TEST_EQUAL(st.erased[0]->address(), address_v4::from_string("10.13.0.0")); + TEST_EQUAL(p.num_peers(), 99); + TEST_EQUAL(p.num_connect_candidates(), 99); + } + + // test set_port_filter + { + std::vector
banned; + st.erased.clear(); + + mock_torrent t; + peer_list p; + t.m_p = &p; + + for (int i = 0; i < 100; ++i) + { + p.add_peer(tcp::endpoint( + address_v4((10 << 24) + ((i + 10) << 16)), i + 10), 0, 0, &st); + TEST_EQUAL(st.erased.size(), 0); + st.erased.clear(); + } + TEST_EQUAL(p.num_peers(), 100); + TEST_EQUAL(p.num_connect_candidates(), 100); + + // trigger the removal of one peer + port_filter filter; + filter.add_rule(13, 13, port_filter::blocked); + p.apply_port_filter(filter, &st, banned); + TEST_EQUAL(st.erased.size(), 1); + TEST_EQUAL(st.erased[0]->address(), address_v4::from_string("10.13.0.0")); + TEST_EQUAL(st.erased[0]->port, 13); + TEST_EQUAL(p.num_peers(), 99); + TEST_EQUAL(p.num_connect_candidates(), 99); + } + + // test set_max_failcount + { + st.erased.clear(); + + mock_torrent t; + peer_list p; + t.m_p = &p; + + for (int i = 0; i < 100; ++i) + { + torrent_peer* peer = p.add_peer(tcp::endpoint( + address_v4((10 << 24) + ((i + 10) << 16)), i + 10), 0, 0, &st); + TEST_EQUAL(st.erased.size(), 0); + st.erased.clear(); + // every other peer has a failcount of 1 + if (i % 2) p.inc_failcount(peer); + } + TEST_EQUAL(p.num_peers(), 100); + TEST_EQUAL(p.num_connect_candidates(), 100); + + // set the max failcount to 1 and observe how half the peers no longer + // are connect candidates + st.max_failcount = 1; + p.set_max_failcount(&st); + + TEST_EQUAL(p.num_connect_candidates(), 50); + TEST_EQUAL(p.num_peers(), 100); + } + + // test set_seed + { + st.erased.clear(); + + mock_torrent t; + peer_list p; + t.m_p = &p; + + for (int i = 0; i < 100; ++i) + { + torrent_peer* peer = p.add_peer(tcp::endpoint( + address_v4((10 << 24) + ((i + 10) << 16)), i + 10), 0, 0, &st); + TEST_EQUAL(st.erased.size(), 0); + st.erased.clear(); + // make every other peer a seed + if (i % 2) p.set_seed(peer, true); + } + TEST_EQUAL(p.num_peers(), 100); + TEST_EQUAL(p.num_connect_candidates(), 100); + + // now, the torrent completes and we're no longer interested in + // connecting to seeds. Make sure half the peers are no longer + // considered connect candidates + st.is_finished = true; + + // this will make the peer_list recalculate the connect candidates + std::vector peers; + torrent_peer* peer = p.connect_one_peer(1, &st); + + TEST_EQUAL(p.num_connect_candidates(), 50); + TEST_EQUAL(p.num_peers(), 100); + } + // TODO: test erasing peers -// TODO: test using port and ip filter -// TODO: test incrementing failcount (and make sure we no longer consider the peer a connect canidate) -// TODO: test max peerlist size -// TODO: test logic for which connection to keep when receiving an incoming connection to the same peer as we just made an outgoing connection to +// TODO: test logic for which connection to keep when receiving an incoming +// connection to the same peer as we just made an outgoing connection to // TODO: test update_peer_port with allow_multiple_connections_per_ip -// TODO: test set_seed // TODO: test has_peer -// TODO: test insert_peer with a full list // TODO: test add i2p peers // TODO: test allow_i2p_mixed -// TODO: test insert_peer failing +// TODO: test insert_peer failing with all error conditions // TODO: test IPv6 // TODO: test connect_to_peer() failing // TODO: test connection_closed -// TODO: test recalculate connect candidates -// TODO: add tests here return 0; }