fixed synchronization issue between download queue and piece picker

ChangeLog

@@ -28,6 +28,7 @@
 	  incoming connection
 	* added more detailed instrumentation of the disk I/O thread
 
+	* fixed synchronization issue between download queue and piece picker
 	* fixed bug in udp tracker scrape response parsing
 	* fixed bug in the disk thread that could get triggered under heavy load
 	* fixed bug in add_piece() that would trigger asserts

include/libtorrent/peer_connection.hpp

@@ -98,12 +98,14 @@ namespace libtorrent
 	struct pending_block
 	{
 		pending_block(piece_block const& b)
-			: skipped(0), not_wanted(false), timed_out(false)
+			: block(b), skipped(0), not_wanted(false)
-			, busy(false), block(b) {}
+			, timed_out(false), busy(false) {}
+
+		piece_block block;
 
 		// the number of times the request
 		// has been skipped by out of order blocks
-		boost::uint16_t skipped;
+		boost::uint16_t skipped:13;
 
 		// if any of these are set to true, this block
 		// is not allocated
@@ -120,8 +122,6 @@ namespace libtorrent
 		// busy request at a time in each peer's queue
 		bool busy:1;
 
-		piece_block block;
-
 		bool operator==(pending_block const& b)
 		{
 			return b.skipped == skipped && b.block == block

include/libtorrent/piece_picker.hpp

@@ -63,13 +63,18 @@ namespace libtorrent
 
 	struct TORRENT_EXPORT piece_block
 	{
+		const static piece_block invalid;
+
 		piece_block() {}
-		piece_block(int p_index, int b_index)
+		piece_block(boost::uint32_t p_index, boost::uint16_t b_index)
 			: piece_index(p_index)
 			, block_index(b_index)
-		{}
+		{
-		int piece_index;
+			TORRENT_ASSERT(p_index < (1 << 18));
-		int block_index;
+			TORRENT_ASSERT(b_index < (1 << 14));
+		}
+		boost::uint32_t piece_index:18;
+		boost::uint32_t block_index:14;
 
 		bool operator<(piece_block const& b) const
 		{

src/peer_connection.cpp

@@ -99,7 +99,7 @@ namespace libtorrent
 		, m_socket(s)
 		, m_remote(endp)
 		, m_torrent(tor)
-		, m_receiving_block(-1, -1)
+		, m_receiving_block(piece_block::invalid)
 		, m_last_seen_complete(0)
 		, m_timeout_extend(0)
 		, m_outstanding_bytes(0)
@@ -234,7 +234,7 @@ namespace libtorrent
 		, m_disk_recv_buffer(ses, 0)
 		, m_socket(s)
 		, m_remote(endp)
-		, m_receiving_block(-1, -1)
+		, m_receiving_block(piece_block::invalid)
 		, m_last_seen_complete(0)
 		, m_timeout_extend(0)
 		, m_outstanding_bytes(0)
@@ -2056,6 +2056,7 @@ namespace libtorrent
 				break;
 			}
 
+			m_download_queue.insert(m_download_queue.begin(), b);
 			if (!in_req_queue)
 			{
 				if (t->alerts().should_post<unwanted_block_alert>())
@@ -2067,9 +2068,9 @@ namespace libtorrent
 				(*m_logger) << "          *** The block we just got was not in the "
 					"request queue ***\n";
 #endif
+				TORRENT_ASSERT(m_download_queue.front().block == b);
+				m_download_queue.front().not_wanted = true;
 			}
-			m_download_queue.insert(m_download_queue.begin(), b);
-			if (!in_req_queue) m_download_queue.front().not_wanted = true;
 			m_outstanding_bytes += r.length;
 		}
 	}
@@ -3038,6 +3039,23 @@ namespace libtorrent
 		{
 			pending_block block = m_request_queue.front();
 
+			m_request_queue.erase(m_request_queue.begin());
+			if (m_queued_time_critical) --m_queued_time_critical;
+
+			// if we're a seed, we don't have a piece picker
+			// so we don't have to worry about invariants getting
+			// out of sync with it
+			if (t->is_seed()) continue;
+
+			// this can happen if a block times out, is re-requested and
+			// then arrives "unexpectedly"
+			if (t->picker().is_finished(block.block)
+				|| t->picker().is_downloaded(block.block))
+			{
+				t->picker().abort_download(block.block);
+				continue;
+			}
+
 			int block_offset = block.block.block_index * t->block_size();
 			int block_size = (std::min)(t->torrent_file().piece_size(
 				block.block.piece_index) - block_offset, t->block_size());
@@ -3049,21 +3067,13 @@ namespace libtorrent
 			r.start = block_offset;
 			r.length = block_size;
 
-			m_request_queue.erase(m_request_queue.begin());
-			if (m_queued_time_critical) --m_queued_time_critical;
-			if (t->is_seed()) continue;
-			// this can happen if a block times out, is re-requested and
-			// then arrives "unexpectedly"
-			if (t->picker().is_finished(block.block)
-				|| t->picker().is_downloaded(block.block))
-				continue;
-
 			TORRENT_ASSERT(verify_piece(t->to_req(block.block)));
 			m_download_queue.push_back(block);
 			m_outstanding_bytes += block_size;
 #if !defined TORRENT_DISABLE_INVARIANT_CHECKS && defined TORRENT_DEBUG
 			check_invariant();
 #endif
+
 /*
 #ifdef TORRENT_VERBOSE_LOGGING
 			(*m_logger) << time_now_string()
@@ -3913,7 +3923,7 @@ namespace libtorrent
 		request_a_block(*t, *this);
 		m_desired_queue_size = 1;
 
-		piece_block r(-1, -1);
+		piece_block r(piece_block::invalid);
 		// time out the last request in the queue
 		if (prev_request_queue > 0)
 		{
@@ -4955,7 +4965,7 @@ namespace libtorrent
 	}
 
 #ifdef TORRENT_DEBUG
-	struct peer_count_t { int num_peers; int num_peers_with_timeouts; };
+	struct peer_count_t { int num_peers; int num_peers_with_timeouts; int num_peers_with_nowant; };
 
 	void peer_connection::check_invariant() const
 	{
@@ -5086,12 +5096,14 @@ namespace libtorrent
 				{
 					++num_requests[i->block].num_peers;
 					++num_requests[i->block].num_peers_with_timeouts;
+					++num_requests[i->block].num_peers_with_nowant;
 				}
 				for (std::vector<pending_block>::const_iterator i = p.download_queue().begin()
 					, end(p.download_queue().end()); i != end; ++i)
 				{
 					if (!i->not_wanted && !i->timed_out) ++num_requests[i->block].num_peers;
-					++num_requests[i->block].num_peers_with_timeouts;
+					if (i->timed_out) ++num_requests[i->block].num_peers_with_timeouts;
+					if (i->not_wanted) ++num_requests[i->block].num_peers_with_nowant;
 				}
 			}
 			for (std::map<piece_block, peer_count_t>::iterator i = num_requests.begin()
@@ -5100,6 +5112,7 @@ namespace libtorrent
 				piece_block b = i->first;
 				int count = i->second.num_peers;
 				int count_with_timeouts = i->second.num_peers_with_timeouts;
+				int count_with_nowant = i->second.num_peers_with_nowant;
 				int picker_count = t->picker().num_peers(b);
 				if (!t->picker().is_downloaded(b))
 					TORRENT_ASSERT(picker_count == count);

src/piece_picker.cpp

@@ -55,6 +55,8 @@ POSSIBILITY OF SUCH DAMAGE.
 namespace libtorrent
 {
 
+	const piece_block piece_block::invalid(0x3FFFF, 0x3FFF);
+
 	piece_picker::piece_picker()
 		: m_seeds(0)
 		, m_priority_boundries(1, int(m_pieces.size()))