From a4dfd63ca3aaa482c81ccb40aeaac52757d65d80 Mon Sep 17 00:00:00 2001
From: Arvid Norberg <arvid@libtorrent.org>
Date: Wed, 11 Jun 2008 08:30:06 +0000
Subject: [PATCH] fixed piece picker bug related to sequential download, added
 unit test to expose it

---
 src/piece_picker.cpp       | 23 ++++++++++++-----------
 test/test_piece_picker.cpp | 31 +++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/src/piece_picker.cpp b/src/piece_picker.cpp
index f0b7fb937..7de18a326 100755
--- a/src/piece_picker.cpp
+++ b/src/piece_picker.cpp
@@ -37,9 +37,6 @@ POSSIBILITY OF SUCH DAMAGE.
 #include <algorithm>
 #include <numeric>
 
-// non-standard header, is_sorted()
-//#include <algo.h>
-
 #include "libtorrent/piece_picker.hpp"
 #include "libtorrent/aux_/session_impl.hpp"
 #include "libtorrent/bitfield.hpp"
@@ -255,6 +252,7 @@ namespace libtorrent
 		TORRENT_ASSERT(m_num_have >= 0);
 		TORRENT_ASSERT(m_num_have_filtered >= 0);
 		TORRENT_ASSERT(m_num_filtered >= 0);
+		TORRENT_ASSERT(m_seeds >= 0);
 
 		if (!m_downloads.empty())
 		{
@@ -446,6 +444,7 @@ namespace libtorrent
 
 	float piece_picker::distributed_copies() const
 	{
+		TORRENT_ASSERT(m_seeds >= 0);
 		const float num_pieces = static_cast<float>(m_piece_map.size());
 
 		int min_availability = piece_pos::max_peer_count;
@@ -768,8 +767,12 @@ namespace libtorrent
 		int new_priority = p.priority(this);
 
 		if (new_priority == prev_priority) return;
-		if (m_sequential_download >= 0) return;
 		if (m_dirty) return;
+		if (m_sequential_download >= 0)
+		{
+			m_dirty = true;
+			return;
+		}
 		if (prev_priority == -1)
 		{
 			add(index);
@@ -784,7 +787,6 @@ namespace libtorrent
 	{
 		TORRENT_PIECE_PICKER_INVARIANT_CHECK;
 		++m_seeds;
-		if (m_sequential_download >= 0) return;
 		if (m_seeds == 1)
 		{
 			// when m_seeds is increased from 0 to 1
@@ -798,12 +800,6 @@ namespace libtorrent
 	{
 		TORRENT_PIECE_PICKER_INVARIANT_CHECK;
 
-		if (m_sequential_download >= 0)
-		{
-			--m_seeds;
-			return;
-		}
-		
 		if (m_seeds > 0)
 		{
 			--m_seeds;
@@ -816,6 +812,7 @@ namespace libtorrent
 			}
 			return;
 		}
+		TORRENT_ASSERT(m_seeds == 0);
 
 		for (std::vector<piece_pos>::iterator i = m_piece_map.begin()
 			, end(m_piece_map.end()); i != end; ++i)
@@ -835,6 +832,7 @@ namespace libtorrent
 		if (m_sequential_download >= 0)
 		{
 			++p.peer_count;
+			m_dirty = true;
 			return;
 		}
 	
@@ -856,7 +854,9 @@ namespace libtorrent
 		piece_pos& p = m_piece_map[index];
 		if (m_sequential_download >= 0)
 		{
+			TORRENT_ASSERT(p.peer_count > 0);
 			--p.peer_count;
+			m_dirty = true;
 			return;
 		}
 		int prev_priority = p.priority(this);
@@ -1756,6 +1756,7 @@ namespace libtorrent
 	
 	void piece_picker::get_availability(std::vector<int>& avail) const
 	{
+		TORRENT_ASSERT(m_seeds >= 0);
 		TORRENT_PIECE_PICKER_INVARIANT_CHECK;
 	
 		avail.resize(m_piece_map.size());
diff --git a/test/test_piece_picker.cpp b/test/test_piece_picker.cpp
index d65fb6259..f3d6bbb9b 100644
--- a/test/test_piece_picker.cpp
+++ b/test/test_piece_picker.cpp
@@ -437,6 +437,37 @@ int test_main()
 	dc = p->distributed_copies();
 	std::cout << "distributed copies: " << dc << std::endl;
 	TEST_CHECK(fabs(dc - (1.f + 5.f / 7.f)) < 0.01f);
+	p->inc_refcount(0);
+	p->dec_refcount_all();
+	dc = p->distributed_copies();
+	std::cout << "distributed copies: " << dc << std::endl;
+	TEST_CHECK(fabs(dc - (0.f + 6.f / 7.f)) < 0.01f);
+	TEST_CHECK(test_pick(p) == 2);
+
+// ========================================================
+	
+	// test have_all and have_none with sequential download
+	print_title("test have_all and have_none with sequential download");
+	p = setup_picker("0123333", "*      ", "", "");
+	dc = p->distributed_copies();
+	std::cout << "distributed copies: " << dc << std::endl;
+	TEST_CHECK(fabs(dc - (1.f + 5.f / 7.f)) < 0.01f);
+	p->inc_refcount_all();
+	dc = p->distributed_copies();
+	std::cout << "distributed copies: " << dc << std::endl;
+	TEST_CHECK(fabs(dc - (2.f + 5.f / 7.f)) < 0.01f);
+	p->sequential_download(true);
+	p->dec_refcount_all();
+	dc = p->distributed_copies();
+	std::cout << "distributed copies: " << dc << std::endl;
+	TEST_CHECK(fabs(dc - (1.f + 5.f / 7.f)) < 0.01f);
+	p->inc_refcount(0);
+	p->dec_refcount_all();
+	dc = p->distributed_copies();
+	std::cout << "distributed copies: " << dc << std::endl;
+	TEST_CHECK(fabs(dc - (0.f + 6.f / 7.f)) < 0.01f);
+	p->inc_refcount(1);
+	TEST_CHECK(test_pick(p) == 1);
 
 // ========================================================