fix issue when peer list is full, previously we would sometimes reject new connections

include/libtorrent/aux_/session_impl.hpp

@@ -944,6 +944,9 @@ namespace libtorrent
 			int m_connect_timeouts;
 			int m_uninteresting_peers;
 			int m_timeout_peers;
+			int m_no_memory_peers;
+			int m_too_many_peers;
+			int m_transport_timeout_peers;
 			cache_status m_last_cache_status;
 			size_type m_last_failed;
 			size_type m_last_redundant;

include/libtorrent/policy.hpp

@@ -108,10 +108,6 @@ namespace libtorrent
 
 		policy(torrent* t);
 
-		// this is called every 10 seconds to allow
-		// for peer choking management
-		void pulse();
-
 		struct peer;
 
 #if TORRENT_USE_I2P
@@ -433,9 +429,11 @@ namespace libtorrent
 
 		bool is_connect_candidate(peer const& p, bool finished) const;
 		bool is_erase_candidate(peer const& p, bool finished) const;
+		bool is_force_erase_candidate(peer const& pe) const;
 		bool should_erase_immediately(peer const& p) const;
 
-		void erase_peers();
+		enum flags_t { force_erase = 1 };
+		void erase_peers(int flags = 0);
 
 		peers_t m_peers;
 

parse_session_stats.py

@@ -132,7 +132,7 @@ reports = [
 	('disk_write_queue', 'Bytes', 'B', 'bytes queued up by peers, to be written to disk', ['disk write queued bytes', 'disk queue limit', 'disk queue low watermark']),
 	('peers_upload', 'num', '', 'number of peers by state wrt. uploading', ['peers up interested', 'peers up unchoked', 'peers up requests', 'peers disk-up', 'peers bw-up', 'max unchoked']),
 	('peers_download', 'num', '', 'number of peers by state wrt. downloading', ['peers down interesting', 'peers down unchoked', 'peers down requests', 'peers disk-down', 'peers bw-down','num end-game peers']),
-	('peer_errors', 'num', '', 'number of peers by error that disconnected them', ['error peers', 'peer disconnects', 'peers eof', 'peers connection reset', 'connect timeouts', 'uninteresting peers disconnect', 'banned for hash failure']),
+	('peer_errors', 'num', '', 'number of peers by error that disconnected them', ['error peers', 'peer disconnects', 'peers eof', 'peers connection reset', 'connect timeouts', 'uninteresting peers disconnect', 'banned for hash failure', 'no memory peer errors', 'too many peers', 'transport timeout peers']),
 	('waste', '% of all downloaded bytes', '%%', 'proportion of all downloaded bytes that were wasted', ['% failed payload bytes', '% wasted payload bytes', '% protocol bytes']),
 	('waste by source', '% of all wasted bytes', '%%', 'what\' causing the waste', [ 'redundant timed-out', 'redundant cancelled', 'redundant unknown', 'redundant seed', 'redundant end-game', 'redundant closing']),
 	('average_disk_time_absolute', 'job time', 's', 'running averages of timings of disk operations', ['disk read time', 'disk write time', 'disk hash time', 'disk job time', 'disk sort time']),

src/peer_connection.cpp

@@ -3454,12 +3454,21 @@ namespace libtorrent
 			|| ec == error_code(errors::self_connection)
 			|| ec == error_code(errors::torrent_paused))
 			++m_ses.m_uninteresting_peers;
+
+		if (ec == error_code(errors::timed_out))
+			++m_ses.m_transport_timeout_peers;
 		
 		if (ec == error_code(errors::timed_out_inactivity)
 			|| ec == error_code(errors::timed_out_no_request)
 			|| ec == error_code(errors::timed_out_no_interest))
 			++m_ses.m_timeout_peers;
 
+		if (ec == error_code(errors::no_memory))
+			++m_ses.m_no_memory_peers;
+
+		if (ec == error_code(errors::too_many_connections))
+			++m_ses.m_too_many_peers;
+
 		if (ec == error_code(errors::timed_out_no_handshake))
 			++m_ses.m_connect_timeouts;
 #endif

src/policy.cpp

@@ -459,20 +459,24 @@ namespace libtorrent
 
 	bool policy::should_erase_immediately(peer const& p) const
 	{
-		return p.source == peer_info::resume_data
-			&& p.failcount > 0
-			&& !p.banned;
+		return p.source == peer_info::resume_data;
 	}
 
 	bool policy::is_erase_candidate(peer const& pe, bool finished) const
 	{
-		return pe.connection == 0
-			&& pe.last_connected != 0
-			&& !pe.banned
-			&& !is_connect_candidate(pe, m_finished);
+		if (pe.connection) return false;
+		if (is_connect_candidate(pe, finished)) return false;
+
+		return (pe.failcount > 0)
+			|| (pe.source == peer_info::resume_data);
 	}
 
-	void policy::erase_peers()
+	bool policy::is_force_erase_candidate(peer const& pe) const
+	{
+		return pe.connection == 0;
+	}
+
+	void policy::erase_peers(int flags)
 	{
 		INVARIANT_CHECK;
 
@@ -483,15 +487,19 @@ namespace libtorrent
 		if (max_peerlist_size == 0 || m_peers.empty()) return;
 
 		int erase_candidate = -1;
+		int force_erase_candidate = -1;
 
 		TORRENT_ASSERT(m_finished == m_torrent->is_finished());
 
 		int round_robin = random() % m_peers.size();
 
+		int low_watermark = max_peerlist_size * 95 / 100;
+		if (low_watermark == max_peerlist_size) --low_watermark;
+
 		for (int iterations = (std::min)(int(m_peers.size()), 300);
 			iterations > 0; --iterations)
 		{
-			if (int(m_peers.size()) < max_peerlist_size * 0.95)
+			if (int(m_peers.size()) < low_watermark)
 				break;
 
 			if (round_robin == int(m_peers.size())) round_robin = 0;
@@ -507,6 +515,7 @@ namespace libtorrent
 					if (should_erase_immediately(pe))
 					{
 						if (erase_candidate > current) --erase_candidate;
+						if (force_erase_candidate > current) --force_erase_candidate;
 						TORRENT_ASSERT(current >= 0 && current < int(m_peers.size()));
 						--round_robin;
 						erase_peer(m_peers.begin() + current);
@@ -516,6 +525,12 @@ namespace libtorrent
 						erase_candidate = current;
 					}
 				}
+				if (is_force_erase_candidate(pe)
+					&& (force_erase_candidate == -1
+						|| !compare_peer_erase(*m_peers[force_erase_candidate], pe)))
+				{
+					force_erase_candidate = current;
+				}
 			}
 
 			++round_robin;
@@ -526,6 +541,11 @@ namespace libtorrent
 			TORRENT_ASSERT(erase_candidate >= 0 && erase_candidate < int(m_peers.size()));
 			erase_peer(m_peers.begin() + erase_candidate);
 		}
+		else if ((flags & force_erase) && force_erase_candidate > -1)
+		{
+			TORRENT_ASSERT(force_erase_candidate >= 0 && force_erase_candidate < int(m_peers.size()));
+			erase_peer(m_peers.begin() + force_erase_candidate);
+		}
 	}
 
 	void policy::ban_peer(policy::peer* p)
@@ -710,13 +730,6 @@ namespace libtorrent
 		return m_peers.begin() + candidate;
 	}
 
-	void policy::pulse()
-	{
-		INVARIANT_CHECK;
-
-		erase_peers();
-	}
-
 	bool policy::new_connection(peer_connection& c, int session_time)
 	{
 		TORRENT_ASSERT(!c.is_local());
@@ -739,6 +752,18 @@ namespace libtorrent
 			&& ses.num_connections() >= ses.settings().connections_limit
 			&& c.remote().address() != m_torrent->current_tracker().address())
 		{
+#if defined TORRENT_LOGGING || defined TORRENT_VERBOSE_LOGGING
+			(*m_torrent->session().m_logger) << time_now_string()
+				<< " *** TOO MANY CONNECTIONS ["
+				" torrent: " << m_torrent->name() <<
+				" torrent peers: " << m_torrent->num_peers() <<
+				" torrent limit: " << m_torrent->max_connections() <<
+				" global peers: " << ses.num_connections() <<
+				" global limit: " << ses.settings().connections_limit <<
+				" global list peers " << int(m_peers.size()) <<
+				" global list limit: " << m_torrent->settings().max_peerlist_size <<
+				" ]\n";
+#endif
 			c.disconnect(errors::too_many_connections);
 			return false;
 		}
@@ -859,9 +884,21 @@ namespace libtorrent
 			if (int(m_peers.size()) >= m_torrent->settings().max_peerlist_size)
 			{
 				// this may invalidate our iterator!
-				erase_peers();
+				erase_peers(force_erase);
 				if (int(m_peers.size()) >= m_torrent->settings().max_peerlist_size)
 				{
+#if defined TORRENT_LOGGING || defined TORRENT_VERBOSE_LOGGING
+					(*m_torrent->session().m_logger) << time_now_string()
+						<< " *** TOO MANY CONNECTIONS ["
+						" torrent: " << m_torrent->name() <<
+						" torrent peers: " << m_torrent->num_peers() <<
+						" torrent limit: " << m_torrent->max_connections() <<
+						" global peers: " << ses.num_connections() <<
+						" global limit: " << ses.settings().connections_limit <<
+						" global list peers " << int(m_peers.size()) <<
+						" global list limit: " << m_torrent->settings().max_peerlist_size <<
+						" ]\n";
+#endif
 					c.disconnect(errors::too_many_connections);
 					return false;
 				}
@@ -1678,6 +1715,13 @@ namespace libtorrent
 	// this returns true if lhs is a better erase candidate than rhs
 	bool policy::compare_peer_erase(policy::peer const& lhs, policy::peer const& rhs) const
 	{
+		TORRENT_ASSERT(lhs.connection == 0);
+		TORRENT_ASSERT(rhs.connection == 0);
+
+		// primarily, prefer getting rid of peers we've already tried and failed
+		if (lhs.failcount != rhs.failcount)
+			return lhs.failcount > rhs.failcount;
+
 		bool lhs_resume_data_source = lhs.source == peer_info::resume_data;
 		bool rhs_resume_data_source = rhs.source == peer_info::resume_data;
 
@@ -1689,7 +1733,10 @@ namespace libtorrent
 			return lhs.connectable < rhs.connectable;
 
 		// prefer peers with higher failcount
-		return lhs.failcount > rhs.failcount;
+		if (lhs.failcount != rhs.failcount)
+			return lhs.failcount > rhs.failcount;
+
+		return lhs.trust_points < rhs.trust_points;
 	}
 
 	// this returns true if lhs is a better connect candidate than rhs

src/session_impl.cpp

@@ -1145,6 +1145,9 @@ namespace aux {
 			":redundant seed"
 			":redundant end-game"
 			":redundant closing"
+			":no memory peer errors"
+			":too many peers"
+			":transport timeout peers"
 			"\n\n", m_stats_logger);
 	}
 #endif
@@ -3290,7 +3293,10 @@ namespace aux {
 		m_snubbed_piece_picks = 0;
 		m_connect_timeouts = 0;
 		m_uninteresting_peers = 0;
+		m_transport_timeout_peers = 0;
 		m_timeout_peers = 0;
+		m_no_memory_peers = 0;
+		m_too_many_peers = 0;
 		m_connection_attempts = 0;
 		m_num_banned_peers = 0;
 		m_banned_for_hash_failure = 0;
@@ -3624,6 +3630,10 @@ namespace aux {
 			for (int i = 0; i < torrent::waste_reason_max; ++i)
 				STAT_LOG(f, (m_redundant_bytes[i] * 100.) / double(m_total_redundant_bytes));
 
+			STAT_LOG(d, m_no_memory_peers);
+			STAT_LOG(d, m_too_many_peers);
+			STAT_LOG(d, m_transport_timeout_peers);
+
 			fprintf(m_stats_logger, "\n");
 
 #undef STAT_LOG

src/torrent.cpp

@@ -7187,8 +7187,6 @@ ctx->set_verify_callback(verify_function, ec);
 				m_available_free_upload = distribute_free_upload(
 					this->begin(), this->end(), m_available_free_upload);
 			}
-
-			m_policy.pulse();
 		}
 
 		// if we're in upload only mode and we're auto-managed