fix peer picking algorithm (#1141)

fix peer picking algorithm. The old code was always picking the first to_pick peers from the set.

src/kademlia/dht_storage.cpp

@@ -241,22 +241,28 @@ namespace
 			}
 			else
 			{
-				int num = (std::min)(int(v.peers.size()), m_settings.max_peers_reply);
+				int to_pick = m_settings.max_peers_reply;
+				int candidates = int(v.peers.size());
 				std::set<peer_entry>::const_iterator iter = v.peers.begin();
 				entry::list_type& pe = peers["values"].list();
 				std::string endpoint;
 
-				for (int t = 0, m = 0; m < num && iter != v.peers.end(); ++iter, ++t)
+				for (; to_pick > 0 && iter != v.peers.end(); ++iter, --candidates)
 				{
-					if ((random() / float(UINT_MAX + 1.f)) * (num - t) >= num - m) continue;
 					if (noseed && iter->seed) continue;
+
+					// pick this peer with probability
+					// <peers left to pick> / <peers left in the set>
+					if (random() % candidates > to_pick)
+						continue;
+
 					endpoint.resize(18);
 					std::string::iterator out = endpoint.begin();
 					write_endpoint(iter->addr, out);
 					endpoint.resize(out - endpoint.begin());
 					pe.push_back(entry(endpoint));
 
-					++m;
+					--to_pick;
 				}
 			}
 			return true;

test/test_dht_storage.cpp

@@ -316,5 +316,43 @@ TORRENT_TEST(mutable_item_limit)
 	TEST_EQUAL(cnt.mutable_data, 42);
 }
 
+TORRENT_TEST(get_peers_dist)
+{
+	// test that get_peers returns reasonably disjoint sets of peers with each call
+	// take two samples of 100 peers from 1000 and make sure there aren't too many
+	// peers found in both lists
+	dht_settings sett = test_settings();
+	sett.max_peers = 1000;
+	sett.max_peers_reply = 100;
+	boost::scoped_ptr<dht_storage_interface> s(dht_default_storage_constructor(node_id(0), sett));
+
+	address addr = rand_v4();
+	for (int i = 0; i < 1000; ++i)
+	{
+		s->announce_peer(n1, tcp::endpoint(addr, uint16_t(i))
+			, "torrent_name", false);
+	}
+
+	std::set<int> peer_set;
+	int duplicates = 0;
+	for (int i = 0; i < 2; ++i)
+	{
+		entry peers;
+		s->get_peers(n1, false, false, peers);
+		TEST_EQUAL(peers["values"].list().size(), 100);
+		entry::list_type const& peers_list = peers["values"].list();
+		for (entry::list_type::const_iterator p = peers_list.begin();
+			p != peers_list.end(); ++p)
+		{
+			std::string::const_iterator it = p->string().begin();
+			int port = detail::read_v4_endpoint<tcp::endpoint>(it).port();
+			if (!peer_set.insert(port).second)
+				++duplicates;
+		}
+	}
+	std::printf("duplicate peers found: %d\n", duplicates);
+	TEST_CHECK(duplicates < 20);
+}
+
 #endif