From 4e0a4db9b4a7786367c8b3c03e054643f86db8da Mon Sep 17 00:00:00 2001 From: Arvid Norberg Date: Sun, 12 Jun 2016 20:36:05 -0400 Subject: [PATCH 1/6] fixed inverted priority of incoming piece suggestions (#809) --- ChangeLog | 1 + src/peer_connection.cpp | 8 +++-- test/test_fast_extension.cpp | 62 +++++++++++++++++++++++++++++++++++- 3 files changed, 68 insertions(+), 3 deletions(-) diff --git a/ChangeLog b/ChangeLog index 14a6db264..5bed298dd 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,6 @@ 1.1.1 release + * fixed inverted priority of incoming piece suggestions * optimize allow-fast logic * fix issue where FAST extension messages were not used during handshake * fixed crash on invalid input in http_parser diff --git a/src/peer_connection.cpp b/src/peer_connection.cpp index ffb399a7c..e77bd43d1 100644 --- a/src/peer_connection.cpp +++ b/src/peer_connection.cpp @@ -1580,10 +1580,14 @@ namespace libtorrent return; } + // the piece picker will prioritize the pieces from the beginning to end. + // the later the suggestion is received, the higher priority we should + // ascribe to it, so we need to insert suggestions at the front of the + // queue. if (int(m_suggested_pieces.size()) > m_settings.get_int(settings_pack::max_suggest_pieces)) - m_suggested_pieces.erase(m_suggested_pieces.begin()); + m_suggested_pieces.resize(m_settings.get_int(settings_pack::max_suggest_pieces) - 1); - m_suggested_pieces.push_back(index); + m_suggested_pieces.insert(m_suggested_pieces.begin(), index); #ifndef TORRENT_DISABLE_LOGGING peer_log(peer_log_alert::info, "SUGGEST_PIECE", "piece: %d added to set: %d" diff --git a/test/test_fast_extension.cpp b/test/test_fast_extension.cpp index bd99207dd..61e8ceeb2 100644 --- a/test/test_fast_extension.cpp +++ b/test/test_fast_extension.cpp @@ -626,7 +626,7 @@ TORRENT_TEST(reject_suggest) using namespace libtorrent::detail; char* ptr = recv_buffer + 1; - int piece = read_int32(ptr); + int const piece = read_int32(ptr); std::vector::iterator i = std::find(suggested.begin() , suggested.end(), piece); @@ -660,6 +660,66 @@ TORRENT_TEST(reject_suggest) print_session_log(*ses); } +TORRENT_TEST(suggest_order) +{ + std::cerr << "\n === test suggest ===\n" << std::endl; + + sha1_hash ih; + boost::shared_ptr ses; + io_service ios; + tcp::socket s(ios); + setup_peer(s, ih, ses); + + char recv_buffer[1000]; + do_handshake(s, ih, recv_buffer); + print_session_log(*ses); + send_have_all(s); + print_session_log(*ses); + + std::vector suggested; + suggested.push_back(0); + suggested.push_back(1); + suggested.push_back(2); + suggested.push_back(3); + + std::for_each(suggested.begin(), suggested.end() + , boost::bind(&send_suggest_piece, boost::ref(s), _1)); + print_session_log(*ses); + + send_unchoke(s); + print_session_log(*ses); + + int fail_counter = 100; + while (!suggested.empty() && fail_counter > 0) + { + print_session_log(*ses); + int len = read_message(s, recv_buffer, sizeof(recv_buffer)); + if (len == -1) break; + print_message(recv_buffer, len); + int msg = recv_buffer[0]; + fail_counter--; + + // we're just interested in requests + if (msg != 0x6) continue; + + using namespace libtorrent::detail; + char* ptr = recv_buffer + 1; + int const piece = read_int32(ptr); + + // make sure we receive the requests inverse order of sending the suggest + // messages. The last suggest should be the highest priority + int const expected_piece = suggested.back(); + suggested.pop_back(); + TEST_EQUAL(piece, expected_piece); + } + print_session_log(*ses); + TEST_CHECK(fail_counter > 0); + + s.close(); + test_sleep(500); + print_session_log(*ses); +} + TORRENT_TEST(multiple_bitfields) { std::cerr << "\n === test multiple bitfields ===\n" << std::endl; From 96ea0dc4d07c5e9e415dbbacedbc9aa2e4fa3bac Mon Sep 17 00:00:00 2001 From: Arvid Norberg Date: Mon, 13 Jun 2016 07:46:34 -0400 Subject: [PATCH 2/6] fixed bug related to flag_merge_resume_http_seeds flag in add_torrent_params (#813) --- ChangeLog | 1 + include/libtorrent/add_torrent_params.hpp | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 5bed298dd..80e536e0d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,6 @@ 1.1.1 release + * fixed bug related to flag_merge_resume_http_seeds flag in add_torrent_params * fixed inverted priority of incoming piece suggestions * optimize allow-fast logic * fix issue where FAST extension messages were not used during handshake diff --git a/include/libtorrent/add_torrent_params.hpp b/include/libtorrent/add_torrent_params.hpp index 61ebc4a11..a52ad0e9e 100644 --- a/include/libtorrent/add_torrent_params.hpp +++ b/include/libtorrent/add_torrent_params.hpp @@ -265,7 +265,7 @@ namespace libtorrent // add_torrent_params are also replaced. The default behavior is to // have any web seeds in the resume data take precedence over whatever // is passed in here as well as the .torrent file. - flag_merge_resume_http_seeds = 0x2000, + flag_merge_resume_http_seeds = 0x8000, // the stop when ready flag. Setting this flag is equivalent to calling // torrent_handle::stop_when_ready() immediately after the torrent is From d759d8f742de32276765c375aed2813c31c36b8f Mon Sep 17 00:00:00 2001 From: Alden Torres Date: Mon, 13 Jun 2016 07:47:16 -0400 Subject: [PATCH 3/6] not add peers from url if add_torrent_impl returns invalid handle (#814) --- include/libtorrent/bt_peer_connection.hpp | 2 +- include/libtorrent/torrent.hpp | 2 +- src/session_impl.cpp | 18 +++++++++--------- src/torrent.cpp | 12 ++++++------ 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/include/libtorrent/bt_peer_connection.hpp b/include/libtorrent/bt_peer_connection.hpp index e22f97623..880172773 100644 --- a/include/libtorrent/bt_peer_connection.hpp +++ b/include/libtorrent/bt_peer_connection.hpp @@ -74,7 +74,7 @@ namespace libtorrent public: // this is the constructor where the we are the active part. - // The peer_conenction should handshake and verify that the + // The peer_connection should handshake and verify that the // other end has the correct id bt_peer_connection(peer_connection_args const& pack , peer_id const& pid); diff --git a/include/libtorrent/torrent.hpp b/include/libtorrent/torrent.hpp index fee3e2273..8c0c00d92 100644 --- a/include/libtorrent/torrent.hpp +++ b/include/libtorrent/torrent.hpp @@ -884,7 +884,7 @@ namespace libtorrent void on_inactivity_tick(error_code const& ec); - // calculate the instantaneuos inactive state (the externally facing + // calculate the instantaneous inactive state (the externally facing // inactive state is not instantaneous, but low-pass filtered) bool is_inactive_internal() const; diff --git a/src/session_impl.cpp b/src/session_impl.cpp index 12ed54496..e3b5afe3c 100644 --- a/src/session_impl.cpp +++ b/src/session_impl.cpp @@ -4785,6 +4785,14 @@ retry: add_torrent_params params = p; boost::shared_ptr const torrent_ptr = add_torrent_impl(params, ec); + torrent_handle const handle(torrent_ptr); + m_alerts.emplace_alert(handle, params, ec); + + if (!torrent_ptr) return handle; + + // params.info_hash should have been initialized by add_torrent_impl() + TORRENT_ASSERT(params.info_hash != sha1_hash(0)); + // --- PEERS --- (delete when merged to master) std::vector peers; parse_magnet_uri_peers(p.url, peers); @@ -4798,14 +4806,6 @@ retry: if (!peers.empty()) torrent_ptr->update_want_peers(); - torrent_handle const handle(torrent_ptr); - m_alerts.emplace_alert(handle, params, ec); - - if (!torrent_ptr) return handle; - - // params.info_hash should have been initialized by add_torrent_impl() - TORRENT_ASSERT(params.info_hash != sha1_hash(0)); - if (m_alerts.should_post()) m_alerts.emplace_alert(handle); @@ -6405,7 +6405,7 @@ retry: void session_impl::on_trigger_auto_manage() { - assert(m_pending_auto_manage); + TORRENT_ASSERT(m_pending_auto_manage); if (!m_need_auto_manage || m_abort) { m_pending_auto_manage = false; diff --git a/src/torrent.cpp b/src/torrent.cpp index 83a2e9f0a..db23f02bc 100644 --- a/src/torrent.cpp +++ b/src/torrent.cpp @@ -2001,7 +2001,7 @@ namespace libtorrent } // ugly edge case where padfiles are not used they way they're // supposed to be. i.e. added back-to back or at the end - if (int(pb.block_index) == blocks_per_piece) { pb.block_index = 0; ++pb.piece_index; } + if (pb.block_index == blocks_per_piece) { pb.block_index = 0; ++pb.piece_index; } if (pr.length > 0 && ((i+1 != fs.num_files() && fs.pad_file_at(i+1)) || i + 1 == fs.num_files())) { @@ -3875,10 +3875,10 @@ namespace libtorrent file_storage const& fs = m_torrent_file->files(); int piece_size = m_torrent_file->piece_size(p.piece_index); int offset = p.block_index * block_size(); - if (m_padding == 0) return (std::min)(piece_size - offset, int(block_size())); + if (m_padding == 0) return (std::min)(piece_size - offset, block_size()); std::vector files = fs.map_block( - p.piece_index, offset, (std::min)(piece_size - offset, int(block_size()))); + p.piece_index, offset, (std::min)(piece_size - offset, block_size())); int ret = 0; for (std::vector::iterator i = files.begin() , end(files.end()); i != end; ++i) @@ -3886,7 +3886,7 @@ namespace libtorrent if (fs.pad_file_at(i->file_index)) continue; ret += i->size; } - TORRENT_ASSERT(ret <= (std::min)(piece_size - offset, int(block_size()))); + TORRENT_ASSERT(ret <= (std::min)(piece_size - offset, block_size())); return ret; } @@ -8631,7 +8631,7 @@ namespace libtorrent // this will move the tracker with the given index // to a prioritized position in the list (move it towards - // the begining) and return the new index to the tracker. + // the beginning) and return the new index to the tracker. int torrent::prioritize_tracker(int index) { INVARIANT_CHECK; @@ -8832,7 +8832,7 @@ namespace libtorrent TORRENT_ASSERT(index >= 0); TORRENT_ASSERT(index < m_torrent_file->num_files()); - // stoage may be NULL during shutdown + // storage may be NULL during shutdown if (!m_storage.get()) { if (alerts().should_post()) From 38f4765ebc5e12000ef71451681ed5ffa1027198 Mon Sep 17 00:00:00 2001 From: Alden Torres Date: Mon, 13 Jun 2016 17:16:09 -0400 Subject: [PATCH 4/6] added assert and documentation typos (#816) --- include/libtorrent/aux_/session_interface.hpp | 2 +- src/bdecode.cpp | 4 ++-- src/session_impl.cpp | 7 ++++--- src/storage.cpp | 4 ++-- src/torrent.cpp | 2 +- 5 files changed, 10 insertions(+), 9 deletions(-) diff --git a/include/libtorrent/aux_/session_interface.hpp b/include/libtorrent/aux_/session_interface.hpp index 363a64b95..fcd2c4317 100644 --- a/include/libtorrent/aux_/session_interface.hpp +++ b/include/libtorrent/aux_/session_interface.hpp @@ -125,7 +125,7 @@ namespace libtorrent { namespace aux }; #endif // TORRENT_DISABLE_LOGGING || TORRENT_USE_ASSERTS - // TOOD: 2 make this interface a lot smaller. It could be split up into + // TODO: 2 make this interface a lot smaller. It could be split up into // several smaller interfaces. Each subsystem could then limit the size // of the mock object to test it. struct TORRENT_EXTRA_EXPORT session_interface diff --git a/src/bdecode.cpp b/src/bdecode.cpp index 5df475b00..e5bee9b0e 100644 --- a/src/bdecode.cpp +++ b/src/bdecode.cpp @@ -103,7 +103,7 @@ namespace libtorrent stack_frame(int t): token(t), state(0) {} // this is an index into m_tokens boost::uint32_t token:31; - // this is used for doctionaries to indicate whether we're + // this is used for dictionaries to indicate whether we're // reading a key or a vale. 0 means key 1 is value boost::uint32_t state:1; }; @@ -801,7 +801,7 @@ namespace libtorrent && stack[sp-1].state == 1) { // this means we're parsing a dictionary and about to parse a - // value associated with a key. Instad, we got a termination + // value associated with a key. Instead, we got a termination TORRENT_FAIL_BDECODE(bdecode_errors::expected_value); } diff --git a/src/session_impl.cpp b/src/session_impl.cpp index e3b5afe3c..58286ec56 100644 --- a/src/session_impl.cpp +++ b/src/session_impl.cpp @@ -1494,7 +1494,7 @@ namespace aux { int loaded_limit = m_settings.get_int(settings_pack::active_loaded_limit); - // 0 means unlimited, never evict enything + // 0 means unlimited, never evict anything if (loaded_limit == 0) return; if (m_torrent_lru.size() > loaded_limit) @@ -1518,7 +1518,7 @@ namespace aux { int loaded_limit = m_settings.get_int(settings_pack::active_loaded_limit); - // 0 means unlimited, never evict enything + // 0 means unlimited, never evict anything if (loaded_limit == 0) return; // if the torrent we're ignoring (i.e. making room for), allow @@ -1559,6 +1559,7 @@ namespace aux { // we wouldn't be loading the torrent if it was already // in the LRU (and loaded) TORRENT_ASSERT(t->next == NULL && t->prev == NULL && m_torrent_lru.front() != t); + TORRENT_ASSERT(m_user_load_torrent); // now, load t into RAM std::vector buffer; @@ -6250,7 +6251,7 @@ retry: void session_impl::update_user_agent() { - // replace all occurances of '\n' with ' '. + // replace all occurrences of '\n' with ' '. std::string agent = m_settings.get_str(settings_pack::user_agent); std::string::iterator i = agent.begin(); while ((i = std::find(i, agent.end(), '\n')) diff --git a/src/storage.cpp b/src/storage.cpp index 6cb36fc0e..7511c4a45 100644 --- a/src/storage.cpp +++ b/src/storage.cpp @@ -965,7 +965,7 @@ namespace libtorrent bdecode_node slots = rd.dict_find_list("slots"); if (slots) { - if (int(slots.list_size()) == m_files.num_pieces()) + if (slots.list_size() == m_files.num_pieces()) { seed = true; for (int i = 0; i < slots.list_size(); ++i) @@ -978,7 +978,7 @@ namespace libtorrent } else if (bdecode_node pieces = rd.dict_find_string("pieces")) { - if (int(pieces.string_length()) == m_files.num_pieces()) + if (pieces.string_length() == m_files.num_pieces()) { seed = true; char const* p = pieces.string_ptr(); diff --git a/src/torrent.cpp b/src/torrent.cpp index db23f02bc..d6ec09053 100644 --- a/src/torrent.cpp +++ b/src/torrent.cpp @@ -7117,7 +7117,7 @@ namespace libtorrent // some sanity checking. Maybe we shouldn't be in seed mode anymore bdecode_node pieces = rd.dict_find("pieces"); if (pieces && pieces.type() == bdecode_node::string_t - && int(pieces.string_length()) == m_torrent_file->num_pieces()) + && pieces.string_length() == m_torrent_file->num_pieces()) { char const* pieces_str = pieces.string_ptr(); for (int i = 0, end(pieces.string_length()); i < end; ++i) From f01ac8f86c7180203651a7da46ef8e0e898a45c2 Mon Sep 17 00:00:00 2001 From: arvidn Date: Tue, 14 Jun 2016 20:10:18 -0400 Subject: [PATCH 5/6] fix announce_entry python binding --- ChangeLog | 1 + bindings/python/src/torrent_info.cpp | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/ChangeLog b/ChangeLog index 80e536e0d..10ac5eeff 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,6 @@ 1.1.1 release + * fix bug in python binding of announce_entry * fixed bug related to flag_merge_resume_http_seeds flag in add_torrent_params * fixed inverted priority of incoming piece suggestions * optimize allow-fast logic diff --git a/bindings/python/src/torrent_info.cpp b/bindings/python/src/torrent_info.cpp index 0ca2dd018..78e3547c3 100644 --- a/bindings/python/src/torrent_info.cpp +++ b/bindings/python/src/torrent_info.cpp @@ -113,10 +113,10 @@ namespace int get_tier(announce_entry const& ae) { return ae.tier; } void set_tier(announce_entry& ae, int v) { ae.tier = v; } - bool get_fail_limit(announce_entry const& ae) { return ae.fail_limit; } + int get_fail_limit(announce_entry const& ae) { return ae.fail_limit; } void set_fail_limit(announce_entry& ae, int l) { ae.fail_limit = l; } - bool get_fails(announce_entry const& ae) { return ae.fails; } - bool get_source(announce_entry const& ae) { return ae.source; } + int get_fails(announce_entry const& ae) { return ae.fails; } + int get_source(announce_entry const& ae) { return ae.source; } bool get_verified(announce_entry const& ae) { return ae.verified; } bool get_updating(announce_entry const& ae) { return ae.updating; } bool get_start_sent(announce_entry const& ae) { return ae.start_sent; } From 42c6376d5cb02a5095dc6f0dc59c25f26f284087 Mon Sep 17 00:00:00 2001 From: arvidn Date: Tue, 14 Jun 2016 20:16:04 -0400 Subject: [PATCH 6/6] another python binding fix --- bindings/python/src/torrent_handle.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bindings/python/src/torrent_handle.cpp b/bindings/python/src/torrent_handle.cpp index 59a269de3..62e035631 100644 --- a/bindings/python/src/torrent_handle.cpp +++ b/bindings/python/src/torrent_handle.cpp @@ -193,9 +193,9 @@ void dict_to_announce_entry(dict d, announce_entry& ae) if (d.has_key("source")) ae.source = extract(d["source"]); if (d.has_key("verified")) - ae.verified = extract(d["verified"]); + ae.verified = extract(d["verified"]); if (d.has_key("send_stats")) - ae.send_stats = extract(d["send_stats"]); + ae.send_stats = extract(d["send_stats"]); } void replace_trackers(torrent_handle& h, object trackers)