made the dht be a fallback by default, fixed glitch in earlier fix to make sure block requests are sent properly, fixed problem in peer_connection::second_tick if the piece picker was removed because of the torrent becoming a seed

ChangeLog

@@ -1,3 +1,5 @@
+	* metadata extension now respects the private flag in the torrent.
+	* made the DHT to only be used as a fallback to trackers by default.
 	* added support for HTTP redirection support for web seeds.
 	* fixed race condition when accessing a torrent that was checking its
 	  fast resume data.

docs/manual.html

@@ -2004,6 +2004,7 @@ struct session_settings
         int urlseed_pipeline_size;
         int file_pool_size;
         bool allow_multiple_connections_per_ip;
+        bool use_dht_as_fallback;
 };
 </pre>
 <p><tt class="docutils literal"><span class="pre">proxy_ip</span></tt> may be a hostname or ip to a http proxy to use. If this is
@@ -2078,6 +2079,10 @@ connections from the same IP address is not allowed by default, to prevent
 abusive behavior by peers. It may be useful to allow such connections in
 cases where simulations are run on the same machie, and all peers in a
 swarm has the same IP address.</p>
+<p><tt class="docutils literal"><span class="pre">use_dht_as_fallback</span></tt> determines how the DHT is used. If this is true
+(which it is by default), the DHT will only be used for torrents where
+all trackers in its tracker list has failed. Either by an explicit error
+message or a time out.</p>
 </div>
 <div class="section">
 <h1><a id="ip-filter" name="ip-filter">ip_filter</a></h1>

docs/manual.rst

@@ -1995,6 +1995,7 @@ that will be sent to the tracker. The user-agent is a good way to identify your
 		int urlseed_pipeline_size;
 		int file_pool_size;
 		bool allow_multiple_connections_per_ip;
+		bool use_dht_as_fallback;
 	};
 
 ``proxy_ip`` may be a hostname or ip to a http proxy to use. If this is
@@ -2087,6 +2088,11 @@ abusive behavior by peers. It may be useful to allow such connections in
 cases where simulations are run on the same machie, and all peers in a
 swarm has the same IP address.
 
+``use_dht_as_fallback`` determines how the DHT is used. If this is true
+(which it is by default), the DHT will only be used for torrents where
+all trackers in its tracker list has failed. Either by an explicit error
+message or a time out.
+
 ip_filter
 =========
 

include/libtorrent/peer_connection.hpp

@@ -292,6 +292,10 @@ namespace libtorrent
 
 		// adds a block to the request queue
 		void add_request(piece_block const& b);
+		// removes a block from the request queue or download queue
+		// sends a cancel message if appropriate
+		// refills the request queue, and possibly ignoring pieces requested
+		// by peers in the ignore list (to avoid recursion)
 		void cancel_request(piece_block const& b);
 		void send_block_requests();
 

include/libtorrent/policy.hpp

@@ -69,6 +69,10 @@ namespace libtorrent
 		free_upload_amount = 4 * 16 * 1024
 	};
 
+	void request_a_block(
+		torrent& t
+		, peer_connection& c
+		, std::vector<peer_connection*> ignore = std::vector<peer_connection*>());
 
 	class TORRENT_EXPORT policy
 	{

include/libtorrent/session_settings.hpp

@@ -58,6 +58,7 @@ namespace libtorrent
 			, urlseed_pipeline_size(5)
 			, file_pool_size(40)
 			, allow_multiple_connections_per_ip(false)
+			, use_dht_as_fallback(true)
 		{}
 
 		std::string proxy_ip;
@@ -150,6 +151,12 @@ namespace libtorrent
 		// false to not allow multiple connections from the same
 		// IP address. true will allow it.
 		bool allow_multiple_connections_per_ip;
+
+#ifndef TORRENT_DISABLE_DHT
+		// while this is true, the dht will note be used unless the
+		// tracker is online
+		bool use_dht_as_fallback;
+#endif
 	};
 	
 #ifndef TORRENT_DISABLE_DHT

include/libtorrent/torrent.hpp

@@ -548,6 +548,7 @@ namespace libtorrent
 		deadline_timer m_dht_announce_timer;
 		void on_dht_announce(asio::error_code const& e);
 		void on_dht_announce_response(std::vector<tcp::endpoint> const& peers);
+		bool should_announce_dht() const;
 #endif
 
 		// this is the upload and download statistics for the whole torrent.

src/metadata_transfer.cpp

@@ -315,7 +315,8 @@ namespace libtorrent { namespace
 			// abort if the peer doesn't support the metadata extension
 			if (m_message_index == 0) return;
 
-			if (m_torrent.valid_metadata())
+			// only send metadata if the torrent is non-private
+			if (m_torrent.valid_metadata() && !m_torrent.torrent_file().priv())
 			{
 				std::pair<int, int> offset
 					= req_to_offset(req, (int)m_tp.metadata().size());

src/peer_connection.cpp

@@ -985,6 +985,11 @@ namespace libtorrent
 				if (pc && pc != this)
 				{
 					pc->cancel_request(block_finished);
+					if (!pc->has_peer_choked() && !t->is_seed())
+					{
+						request_a_block(*t, *pc);
+						pc->send_block_requests();
+					}
 				}
 			}
 			else
@@ -1181,10 +1186,6 @@ namespace libtorrent
 			assert(it != m_request_queue.end());
 			if (it == m_request_queue.end()) return;
 			m_request_queue.erase(it);
-			
-			policy& pol = t->get_policy();
-			pol.block_finished(*this, block);
-			send_block_requests();
 			// since we found it in the request queue, it means it hasn't been
 			// sent yet, so we don't have to send a cancel.
 			return;
@@ -1208,10 +1209,6 @@ namespace libtorrent
 
 		write_cancel(r);
 
-		policy& pol = t->get_policy();
-		pol.block_finished(*this, block);
-		send_block_requests();
-
 #ifdef TORRENT_VERBOSE_LOGGING
 		using namespace boost::posix_time;
 		(*m_logger) << to_simple_string(second_clock::universal_time())
@@ -1545,25 +1542,33 @@ namespace libtorrent
 				<< " " << to_simple_string(now - m_last_piece) << "] ***\n";
 #endif
 
-			piece_picker& picker = t->picker();
+			if (t->is_seed())
-			while (!m_download_queue.empty())
 			{
-				picker.abort_download(m_download_queue.back());
+				m_download_queue.clear();
-				m_download_queue.pop_back();
+				m_request_queue.clear();
 			}
-			while (!m_request_queue.empty())
+			else
 			{
-				picker.abort_download(m_request_queue.back());
+				piece_picker& picker = t->picker();
-				m_request_queue.pop_back();
+				while (!m_download_queue.empty())
+				{
+					picker.abort_download(m_download_queue.back());
+					m_download_queue.pop_back();
+				}
+				while (!m_request_queue.empty())
+				{
+					picker.abort_download(m_request_queue.back());
+					m_request_queue.pop_back();
+				}
+
+				// TODO: If we have a limited number of upload
+				// slots, choke this peer
+				
+				m_assume_fifo = true;
+
+				request_a_block(*t, *this);
+				send_block_requests();
 			}
-
-			// TODO: If we have a limited number of upload
-			// slots, choke this peer
-			
-			m_assume_fifo = true;
-
-			// this will trigger new picking of pieces
-			t->get_policy().unchoked(*this);
 		}
 
 		m_statistics.second_tick(tick_interval);

src/policy.cpp

@@ -65,6 +65,102 @@ namespace
 {
 	using namespace libtorrent;
 
+	size_type collect_free_download(
+		torrent::peer_iterator start
+		, torrent::peer_iterator end)
+	{
+		size_type accumulator = 0;
+		for (torrent::peer_iterator i = start; i != end; ++i)
+		{
+			// if the peer is interested in us, it means it may
+			// want to trade it's surplus uploads for downloads itself
+			// (and we should not consider it free). If the share diff is
+			// negative, there's no free download to get from this peer.
+			size_type diff = i->second->share_diff();
+			assert(diff < std::numeric_limits<size_type>::max());
+			if (i->second->is_peer_interested() || diff <= 0)
+				continue;
+
+			assert(diff > 0);
+			i->second->add_free_upload(-diff);
+			accumulator += diff;
+			assert(accumulator > 0);
+		}
+		assert(accumulator >= 0);
+		return accumulator;
+	}
+
+
+	// returns the amount of free upload left after
+	// it has been distributed to the peers
+	size_type distribute_free_upload(
+		torrent::peer_iterator start
+		, torrent::peer_iterator end
+		, size_type free_upload)
+	{
+		if (free_upload <= 0) return free_upload;
+		int num_peers = 0;
+		size_type total_diff = 0;
+		for (torrent::peer_iterator i = start; i != end; ++i)
+		{
+			size_type d = i->second->share_diff();
+			assert(d < std::numeric_limits<size_type>::max());
+			total_diff += d;
+			if (!i->second->is_peer_interested() || i->second->share_diff() >= 0) continue;
+			++num_peers;
+		}
+
+		if (num_peers == 0) return free_upload;
+		size_type upload_share;
+		if (total_diff >= 0)
+		{
+			upload_share = std::min(free_upload, total_diff) / num_peers;
+		}
+		else
+		{
+			upload_share = (free_upload + total_diff) / num_peers;
+		}
+		if (upload_share < 0) return free_upload;
+
+		for (torrent::peer_iterator i = start; i != end; ++i)
+		{
+			peer_connection* p = i->second;
+			if (!p->is_peer_interested() || p->share_diff() >= 0) continue;
+			p->add_free_upload(upload_share);
+			free_upload -= upload_share;
+		}
+		return free_upload;
+	}
+
+	struct match_peer_ip
+	{
+		match_peer_ip(tcp::endpoint const& ip)
+			: m_ip(ip)
+		{}
+
+		bool operator()(policy::peer const& p) const
+		{ return p.ip.address() == m_ip.address(); }
+
+		tcp::endpoint m_ip;
+	};
+
+	struct match_peer_connection
+	{
+		match_peer_connection(peer_connection const& c)
+			: m_conn(c)
+		{}
+
+		bool operator()(policy::peer const& p) const
+		{ return p.connection == &m_conn; }
+
+		const peer_connection& m_conn;
+	};
+
+
+}
+
+namespace libtorrent
+{
 	// the case where ignore_peer is motivated is if two peers
 	// have only one piece that we don't have, and it's the
 	// same piece for both peers. Then they might get into an
@@ -72,9 +168,10 @@ namespace
 	void request_a_block(
 		torrent& t
 		, peer_connection& c
-		, std::vector<peer_connection*> ignore = std::vector<peer_connection*>())
+		, std::vector<peer_connection*> ignore)
 	{
 		assert(!t.is_seed());
+		assert(!c.has_peer_choked());
 		int num_requests = c.desired_queue_size()
 			- (int)c.download_queue().size()
 			- (int)c.request_queue().size();
@@ -245,15 +342,20 @@ namespace
 			}
 
 			piece_block block = *common_block;
-			peer->cancel_request(block);
-			c.add_request(block);
 
 			// the one we interrupted may need to request a new piece.
 			// make sure it doesn't take over a block from the peer
 			// that just took over its block (that would cause an
 			// infinite recursion)
+			peer->cancel_request(block);
+			c.add_request(block);
 			ignore.push_back(&c);
-			request_a_block(t, *peer, ignore);
+			if (!peer->has_peer_choked() && !t.is_seed())
+			{
+				request_a_block(t, *peer, ignore);
+				peer->send_block_requests();
+			}
+
 			num_requests--;
 
 			const int queue_size = (int)c.download_queue().size()
@@ -269,107 +371,8 @@ namespace
 		c.send_block_requests();
 	}
 
-
-	size_type collect_free_download(
-		torrent::peer_iterator start
-		, torrent::peer_iterator end)
-	{
-		size_type accumulator = 0;
-		for (torrent::peer_iterator i = start; i != end; ++i)
-		{
-			// if the peer is interested in us, it means it may
-			// want to trade it's surplus uploads for downloads itself
-			// (and we should not consider it free). If the share diff is
-			// negative, there's no free download to get from this peer.
-			size_type diff = i->second->share_diff();
-			assert(diff < std::numeric_limits<size_type>::max());
-			if (i->second->is_peer_interested() || diff <= 0)
-				continue;
-
-			assert(diff > 0);
-			i->second->add_free_upload(-diff);
-			accumulator += diff;
-			assert(accumulator > 0);
-		}
-		assert(accumulator >= 0);
-		return accumulator;
-	}
-
-
-	// returns the amount of free upload left after
-	// it has been distributed to the peers
-	size_type distribute_free_upload(
-		torrent::peer_iterator start
-		, torrent::peer_iterator end
-		, size_type free_upload)
-	{
-		if (free_upload <= 0) return free_upload;
-		int num_peers = 0;
-		size_type total_diff = 0;
-		for (torrent::peer_iterator i = start; i != end; ++i)
-		{
-			size_type d = i->second->share_diff();
-			assert(d < std::numeric_limits<size_type>::max());
-			total_diff += d;
-			if (!i->second->is_peer_interested() || i->second->share_diff() >= 0) continue;
-			++num_peers;
-		}
-
-		if (num_peers == 0) return free_upload;
-		size_type upload_share;
-		if (total_diff >= 0)
-		{
-			upload_share = std::min(free_upload, total_diff) / num_peers;
-		}
-		else
-		{
-			upload_share = (free_upload + total_diff) / num_peers;
-		}
-		if (upload_share < 0) return free_upload;
-
-		for (torrent::peer_iterator i = start; i != end; ++i)
-		{
-			peer_connection* p = i->second;
-			if (!p->is_peer_interested() || p->share_diff() >= 0) continue;
-			p->add_free_upload(upload_share);
-			free_upload -= upload_share;
-		}
-		return free_upload;
-	}
-
-	struct match_peer_ip
-	{
-		match_peer_ip(tcp::endpoint const& ip)
-			: m_ip(ip)
-		{}
-
-		bool operator()(policy::peer const& p) const
-		{ return p.ip.address() == m_ip.address(); }
-
-		tcp::endpoint m_ip;
-	};
-
-	struct match_peer_connection
-	{
-		match_peer_connection(peer_connection const& c)
-			: m_conn(c)
-		{}
-
-		bool operator()(policy::peer const& p) const
-		{ return p.connection == &m_conn; }
-
-		const peer_connection& m_conn;
-	};
-
-
-}
-
-namespace libtorrent
-{
 	policy::policy(torrent* t)
 		: m_torrent(t)
-//		, m_max_uploads(std::numeric_limits<int>::max())
-//		, m_max_connections(std::numeric_limits<int>::max())
 		, m_num_unchoked(0)
 		, m_available_free_upload(0)
 		, m_last_optimistic_disconnect(boost::gregorian::date(1970,boost::gregorian::Jan,1))
@@ -451,7 +454,6 @@ namespace libtorrent
 			if (c->share_diff() < -free_upload_amount
 				&& m_torrent->ratio() != 0) continue;
 			if (c->statistics().download_rate() < max_down_speed) continue;
-//			if (i->last_optimistically_unchoked > min_time) continue;
 
 			min_time = i->last_optimistically_unchoked;
 			max_down_speed = c->statistics().download_rate();
@@ -647,10 +649,6 @@ namespace libtorrent
 
 		using namespace boost::posix_time;
 
-		// TODO: we must also remove peers that
-		// we failed to connect to from this list
-		// to avoid being part of a DDOS-attack
-
 		// remove old disconnected peers from the list
 		m_peers.erase(
 			std::remove_if(m_peers.begin()

src/torrent.cpp

@@ -296,7 +296,7 @@ namespace libtorrent
 		init();
 	
 #ifndef TORRENT_DISABLE_DHT
-		if (!tf.priv())
+		if (should_announce_dht())
 		{
 			m_dht_announce_timer.expires_from_now(seconds(10));
 			m_dht_announce_timer.async_wait(m_ses.m_strand.wrap(
@@ -384,12 +384,27 @@ namespace libtorrent
 		m_policy.reset(new policy(this));
 		m_torrent_file.add_tracker(tracker_url);
 #ifndef TORRENT_DISABLE_DHT
-		m_dht_announce_timer.expires_from_now(seconds(10));
+		if (should_announce_dht())
-		m_dht_announce_timer.async_wait(m_ses.m_strand.wrap(
+		{
-			bind(&torrent::on_dht_announce, this, _1)));
+			m_dht_announce_timer.expires_from_now(seconds(10));
+			m_dht_announce_timer.async_wait(m_ses.m_strand.wrap(
+				bind(&torrent::on_dht_announce, this, _1)));
+		}
 #endif
 	}
 
+#ifndef TORRENT_DISABLE_DHT
+	bool torrent::should_announce_dht() const
+	{
+		// don't announce private torrents
+		if (m_torrent_file.is_valid() && m_torrent_file.priv()) return false;
+	
+		if (m_trackers.empty()) return true;
+			
+		return m_failed_trackers > 0 || !m_ses.settings().use_dht_as_fallback;
+	}
+#endif
+
 	torrent::~torrent()
 	{
 		// The invariant can't be maintained here, since the torrent
@@ -465,9 +480,12 @@ namespace libtorrent
 	void torrent::on_dht_announce(asio::error_code const& e)
 	{
 		if (e) return;
-		m_dht_announce_timer.expires_from_now(boost::posix_time::minutes(30));
+		if (should_announce_dht())
-		m_dht_announce_timer.async_wait(m_ses.m_strand.wrap(
+		{
-			bind(&torrent::on_dht_announce, this, _1)));
+			m_dht_announce_timer.expires_from_now(boost::posix_time::minutes(30));
+			m_dht_announce_timer.async_wait(m_ses.m_strand.wrap(
+				bind(&torrent::on_dht_announce, this, _1)));
+		}
 		if (!m_ses.m_dht) return;
 		// TODO: There should be a way to abort an announce operation on the dht.
 		// when the torrent is destructed
@@ -1968,6 +1986,19 @@ namespace libtorrent
 			// if we've looped the tracker list, wait a bit before retrying
 			m_currently_trying_tracker = 0;
 			m_next_request = second_clock::universal_time() + seconds(delay);
+
+#ifndef DISABLE_DHT_SUPPORT
+			// only start the dht announce unless we already are already running
+			// the announce timer (a positive expiration time indicates
+			// that it's running)
+			if (m_dht_announce_timer.expires_from_now().is_negative() && should_announce_dht())
+			{
+				m_dht_announce_timer.expires_from_now(boost::posix_time::seconds(1));
+				m_dht_announce_timer.async_wait(m_ses.m_strand.wrap(
+					bind(&torrent::on_dht_announce, this, _1)));
+			}
+#endif
+
 		}
 		else
 		{