From a0338f6d24299a18274fa8fd81ea482aaebd5904 Mon Sep 17 00:00:00 2001 From: Arvid Norberg Date: Thu, 27 Jan 2011 06:49:32 +0000 Subject: [PATCH] made the metadata block requesting algorithm more robust against hash check failures --- ChangeLog | 2 + src/ut_metadata.cpp | 209 +++++++++++++++++++++++++++++--------------- 2 files changed, 142 insertions(+), 69 deletions(-) diff --git a/ChangeLog b/ChangeLog index 1c8b1798d..9c61e84f1 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,4 @@ + * made the metadata block requesting algorithm more robust against hash check failures * support a separate option to use proxies for peers or not * pausing the session now also pauses checking torrents * moved alert queue size limit into session_settings @@ -75,6 +76,7 @@ incoming connection * added more detailed instrumentation of the disk I/O thread + * fixed minor bug in metadata block requester (for magnet links) * fixed race condition in iconv string converter * fixed error handling in torrent_info constructor * fixed bug in torrent_info::remap_files diff --git a/src/ut_metadata.cpp b/src/ut_metadata.cpp index b7c689b41..a47d5ee66 100644 --- a/src/ut_metadata.cpp +++ b/src/ut_metadata.cpp @@ -66,6 +66,8 @@ namespace libtorrent { namespace return (numerator + denominator - 1) / denominator; } + struct ut_metadata_peer_plugin; + struct ut_metadata_plugin : torrent_plugin { ut_metadata_plugin(torrent& t) @@ -100,57 +102,8 @@ namespace libtorrent { namespace + m_metadata_size); } - bool received_metadata(char const* buf, int size, int piece, int total_size) - { - if (m_torrent.valid_metadata()) return false; - - if (!m_metadata) - { - // verify the total_size - if (total_size <= 0 || total_size > 500 * 1024) return false; - - m_metadata.reset(new char[total_size]); - m_requested_metadata.resize(div_round_up(total_size, 16 * 1024), 0); - m_metadata_size = total_size; - } - - if (piece < 0 || piece >= int(m_requested_metadata.size())) - return false; - - if (total_size != m_metadata_size) - { - // they disagree about the size! - return false; - } - - if (piece * 16 * 1024 + size > m_metadata_size) - { - // this piece is invalid - return false; - } - - std::memcpy(&m_metadata[piece * 16 * 1024], buf, size); - // mark this piece has 'have' - m_requested_metadata[piece] = (std::numeric_limits::max)(); - - bool have_all = std::count(m_requested_metadata.begin() - , m_requested_metadata.end(), (std::numeric_limits::max)()) - == int(m_requested_metadata.size()); - - if (!have_all) return false; - - if (!m_torrent.set_metadata(&m_metadata[0], m_metadata_size)) - { - if (!m_torrent.valid_metadata()) - std::fill(m_requested_metadata.begin(), m_requested_metadata.end(), 0); - return false; - } - - // clear the storage for the bitfield - std::vector().swap(m_requested_metadata); - - return true; - } + bool received_metadata(boost::weak_ptr const& source + , char const* buf, int size, int piece, int total_size); // returns a piece of the metadata that // we should request. @@ -178,7 +131,7 @@ namespace libtorrent { namespace if (m_metadata_size > 0 || size <= 0 || size > 500 * 1024) return; m_metadata_size = size; m_metadata.reset(new char[size]); - m_requested_metadata.resize(div_round_up(size, 16 * 1024), 0); + m_requested_metadata.resize(div_round_up(size, 16 * 1024)); } private: @@ -193,19 +146,28 @@ namespace libtorrent { namespace int m_metadata_progress; mutable int m_metadata_size; + struct metadata_piece + { + metadata_piece(): num_requests(0) {} + int num_requests; + boost::weak_ptr source; + bool operator<(metadata_piece const& rhs) const + { return num_requests < rhs.num_requests; } + }; + // this vector keeps track of how many times each meatdata - // block has been requested + // block has been requested and who we ended up getting it from // std::numeric_limits::max() means we have the piece - std::vector m_requested_metadata; + std::vector m_requested_metadata; }; - struct ut_metadata_peer_plugin : peer_plugin + struct ut_metadata_peer_plugin : peer_plugin, boost::enable_shared_from_this { ut_metadata_peer_plugin(torrent& t, bt_peer_connection& pc , ut_metadata_plugin& tp) : m_message_index(0) - , m_no_metadata(min_time()) + , m_request_limit(min_time()) , m_torrent(t) , m_pc(pc) , m_tp(tp) @@ -244,13 +206,13 @@ namespace libtorrent { namespace void write_metadata_packet(int type, int piece) { TORRENT_ASSERT(type >= 0 && type <= 2); - TORRENT_ASSERT(piece >= 0); TORRENT_ASSERT(!m_pc.associated_torrent().expired()); #ifdef TORRENT_VERBOSE_LOGGING (*m_pc.m_logger) << time_now_string() << " ==> UT_METADATA [ " "type: " << type << " | piece: " << piece << " ]\n"; #endif + // abort if the peer doesn't support the metadata extension if (m_message_index == 0) return; @@ -263,6 +225,17 @@ namespace libtorrent { namespace if (type == 1) { + + if (piece < 0 || piece >= int(m_tp.metadata().left() + 16 * 1024 - 1)/(16*1024)) + { +#ifdef TORRENT_VERBOSE_LOGGING + (*m_pc.m_logger) << time_now_string() << " <== UT_METADATA [ invalid piece " + << piece << " metadata size: " << m_tp.metadata().left() << " ]\n"; +#endif + m_pc.disconnect(errors::invalid_metadata_message, 2); + return; + } + TORRENT_ASSERT(m_pc.associated_torrent().lock()->valid_metadata()); e["total_size"] = m_tp.metadata().left(); int offset = piece * 16 * 1024; @@ -297,6 +270,10 @@ namespace libtorrent { namespace if (length > 17 * 1024) { +#ifdef TORRENT_VERBOSE_LOGGING + (*m_pc.m_logger) << time_now_string() << " <== UT_METADATA [ packet too big " + << length << " ]\n"; +#endif m_pc.disconnect(errors::invalid_metadata_message, 2); return true; } @@ -307,12 +284,26 @@ namespace libtorrent { namespace entry msg = bdecode(body.begin, body.end, len); if (msg.type() == entry::undefined_t) { +#ifdef TORRENT_VERBOSE_LOGGING + (*m_pc.m_logger) << time_now_string() << " <== UT_METADATA [ not a dictionary ]\n"; +#endif m_pc.disconnect(errors::invalid_metadata_message, 2); return true; } - int type = msg["msg_type"].integer(); - int piece = msg["piece"].integer(); + entry const* type_ent = msg.find_key("msg_type"); + entry const* piece_ent = msg.find_key("piece"); + if (type_ent == 0 || type_ent->type() != entry::int_t + || piece_ent == 0 || piece_ent->type() != entry::int_t) + { +#ifdef TORRENT_VERBOSE_LOGGING + (*m_pc.m_logger) << time_now_string() << " <== UT_METADATA [ missing or invalid keys ]\n"; +#endif + m_pc.disconnect(errors::invalid_metadata_message, 2); + return true; + } + int type = type_ent->integer(); + int piece = type_ent->integer(); #ifdef TORRENT_VERBOSE_LOGGING (*m_pc.m_logger) << time_now_string() << " <== UT_METADATA [ " @@ -342,14 +333,14 @@ namespace libtorrent { namespace m_sent_requests.erase(i); entry const* total_size = msg.find_key("total_size"); - m_tp.received_metadata(body.begin + len, body.left() - len, piece + m_tp.received_metadata(shared_from_this(), body.begin + len, body.left() - len, piece , (total_size && total_size->type() == entry::int_t) ? total_size->integer() : 0); maybe_send_request(); } break; case 2: // have no data { - m_no_metadata = time_now(); + m_request_limit = (std::min)(time_now() + minutes(1), m_request_limit); std::vector::iterator i = std::find(m_sent_requests.begin() , m_sent_requests.end(), piece); // unwanted piece? @@ -388,7 +379,12 @@ namespace libtorrent { namespace bool has_metadata() const { - return time_now() - m_no_metadata > minutes(1); + return time_now() > m_request_limit; + } + + void failed_hash_check(ptime const& now) + { + m_request_limit = now + seconds(20 + (rand() * 50) / RAND_MAX); } private: @@ -397,9 +393,11 @@ namespace libtorrent { namespace // for metadata extension messages. int m_message_index; - // this is set to the current time each time we get a - // "I don't have metadata" message. - ptime m_no_metadata; + // this is set to the next time we can request pieces + // again. It is updated every time we get a + // "I don't have metadata" message, but also when + // we receive metadata that fails the infohash check + ptime m_request_limit; // request queues std::vector m_sent_requests; @@ -422,22 +420,95 @@ namespace libtorrent { namespace int ut_metadata_plugin::metadata_request() { - std::vector::iterator i = std::min_element( + std::vector::iterator i = std::min_element( m_requested_metadata.begin(), m_requested_metadata.end()); if (m_requested_metadata.empty()) { // if we don't know how many pieces there are // just ask for piece 0 - m_requested_metadata.resize(1, 1); - return 0; + m_requested_metadata.resize(1); + i = m_requested_metadata.begin(); } int piece = i - m_requested_metadata.begin(); - m_requested_metadata[piece] = piece; + ++m_requested_metadata[piece].num_requests; return piece; } + inline bool ut_metadata_plugin::received_metadata( + boost::weak_ptr const& source + , char const* buf, int size, int piece, int total_size) + { + if (m_torrent.valid_metadata()) return false; + + if (!m_metadata) + { + // verify the total_size + if (total_size <= 0 || total_size > 500 * 1024) return false; + + m_metadata.reset(new char[total_size]); + m_requested_metadata.resize(div_round_up(total_size, 16 * 1024)); + m_metadata_size = total_size; + } + + if (piece < 0 || piece >= int(m_requested_metadata.size())) + return false; + + if (total_size != m_metadata_size) + { + // they disagree about the size! + return false; + } + + if (piece * 16 * 1024 + size > m_metadata_size) + { + // this piece is invalid + return false; + } + + std::memcpy(&m_metadata[piece * 16 * 1024], buf, size); + // mark this piece has 'have' + m_requested_metadata[piece].num_requests = (std::numeric_limits::max)(); + m_requested_metadata[piece].source = source; + + bool have_all = std::count_if(m_requested_metadata.begin() + , m_requested_metadata.end(), boost::bind(&metadata_piece::num_requests, _1) + == (std::numeric_limits::max)()) == int(m_requested_metadata.size()); + + if (!have_all) return false; + + if (!m_torrent.set_metadata(&m_metadata[0], m_metadata_size)) + { + if (!m_torrent.valid_metadata()) + { + ptime now = time_now(); + // any peer that we downloaded metadata from gets a random time + // penalty, from 5 to 30 seconds or so. During this time we don't + // make any metadata requests from those peers (to mix it up a bit + // of which peers we use) + // if we only have one block, and thus requested it from a single + // peer, we bump up the retry time a lot more to try other peers + bool single_peer = m_requested_metadata.size() == 1; + for (int i = 0; i < m_requested_metadata.size(); ++i) + { + m_requested_metadata[i].num_requests = 0; + boost::shared_ptr peer + = m_requested_metadata[i].source.lock(); + if (!peer) continue; + + peer->failed_hash_check(single_peer ? now + minutes(5) : now); + } + } + return false; + } + + // clear the storage for the bitfield + std::vector().swap(m_requested_metadata); + + return true; + } + } } namespace libtorrent