remove the concept of timeout_extend in piece request timeouts. if a request times out, cancel it as soon as it holds up the completion of a piece

examples/client_test.cpp

@@ -1049,6 +1049,7 @@ void print_piece(libtorrent::partial_piece_info* pp
 		char chr = '+';
 		if (index >= 0)
 			chr = (index < 10)?'0' + index:'A' + index - 10;
+		bool snubbed = index >= 0 ? peers[index].flags & peer_info::snubbed : false;
 
 		char const* color = "";
 
@@ -1065,12 +1066,12 @@ void print_piece(libtorrent::partial_piece_info* pp
 					&& pp->blocks[j].state == block_info::requested)
 			{
 				if (pp->blocks[j].num_peers > 1) color = esc("1;7");
-				else color = esc("33;7");
+				else color = snubbed ? esc("35;7") : esc("33;7");
 				chr = '0' + (pp->blocks[j].bytes_progress * 10 / pp->blocks[j].block_size);
 			}
 			else if (pp->blocks[j].state == block_info::finished) color = esc("32;7");
 			else if (pp->blocks[j].state == block_info::writing) color = esc("36;7");
-			else if (pp->blocks[j].state == block_info::requested) color = esc("0");
+			else if (pp->blocks[j].state == block_info::requested) color = snubbed ? esc("35;7") : esc("0");
 			else { color = esc("0"); chr = ' '; }
 		}
 		if (last_color == 0 || strcmp(last_color, color) != 0)
@@ -2001,11 +2002,13 @@ int main(int argc, char* argv[])
 				}
 				if (out[out.size()] != '\n') out += "\n";
 
-				snprintf(str, sizeof(str), "%s %s: read cache %s %s: downloading %s %s: cached %s %s: flushed\n"
+				snprintf(str, sizeof(str), "%s %s read cache | %s %s downloading | %s %s cached | %s %s flushed | %s %s snubbed\n"
 					, esc("34;7"), esc("0") // read cache
 					, esc("33;7"), esc("0") // downloading
 					, esc("36;7"), esc("0") // cached
-					, esc("32;7"), esc("0")); // flushed
+					, esc("32;7"), esc("0") // flushed
+					, esc("35;7"), esc("0") // snubbed
+					);
 				out += str;
 				out += "___________________________________\n";
 			}

include/libtorrent/peer_connection.hpp

@@ -948,10 +948,13 @@ namespace libtorrent
 		ptime m_last_receive;
 		ptime m_last_sent;
 
-		// the time when the first entry in the
+		// the time when the first entry in the request queue was requested. Used
-		// request queue was requested, increased
+		// for request timeout. it doesn't necessarily represent the time when a
-		// for each entry that is popped from the
+		// specific request was made. Since requests can be handled out-of-order,
-		// download queue. Used for request timeout
+		// it represents whichever request the other end decided to respond to.
+		// Once we get that response, we set it to the current time.
+		// for more information, see the blog post at:
+		// http://blog.libtorrent.org/2011/11/block-request-time-outs/
 		ptime m_requested;
 
 		// a timestamp when the remote download rate
@@ -1064,11 +1067,6 @@ namespace libtorrent
 		// torrent and session should implement this interface
 		stat m_statistics;
 
-		// if the timeout is extended for the outstanding
-		// requests, this is the number of seconds it was
-		// extended.
-		int m_timeout_extend;
-
 		// the number of outstanding bytes expected
 		// to be received by extensions
 		int m_extension_outstanding_bytes;

src/peer_connection.cpp

@@ -163,7 +163,6 @@ namespace libtorrent
 		, m_outstanding_bytes(0)
 		, m_last_seen_complete(0)
 		, m_receiving_block(piece_block::invalid)
-		, m_timeout_extend(0)
 		, m_extension_outstanding_bytes(0)
 		, m_queued_time_critical(0)
 		, m_reading_bytes(0)
@@ -2716,10 +2715,11 @@ namespace libtorrent
 			if (m_download_queue.empty())
 				m_counters.inc_stats_counter(counters::num_peers_down_requests, -1);
 
-			m_timeout_extend = 0;
-
 			if (m_disconnecting) return;
 
+			// we completed an incoming block, and there are still outstanding
+			// requests. The next block we expect to receive now has another
+			// timeout period until we time out. So, reset the timer.
 			if (!m_download_queue.empty())
 				m_requested = now;
 
@@ -2728,7 +2728,9 @@ namespace libtorrent
 			send_block_requests();
 			return;
 		}
-		
+
+		// we received a request within the timeout, make sure this peer is
+		// not snubbed anymore
 		if (total_seconds(now - m_requested)
 			< m_settings.get_int(settings_pack::request_timeout)
 			&& m_snubbed)
@@ -2781,17 +2783,11 @@ namespace libtorrent
 				, performance_alert::too_high_disk_queue_limit));
 		}
 
+		// we completed an incoming block, and there are still outstanding
+		// requests. The next block we expect to receive now has another
+		// timeout period until we time out. So, reset the timer.
 		if (!m_download_queue.empty())
-		{
+			m_requested = now;
-			m_timeout_extend = (std::max)(m_timeout_extend
-				- m_settings.get_int(settings_pack::request_timeout), 0);
-			m_requested += seconds(m_settings.get_int(settings_pack::request_timeout));
-			if (m_requested > now) m_requested = now;
-		}
-		else
-		{
-			m_timeout_extend = 0;
-		}
 
 		bool was_finished = picker.is_piece_finished(p.piece);
 		// did we request this block from any other peers?
@@ -3872,7 +3868,9 @@ namespace libtorrent
 		if (!m_download_queue.empty()
 			&& empty_download_queue)
 		{
-			// This means we just added a request to this connection
+			// This means we just added a request to this connection that
+			// previously did not have a request. That's when we start the
+			// request timeout.
 			m_requested = time_now();
 #if defined TORRENT_LOGGING || defined TORRENT_ERROR_LOGGING
 			t->debug_log("REQUEST [%p] (%d ms)", this
@@ -4230,8 +4228,7 @@ namespace libtorrent
 		p.num_pieces = m_num_pieces;
 		if (m_download_queue.empty()) p.request_timeout = -1;
 		else p.request_timeout = int(total_seconds(m_requested - now)
-			+ m_settings.get_int(settings_pack::request_timeout)
+			+ m_settings.get_int(settings_pack::request_timeout));
-			+ m_timeout_extend);
 
 		p.download_queue_time = download_queue_time();
 		p.queue_bytes = m_outstanding_bytes;
@@ -4677,8 +4674,7 @@ namespace libtorrent
 		if (may_timeout
 			&& !m_download_queue.empty()
 			&& m_quota[download_channel] > 0
-			&& now > m_requested + seconds(m_settings.get_int(settings_pack::request_timeout)
+			&& now > m_requested + seconds(m_settings.get_int(settings_pack::request_timeout)))
-			+ m_timeout_extend))
 		{
 			snub_peer();
 		}
@@ -4713,16 +4709,16 @@ namespace libtorrent
 
 		if (!m_download_queue.empty()
 			&& m_quota[download_channel] > 0
-			&& now - m_last_piece > seconds(piece_timeout + m_timeout_extend))
+			&& now - m_last_piece > seconds(piece_timeout))
 		{
 			// this peer isn't sending the pieces we've
 			// requested (this has been observed by BitComet)
 			// in this case we'll clear our download queue and
 			// re-request the blocks.
 #if defined TORRENT_VERBOSE_LOGGING || defined TORRENT_ERROR_LOGGING
-			peer_log("*** PIECE_REQUEST TIMED OUT [ %d time: %d to: %d extend: %d ]"
+			peer_log("*** PIECE_REQUEST TIMED OUT [ %d time: %d to: %d ]"
 				, (int)m_download_queue.size(), total_seconds(now - m_last_piece)
-				, piece_timeout, m_timeout_extend);
+				, piece_timeout);
 #endif
 
 			snub_peer();
@@ -4766,11 +4762,8 @@ namespace libtorrent
 		}
 		m_desired_queue_size = 1;
 
-		if (on_parole())
+		if (on_parole()) return;
-		{
+
-			m_timeout_extend += m_settings.get_int(settings_pack::request_timeout);
-			return;
-		}
 		if (!t->has_picker()) return;
 		piece_picker& picker = t->picker();
 
@@ -4785,21 +4778,6 @@ namespace libtorrent
 
 		TORRENT_ASSERT(!m_download_queue.empty());
 
-		// request a new block before removing the previous
-		// one, in order to prevent it from
-		// picking the same block again, stalling the
-		// same piece indefinitely.
-		m_desired_queue_size = 2;
-		if (request_a_block(*t, *this))
-			m_counters.inc_stats_counter(counters::snubbed_piece_picks);
-
-		// the block we just picked (potentially)
-		// hasn't been put in m_download_queue yet.
-		// it's in m_request_queue and will be sent
-		// once send_block_requests() is called.
-
-		m_desired_queue_size = 1;
-
 		// time out the last request eligible
 		// block in the queue
 		int i = m_download_queue.size() - 1;
@@ -4815,16 +4793,19 @@ namespace libtorrent
 			pending_block& qe = m_download_queue[i];
 			piece_block r = qe.block;
 
-			// only time out a request if it blocks the piece
+			// only cancel a request if it blocks the piece from being completed
-			// from being completed (i.e. no free blocks to
+			// (i.e. no free blocks to request from it)
-			// request from it)
 			piece_picker::downloading_piece p;
 			picker.piece_info(qe.block.piece_index, p);
 			int free_blocks = picker.blocks_in_piece(qe.block.piece_index)
 				- p.finished - p.writing - p.requested;
+
+			// if there are still blocks available for other peers to pick, we're
+			// still not holding up the completion of the piece and there's no
+			// need to cancel the requests. For more information, see:
+			// http://blog.libtorrent.org/2011/11/block-request-time-outs/
 			if (free_blocks > 0)
 			{
-				m_timeout_extend += m_settings.get_int(settings_pack::request_timeout);
 				send_block_requests();
 				return;
 			}
@@ -4834,6 +4815,22 @@ namespace libtorrent
 				t->alerts().post_alert(block_timeout_alert(t->get_handle()
 					, remote(), pid(), qe.block.block_index, qe.block.piece_index));
 			}
+
+			// request a new block before removing the previous
+			// one, in order to prevent it from
+			// picking the same block again, stalling the
+			// same piece indefinitely.
+			m_desired_queue_size = 2;
+			if (request_a_block(*t, *this))
+				m_counters.inc_stats_counter(counters::snubbed_piece_picks);
+
+			// the block we just picked (potentially)
+			// hasn't been put in m_download_queue yet.
+			// it's in m_request_queue and will be sent
+			// once send_block_requests() is called.
+
+			m_desired_queue_size = 1;
+
 			qe.timed_out = true;
 			picker.abort_download(r, peer_info_struct());
 		}