From e526355d245b67da4a8413dd77add9126bf4b414 Mon Sep 17 00:00:00 2001 From: Arvid Norberg Date: Sun, 9 Nov 2014 11:17:13 +0000 Subject: [PATCH] merged changes from RC_1_0 --- ChangeLog | 1 + Jamfile | 4 +-- src/bt_peer_connection.cpp | 2 +- src/peer_connection.cpp | 51 +++++++++++++++++--------------------- src/settings_pack.cpp | 2 +- src/torrent.cpp | 8 +++++- test/Jamfile | 3 +-- test/test_priority.cpp | 51 +++++++++++++++++++------------------- test/test_resume.cpp | 2 +- test/test_session.cpp | 12 +++++++++ 10 files changed, 74 insertions(+), 62 deletions(-) diff --git a/ChangeLog b/ChangeLog index 670759f58..568e7e89b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -35,6 +35,7 @@ 1.0.3 release + * fix bug in interest calculation, causing premature disconnects * tweak flag_override_resume_data semantics to make more sense (breaks backwards compatibility of edge-cases) * improve DHT bootstrapping and periodic refresh diff --git a/Jamfile b/Jamfile index 37ab954c8..9fc7971c9 100755 --- a/Jamfile +++ b/Jamfile @@ -338,8 +338,8 @@ feature iconv : auto on off : composite propagated ; feature.compose on : TORRENT_USE_ICONV=1 ; feature.compose off : TORRENT_USE_ICONV=0 ; -feature valgrind : off on : composite propagated link-incompatible ; -feature.compose on : TORRENT_USE_VALGRIND=1 ; +feature use-valgrind : off on : composite propagated link-incompatible ; +feature.compose on : TORRENT_USE_VALGRIND=1 ; feature memory-optimization : off on : composite propagated link-incompatible ; feature.compose on : TORRENT_OPTIMIZE_MEMORY_USAGE ; diff --git a/src/bt_peer_connection.cpp b/src/bt_peer_connection.cpp index 4b36406fa..5cea62b65 100644 --- a/src/bt_peer_connection.cpp +++ b/src/bt_peer_connection.cpp @@ -2101,7 +2101,7 @@ namespace libtorrent if (piece >= 0) superseed_piece(-1, piece); return; } - else if (m_supports_fast && t->is_seed()) + else if (m_supports_fast && t->is_seed() && !m_settings.get_bool(settings_pack::lazy_bitfields)) { write_have_all(); send_allowed_set(); diff --git a/src/peer_connection.cpp b/src/peer_connection.cpp index d2fd58411..58f7e0e33 100644 --- a/src/peer_connection.cpp +++ b/src/peer_connection.cpp @@ -517,6 +517,8 @@ namespace libtorrent else t->peer_is_interesting(*this); TORRENT_ASSERT(in_handshake() || is_interesting() == interested); + + disconnect_if_redundant(); } #if defined TORRENT_VERBOSE_LOGGING || defined TORRENT_ERROR_LOGGING @@ -1887,6 +1889,15 @@ namespace libtorrent ++m_remote_pieces_dled; } + // it's important to update whether we're intersted in this peer before + // calling disconnect_if_redundant, otherwise we may disconnect even if + // we are interested + if (!t->has_piece_passed(index) + && !t->is_seed() + && !is_interesting() + && (!t->has_picker() || t->picker().piece_priority(index) != 0)) + t->peer_is_interesting(*this); + // it's important to not disconnect before we have // updated the piece picker, otherwise we will incorrectly // decrement the piece count without first incrementing it @@ -1912,12 +1923,6 @@ namespace libtorrent if (is_disconnecting()) return; } - if (!t->has_piece_passed(index) - && !t->is_seed() - && !is_interesting() - && (!t->has_picker() || t->picker().piece_priority(index) != 0)) - t->peer_is_interesting(*this); - // if we're super seeding, this might mean that somebody // forwarded this piece. In which case we need to give // a new piece to that peer @@ -2100,39 +2105,21 @@ namespace libtorrent return; } - // let the torrent know which pieces the - // peer has - // if we're a seed, we don't keep track of piece availability - bool interesting = false; + // let the torrent know which pieces the peer has if we're a seed, we + // don't keep track of piece availability t->peer_has(bits, this); - if (!t->is_upload_only()) - { - for (int i = 0; i < (int)m_have_piece.size(); ++i) - { - bool have = bits[i]; - if (!have || m_have_piece[i]) continue; - // if we don't have a picker, the assumption is that the piece - // priority is 1, or that we're a seed, but in that case have_piece - // would have returned true. - if (!t->have_piece(i) && (!t->has_picker() || t->picker().piece_priority(i) != 0)) - interesting = true; - } - } - m_have_piece = bits; m_num_pieces = num_pieces; - if (interesting) t->peer_is_interesting(*this); - else if (upload_only() - && can_disconnect(error_code(errors::upload_upload_connection, get_libtorrent_category()))) - disconnect(errors::upload_upload_connection, op_bittorrent); + update_interest(); } bool peer_connection::disconnect_if_redundant() { TORRENT_ASSERT(is_single_thread()); if (m_disconnecting) return false; + if (m_need_interest_update) return false; // we cannot disconnect in a constructor TORRENT_ASSERT(m_in_constructor == false); @@ -2153,6 +2140,9 @@ namespace libtorrent if (m_upload_only && t->is_upload_only() && can_disconnect(error_code(errors::upload_upload_connection, get_libtorrent_category()))) { +#ifdef TORRENT_VERBOSE_LOGGING + peer_log("*** the peer is upload-only and our torrent is also upload-only"); +#endif disconnect(errors::upload_upload_connection, op_bittorrent); return true; } @@ -2163,6 +2153,9 @@ namespace libtorrent && t->are_files_checked() && can_disconnect(error_code(errors::uninteresting_upload_peer, get_libtorrent_category()))) { +#ifdef TORRENT_VERBOSE_LOGGING + peer_log("*** the peer is upload-only and we're not interested in it"); +#endif disconnect(errors::uninteresting_upload_peer, op_bittorrent); return true; } @@ -6578,6 +6571,7 @@ namespace libtorrent // make sure upload only peers are disconnected if (t->is_upload_only() && m_upload_only + && !m_need_interest_update && t->valid_metadata() && has_metadata() && ok_to_disconnect) @@ -6585,6 +6579,7 @@ namespace libtorrent if (m_upload_only && !m_interesting + && !m_need_interest_update && m_bitfield_received && t->are_files_checked() && t->valid_metadata() diff --git a/src/settings_pack.cpp b/src/settings_pack.cpp index 5270af38b..883d5ffe1 100644 --- a/src/settings_pack.cpp +++ b/src/settings_pack.cpp @@ -135,7 +135,7 @@ namespace libtorrent SET(allow_multiple_connections_per_ip, false, 0), DEPRECATED_SET(ignore_limits_on_local_network, true, &session_impl::update_ignore_rate_limits_on_local_network), SET(send_redundant_have, true, 0), - SET(lazy_bitfields, true, 0), + SET(lazy_bitfields, false, 0), SET(use_dht_as_fallback, false, 0), SET(upnp_ignore_nonrouters, false, 0), SET(use_parole_mode, true, 0), diff --git a/src/torrent.cpp b/src/torrent.cpp index fac8c9417..aeadd9b9b 100644 --- a/src/torrent.cpp +++ b/src/torrent.cpp @@ -10936,7 +10936,8 @@ namespace libtorrent // if we're a seed, we don't have an m_file_progress anyway // since we don't need one. We know we have all files - if (is_seed() || !has_picker()) + // just fill in the full file sizes as a shortcut + if (is_seed()) { fp.resize(m_torrent_file->num_files()); file_storage const& fs = m_torrent_file->files(); @@ -10944,6 +10945,11 @@ namespace libtorrent fp[i] = fs.file_size(i); return; } + + // we're not a seed and we don't have a picker, that means we donn't + // have any piece yet. + if (!has_picker()) + return; if (num_have() == 0) { diff --git a/test/Jamfile b/test/Jamfile index e75693d83..f1b996be9 100644 --- a/test/Jamfile +++ b/test/Jamfile @@ -87,7 +87,7 @@ project ; feature launcher : none valgrind : composite ; -feature.compose valgrind : "valgrind --tool=memcheck -v --num-callers=20 --read-var-info=yes --track-origins=yes --error-exitcode=222 --suppressions=valgrind_suppressions.txt" on ; +feature.compose valgrind : "valgrind --tool=memcheck -v --num-callers=20 --read-var-info=yes --track-origins=yes --error-exitcode=222 --suppressions=valgrind_suppressions.txt" on ; test-suite libtorrent : [ run test_resume.cpp ] @@ -158,7 +158,6 @@ test-suite libtorrent : [ run test_http_connection.cpp ] [ run test_torrent.cpp ] [ run test_transfer.cpp ] -# [ run test_entry.cpp ] [ run test_metadata_extension.cpp ] [ run test_trackers_extension.cpp ] [ run test_time_critical.cpp ] diff --git a/test/test_priority.cpp b/test/test_priority.cpp index 3e6c74afa..844a9aff4 100644 --- a/test/test_priority.cpp +++ b/test/test_priority.cpp @@ -69,6 +69,14 @@ bool on_alert(alert* a) return false; } +int udp_tracker_port; +int tracker_port; + +// these are declared before the session objects +// so that they are destructed last. This enables +// the sessions to destruct in parallel +std::vector sp; + void test_transfer(settings_pack const& sett) { // in case the previous run was terminated @@ -78,16 +86,11 @@ void test_transfer(settings_pack const& sett) remove_all("tmp1_priority_moved", ec); remove_all("tmp2_priority_moved", ec); - // these are declared before the session objects - // so that they are destructed last. This enables - // the sessions to destruct in parallel - session_proxy p1; - session_proxy p2; - lt::session ses1(fingerprint("LT", 0, 1, 0, 0), std::make_pair(48075, 49000), "0.0.0.0", 0, mask); lt::session ses2(fingerprint("LT", 0, 1, 0, 0), std::make_pair(49075, 50000), "0.0.0.0", 0, mask); settings_pack pack = sett; + // we need a short reconnect time since we // finish the torrent and then restart it // immediately to complete the second half. @@ -106,21 +109,10 @@ void test_transfer(settings_pack const& sett) pack.set_int(settings_pack::out_enc_policy, settings_pack::pe_disabled); pack.set_int(settings_pack::in_enc_policy, settings_pack::pe_disabled); - pack.set_bool(settings_pack::allow_multiple_connections_per_ip, false); - - pack.set_int(settings_pack::unchoke_slots_limit, 0); - ses1.apply_settings(pack); - TEST_CHECK(ses1.get_settings().get_int(settings_pack::unchoke_slots_limit) == 0); - - pack.set_int(settings_pack::unchoke_slots_limit, -1); - ses1.apply_settings(pack); - TEST_CHECK(ses1.get_settings().get_int(settings_pack::unchoke_slots_limit) == -1); - pack.set_int(settings_pack::unchoke_slots_limit, 8); - ses1.apply_settings(pack); - TEST_CHECK(ses1.get_settings().get_int(settings_pack::unchoke_slots_limit) == 8); + ses1.apply_settings(pack); ses2.apply_settings(pack); torrent_handle tor1; @@ -131,9 +123,6 @@ void test_transfer(settings_pack const& sett) boost::shared_ptr t = ::create_torrent(&file, 16 * 1024, 13, false); file.close(); - int udp_tracker_port = start_udp_tracker(); - int tracker_port = start_web_server(); - char tracker_url[200]; snprintf(tracker_url, sizeof(tracker_url), "http://127.0.0.1:%d/announce", tracker_port); t->add_tracker(tracker_url); @@ -392,17 +381,17 @@ void test_transfer(settings_pack const& sett) TEST_CHECK(st2.is_seeding); // this allows shutting down the sessions in parallel - p1 = ses1.abort(); - p2 = ses2.abort(); - - stop_udp_tracker(); - stop_web_server(); + sp.push_back(ses1.abort()); + sp.push_back(ses2.abort()); } int test_main() { using namespace libtorrent; + udp_tracker_port = start_udp_tracker(); + tracker_port = start_web_server(); + // test with all kinds of proxies settings_pack p; @@ -410,6 +399,9 @@ int test_main() p = settings_pack(); p.set_bool(settings_pack::contiguous_recv_buffer, false); test_transfer(p); + + p.set_bool(settings_pack::lazy_bitfields, true); + test_transfer(p); error_code ec; remove_all("tmp1_priorities", ec); @@ -417,6 +409,13 @@ int test_main() remove_all("tmp1_priorities_moved", ec); remove_all("tmp2_priorities_moved", ec); + stop_udp_tracker(); + stop_web_server(); + + // we have to clear them, session doesn't really support being destructed + // as a global destructor (for silly reasons) + sp.clear(); + return 0; } diff --git a/test/test_resume.cpp b/test/test_resume.cpp index 72bd3053a..8b702ec5a 100644 --- a/test/test_resume.cpp +++ b/test/test_resume.cpp @@ -119,7 +119,7 @@ std::vector generate_resume_data(torrent_info* ti) torrent_status test_resume_flags(int flags) { - session ses; + libtorrent::session ses; boost::shared_ptr ti = generate_torrent(); diff --git a/test/test_session.cpp b/test/test_session.cpp index 80a0f7d79..8fe9fb308 100644 --- a/test/test_session.cpp +++ b/test/test_session.cpp @@ -70,6 +70,18 @@ int test_main() TEST_CHECK(a.get()); + sett.set_int(settings_pack::unchoke_slots_limit, 0); + ses.apply_settings(sett); + TEST_CHECK(ses.get_settings().get_int(settings_pack::unchoke_slots_limit) == 0); + + sett.set_int(settings_pack::unchoke_slots_limit, -1); + ses.apply_settings(sett); + TEST_CHECK(ses.get_settings().get_int(settings_pack::unchoke_slots_limit) == -1); + + sett.set_int(settings_pack::unchoke_slots_limit, 8); + ses.apply_settings(sett); + TEST_CHECK(ses.get_settings().get_int(settings_pack::unchoke_slots_limit) == 8); + // make sure the destructor waits properly // for the asynchronous call to set the alert // mask completes, before it goes on to destruct