From 92ed7fb3643d4d16e1b6af5eb5ca1ff0e0603da3 Mon Sep 17 00:00:00 2001 From: Arvid Norberg Date: Fri, 29 Aug 2008 17:21:56 +0000 Subject: [PATCH] fixed metadata extension issues --- include/libtorrent/peer_connection.hpp | 7 +- src/bt_peer_connection.cpp | 15 ++- src/peer_connection.cpp | 146 ++++++++++++++++++------- src/policy.cpp | 1 + src/torrent.cpp | 12 +- 5 files changed, 130 insertions(+), 51 deletions(-) diff --git a/include/libtorrent/peer_connection.hpp b/include/libtorrent/peer_connection.hpp index c97ca66b9..cb4f30e40 100644 --- a/include/libtorrent/peer_connection.hpp +++ b/include/libtorrent/peer_connection.hpp @@ -168,7 +168,9 @@ namespace libtorrent // this is called when the metadata is retrieved // and the files has been checked - virtual void on_metadata() {} + virtual void on_metadata() {}; + + void on_metadata_impl(); void set_upload_limit(int limit); void set_download_limit(int limit); @@ -363,7 +365,7 @@ namespace libtorrent // the following functions appends messages // to the send buffer void send_choke(); - void send_unchoke(); + bool send_unchoke(); void send_interested(); void send_not_interested(); @@ -865,6 +867,7 @@ namespace libtorrent public: bool m_in_constructor:1; bool m_disconnect_started:1; + bool m_initialized:1; #endif }; } diff --git a/src/bt_peer_connection.cpp b/src/bt_peer_connection.cpp index 7e9fd43ce..4f8992a03 100644 --- a/src/bt_peer_connection.cpp +++ b/src/bt_peer_connection.cpp @@ -245,6 +245,10 @@ namespace libtorrent void bt_peer_connection::on_metadata() { + // connections that are still in the handshake + // will send their bitfield when the handshake + // is done + if (m_state < read_packet_size) return; boost::shared_ptr t = associated_torrent().lock(); TORRENT_ASSERT(t); write_bitfield(); @@ -252,7 +256,6 @@ namespace libtorrent if (m_supports_dht_port && m_ses.m_dht) write_dht_port(m_ses.get_dht_settings().service_port); #endif - if (is_interesting()) write_interested(); } void bt_peer_connection::write_dht_port(int listen_port) @@ -938,9 +941,13 @@ namespace libtorrent // if we don't have the metedata, we cannot // verify the bitfield size if (t->valid_metadata() - && packet_size() - 1 != ((int)get_bitfield().size() + 7) / 8) + && packet_size() - 1 != (t->torrent_file().num_pieces() + 7) / 8) { - disconnect("bitfield with invalid size", 2); + std::stringstream msg; + msg << "got bitfield with invalid size: " << (packet_size() - 1) + << "bytes. expected: " << ((t->torrent_file().num_pieces() + 7) / 8) + << " bytes"; + disconnect(msg.str().c_str(), 2); return; } @@ -2500,7 +2507,7 @@ namespace libtorrent m_state = read_packet_size; reset_recv_buffer(5); - if (t->valid_metadata()) + if (t->ready_for_connections()) { write_bitfield(); #ifndef TORRENT_DISABLE_DHT diff --git a/src/peer_connection.cpp b/src/peer_connection.cpp index d2a41f5e8..34e1995c3 100644 --- a/src/peer_connection.cpp +++ b/src/peer_connection.cpp @@ -133,6 +133,7 @@ namespace libtorrent #ifndef NDEBUG , m_in_constructor(true) , m_disconnect_started(false) + , m_initialized(false) #endif { m_channel_state[upload_channel] = peer_info::bw_idle; @@ -238,6 +239,7 @@ namespace libtorrent #ifndef NDEBUG , m_in_constructor(true) , m_disconnect_started(false) + , m_initialized(false) #endif { m_channel_state[upload_channel] = peer_info::bw_idle; @@ -430,6 +432,60 @@ namespace libtorrent } } + void peer_connection::on_metadata_impl() + { + boost::shared_ptr t = associated_torrent().lock(); + m_have_piece.resize(t->torrent_file().num_pieces(), m_have_all); + m_num_pieces = m_have_piece.count(); + if (m_num_pieces == int(m_have_piece.size())) + { +#ifdef TORRENT_VERBOSE_LOGGING + (*m_logger) << time_now_string() + << " *** on_metadata(): THIS IS A SEED ***\n"; +#endif + // if this is a web seed. we don't have a peer_info struct + if (m_peer_info) m_peer_info->seed = true; + m_upload_only = true; + + t->peer_has_all(); + disconnect_if_redundant(); + if (m_disconnecting) return; + + on_metadata(); + if (m_disconnecting) return; + + if (!t->is_finished()) + t->get_policy().peer_is_interesting(*this); + + return; + } + TORRENT_ASSERT(!m_have_all); + + on_metadata(); + if (m_disconnecting) 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; + if (!t->is_seed()) + { + t->peer_has(m_have_piece); + + for (int i = 0; i < (int)m_have_piece.size(); ++i) + { + if (m_have_piece[i]) + { + if (!t->have_piece(i) && t->picker().piece_priority(i) != 0) + interesting = true; + } + } + } + + if (interesting) t->get_policy().peer_is_interesting(*this); + else if (upload_only()) disconnect("upload to upload connections"); + } + void peer_connection::init() { INVARIANT_CHECK; @@ -440,13 +496,15 @@ namespace libtorrent TORRENT_ASSERT(t->ready_for_connections()); m_have_piece.resize(t->torrent_file().num_pieces(), m_have_all); - if (m_have_all) m_num_pieces = t->torrent_file().num_pieces(); + if (m_have_all) m_num_pieces = t->torrent_file().num_pieces(); +#ifndef NDEBUG + m_initialized = true; +#endif // now that we have a piece_picker, // update it with this peer's pieces - TORRENT_ASSERT(m_num_pieces == std::count(m_have_piece.begin() - , m_have_piece.end(), true)); + TORRENT_ASSERT(m_num_pieces == m_have_piece.count()); if (m_num_pieces == int(m_have_piece.size())) { @@ -1105,6 +1163,7 @@ namespace libtorrent else { m_have_piece.set_bit(index); + ++m_num_pieces; // only update the piece_picker if // we have the metadata and if @@ -1112,7 +1171,6 @@ namespace libtorrent // we won't have a piece picker) if (t->valid_metadata()) { - ++m_num_pieces; t->peer_has(index); if (!t->have_piece(index) @@ -1177,11 +1235,11 @@ namespace libtorrent // if we don't have the metedata, we cannot // verify the bitfield size if (t->valid_metadata() - && (bits.size() / 8) != (m_have_piece.size() / 8)) + && (bits.size() + 7) / 8 != (m_have_piece.size() + 7) / 8) { std::stringstream msg; - msg << "got bitfield with invalid size: " << (bits.size() / 8) - << "bytes. expected: " << (m_have_piece.size() / 8) + msg << "got bitfield with invalid size: " << ((bits.size() + 7) / 8) + << "bytes. expected: " << ((m_have_piece.size() + 7) / 8) << " bytes"; disconnect(msg.str().c_str(), 2); return; @@ -1928,20 +1986,22 @@ namespace libtorrent #endif if (is_disconnecting()) return; - if (index < 0 || index >= int(m_have_piece.size())) + if (t->valid_metadata()) { + if (index < 0 || index >= int(m_have_piece.size())) + { #if defined TORRENT_VERBOSE_LOGGING || defined TORRENT_ERROR_LOGGING - (*m_logger) << time_now_string() << " <== INVALID_ALLOWED_FAST [ " << index << " | s: " - << int(m_have_piece.size()) << " ]\n"; + (*m_logger) << time_now_string() << " <== INVALID_ALLOWED_FAST [ " << index << " | s: " + << int(m_have_piece.size()) << " ]\n"; #endif - return; - } + return; + } - // if we already have the piece, we can - // ignore this message - if (t->valid_metadata() - && t->have_piece(index)) - return; + // if we already have the piece, we can + // ignore this message + if (t->have_piece(index)) + return; + } m_allowed_fast.push_back(index); @@ -1949,6 +2009,7 @@ namespace libtorrent // to download it, request it if (int(m_have_piece.size()) > index && m_have_piece[index] + && t->valid_metadata() && t->has_picker() && t->picker().piece_priority(index) > 0) { @@ -2120,11 +2181,13 @@ namespace libtorrent } } - void peer_connection::send_unchoke() + bool peer_connection::send_unchoke() { INVARIANT_CHECK; - if (!m_choked) return; + if (!m_choked) return false; + boost::shared_ptr t = m_torrent.lock(); + if (!t->ready_for_connections()) return false; m_last_unchoke = time_now(); write_unchoke(); m_choked = false; @@ -2132,14 +2195,15 @@ namespace libtorrent #ifdef TORRENT_VERBOSE_LOGGING (*m_logger) << time_now_string() << " ==> UNCHOKE\n"; #endif + return true; } void peer_connection::send_interested() { if (m_interesting) return; - m_interesting = true; boost::shared_ptr t = m_torrent.lock(); - if (!t->valid_metadata()) return; + if (!t->ready_for_connections()) return; + m_interesting = true; write_interested(); #ifdef TORRENT_VERBOSE_LOGGING @@ -2150,9 +2214,9 @@ namespace libtorrent void peer_connection::send_not_interested() { if (!m_interesting) return; - m_interesting = false; boost::shared_ptr t = m_torrent.lock(); - if (!t->valid_metadata()) return; + if (!t->ready_for_connections()) return; + m_interesting = false; write_not_interested(); m_became_uninteresting = time_now(); @@ -2744,7 +2808,7 @@ namespace libtorrent } if (is_disconnecting()) return; - if (!t->valid_metadata()) return; + if (!t->ready_for_connections()) return; // calculate the desired download queue size const float queue_time = m_ses.settings().request_queue_time; @@ -2946,7 +3010,7 @@ namespace libtorrent while (!m_requests.empty() && (send_buffer_size() + m_reading_bytes < buffer_size_watermark)) { - TORRENT_ASSERT(t->valid_metadata()); + TORRENT_ASSERT(t->ready_for_connections()); peer_request& r = m_requests.front(); TORRENT_ASSERT(r.piece >= 0); @@ -3679,6 +3743,8 @@ namespace libtorrent TORRENT_ASSERT(!is_choked()); } + TORRENT_ASSERT(m_have_piece.count() == m_num_pieces); + if (!t) { #ifdef TORRENT_EXPENSIVE_INVARIANT_CHECKS @@ -3691,6 +3757,9 @@ namespace libtorrent return; } + if (t->ready_for_connections() && m_initialized) + TORRENT_ASSERT(t->torrent_file().num_pieces() == m_have_piece.size()); + if (m_ses.settings().close_redundant_connections) { // make sure upload only peers are disconnected @@ -3703,10 +3772,14 @@ namespace libtorrent TORRENT_ASSERT(m_disconnect_started); } - if (t->is_finished()) - TORRENT_ASSERT(!m_interesting); - if (is_seed()) - TORRENT_ASSERT(m_upload_only); + if (!m_disconnect_started) + { + // none of this matters if we're disconnecting anyway + if (t->is_finished()) + TORRENT_ASSERT(!m_interesting); + if (is_seed()) + TORRENT_ASSERT(m_upload_only); + } if (t->has_picker()) { @@ -3777,18 +3850,6 @@ namespace libtorrent */ } } -// expensive when using checked iterators -/* - if (t->valid_metadata()) - { - int piece_count = std::count(m_have_piece.begin() - , m_have_piece.end(), true); - if (m_num_pieces != piece_count) - { - TORRENT_ASSERT(false); - } - } -*/ // extremely expensive invariant check /* @@ -3870,7 +3931,8 @@ namespace libtorrent { // if m_num_pieces == 0, we probably don't have the // metadata yet. - return m_num_pieces == (int)m_have_piece.size() && m_num_pieces > 0; + boost::shared_ptr t = m_torrent.lock(); + return m_num_pieces == (int)m_have_piece.size() && m_num_pieces > 0 && t && t->valid_metadata(); } } diff --git a/src/policy.cpp b/src/policy.cpp index 06990d45a..f51b563cd 100644 --- a/src/policy.cpp +++ b/src/policy.cpp @@ -1058,6 +1058,7 @@ namespace libtorrent { // INVARIANT_CHECK; + if (c.in_handshake()) return; c.send_interested(); if (c.has_peer_choked() && c.allowed_fast().empty()) diff --git a/src/torrent.cpp b/src/torrent.cpp index 7c0ed22f9..3980e9da9 100644 --- a/src/torrent.cpp +++ b/src/torrent.cpp @@ -333,7 +333,11 @@ namespace libtorrent // we need to start announcing since we don't have any // metadata. To receive peers to ask for it. if (m_torrent_file->is_valid()) init(); - else if (!m_trackers.empty()) start_announcing(); + else + { + set_state(torrent_status::downloading_metadata); + if (!m_trackers.empty()) start_announcing(); + } if (m_abort) return; } @@ -2001,7 +2005,7 @@ namespace libtorrent TORRENT_ASSERT(c.is_choked()); if (m_num_uploads >= m_max_uploads) return false; - c.send_unchoke(); + if (!c.send_unchoke()) return false; ++m_num_uploads; return true; } @@ -3334,7 +3338,9 @@ namespace libtorrent { peer_connection* pc = *i; ++i; - pc->on_metadata(); + if (pc->is_disconnecting()) continue; + pc->on_metadata_impl(); + if (pc->is_disconnecting()) continue; pc->init(); } }