fix seed mode and suggest mode (#871)

fix resume data when combining seed-mode with suggest-mode

simulation/Jamfile

@@ -45,5 +45,6 @@ alias libtorrent-sims :
 	[ run test_ip_filter.cpp ]
 	[ run test_dht_rate_limit.cpp ]
 	[ run test_fast_extensions.cpp ]
+	[ run test_save_resume.cpp ]
 	;
 

simulation/setup_swarm.cpp

@@ -399,9 +399,10 @@ void setup_swarm(int num_nodes
 				, [](boost::shared_ptr<lt::session> const& s)
 				{ return is_seed(*s); });
 
-			if (tick > 70 * (num_nodes - 1) && !shut_down)
+			if (tick > 88 * (num_nodes - 1) && !shut_down)
 			{
 				TEST_ERROR("seeding failed!");
+				shut_down = true;
 			}
 		}
 

simulation/test_save_resume.cpp

@@ -0,0 +1,89 @@
+/*
+
+Copyright (c) 2016, Arvid Norberg
+All rights reserved.
+
+Redistribution and use in source and binary forms, with or without
+modification, are permitted provided that the following conditions
+are met:
+
+    * Redistributions of source code must retain the above copyright
+      notice, this list of conditions and the following disclaimer.
+    * Redistributions in binary form must reproduce the above copyright
+      notice, this list of conditions and the following disclaimer in
+      the documentation and/or other materials provided with the distribution.
+    * Neither the name of the author nor the names of its
+      contributors may be used to endorse or promote products derived
+      from this software without specific prior written permission.
+
+THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
+LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+POSSIBILITY OF SUCH DAMAGE.
+
+*/
+
+#include "setup_swarm.hpp"
+#include "test.hpp"
+#include "utils.hpp"
+#include "libtorrent/alert_types.hpp"
+#include "libtorrent/session.hpp"
+#include "settings.hpp"
+
+using namespace libtorrent;
+
+TORRENT_TEST(seed_and_suggest_mode)
+{
+	boost::shared_ptr<entry> resume_data;
+
+	// with seed mode
+	setup_swarm(2, swarm_test::upload
+		// add session
+		, [](lt::settings_pack& pack) {
+			pack.set_int(settings_pack::suggest_mode, settings_pack::suggest_read_cache);
+		}
+		// add torrent
+		, [](lt::add_torrent_params& params) {
+			params.flags |= add_torrent_params::flag_seed_mode;
+		}
+		// on alert
+		, [&](lt::alert const* a, lt::session&)
+		{
+			auto* sr = alert_cast<save_resume_data_alert>(a);
+			if (sr == nullptr) return;
+
+			resume_data = sr->resume_data;
+		}
+		// terminate
+		, [](int ticks, lt::session& ses) -> bool
+		{
+			if (ticks < 5) return false;
+			if (ticks == 5)
+			{
+				ses.get_torrents()[0].save_resume_data();
+				return false;
+			}
+			return true;
+		});
+
+	printf("save-resume: %s\n", resume_data->to_string().c_str());
+	TEST_CHECK((*resume_data)["seed_mode"] == 1);
+
+	// there should not be any pieces in a seed-mode torrent
+	entry const* pieces = resume_data->find_key("pieces");
+	TEST_CHECK(pieces != nullptr);
+	std::string piece_str = pieces->string();
+	for (char c : piece_str)
+	{
+		TEST_CHECK(c & 0x1);
+	}
+}
+
+

simulation/test_swarm.cpp

@@ -38,13 +38,15 @@ POSSIBILITY OF SUCH DAMAGE.
 #include "libtorrent/session.hpp"
 #include "libtorrent/session_stats.hpp"
 #include "libtorrent/file.hpp"
+#include "libtorrent/torrent_info.hpp"
+#include "settings.hpp"
 
 using namespace libtorrent;
 
 TORRENT_TEST(seed_mode)
 {
 	// with seed mode
-	setup_swarm(2, swarm_test::upload
+	setup_swarm(3, swarm_test::upload
 		// add session
 		, [](lt::settings_pack&) {}
 		// add torrent
@@ -55,7 +57,37 @@ TORRENT_TEST(seed_mode)
 		, [](lt::alert const*, lt::session&) {}
 		// terminate
 		, [](int, lt::session&) -> bool
-		{ return true; });
+		{ return false; });
+}
+
+TORRENT_TEST(seed_mode_disable_hash_checks)
+{
+	// all nodes need to disable hash checking, otherwise the downloader would
+	// just fail
+	settings_pack swarm_settings = settings();
+	swarm_settings.set_bool(settings_pack::disable_hash_checks, true);
+
+	dsl_config network_cfg;
+	sim::simulation sim{network_cfg};
+
+	// with seed mode
+	setup_swarm(2, swarm_test::upload, sim, swarm_settings, add_torrent_params()
+		// add session
+		, [](lt::settings_pack& pack) {
+			pack.set_int(settings_pack::suggest_mode, settings_pack::suggest_read_cache);
+		}
+		// add torrent
+		, [](lt::add_torrent_params& params) {
+			params.flags |= add_torrent_params::flag_seed_mode;
+			// just to make sure the disable_hash_checks really work, we
+			// shouldn't be verifying anything from the storage
+			params.storage = disabled_storage_constructor;
+		}
+		// on alert
+		, [](lt::alert const*, lt::session&) {}
+		// terminate
+		, [](int, lt::session&) -> bool
+		{ return false; });
 }
 
 TORRENT_TEST(plain)

src/peer_connection.cpp

@@ -5108,13 +5108,17 @@ namespace libtorrent
 #ifndef TORRENT_DISABLE_LOGGING
 				peer_log(peer_log_alert::outgoing_message, "REJECT_PIECE"
 					, "piece: %d s: %x l: %x torrent deleted"
-					, r.piece , r.start , r.length);
+					, r.piece, r.start , r.length);
 #endif
 				write_reject_request(r);
 				continue;
 			}
 
-			if (t->seed_mode() && !t->verified_piece(r.piece))
+			bool const seed_mode = t->seed_mode();
+
+			if (seed_mode
+				&& !t->verified_piece(r.piece)
+				&& !m_settings.get_bool(settings_pack::disable_hash_checks))
 			{
 				// we're still verifying the hash of this piece
 				// so we can't return it yet.
@@ -5140,10 +5144,7 @@ namespace libtorrent
 				continue;
 			}
 
-			// in seed mode, we might end up accepting a request
-			// which it later turns out we cannot serve, if we ended
-			// up not having that piece
-			if (!t->has_piece_passed(r.piece))
+			if (!t->has_piece_passed(r.piece) && !seed_mode)
 			{
 				// we don't have this piece yet, but we anticipate to have
 				// it very soon, so we have told our peers we have it.

src/torrent.cpp

@@ -1259,7 +1259,10 @@ namespace libtorrent
 		INVARIANT_CHECK;
 
 		// if we have all pieces we should not have a picker
-		TORRENT_ASSERT(!m_have_all);
+		// unless we're in suggest mode
+		TORRENT_ASSERT(!m_have_all
+			|| settings().get_int(settings_pack::suggest_mode)
+			== settings_pack::suggest_read_cache);
 
 		m_picker.reset(new piece_picker());
 		int const blocks_per_piece
@@ -6382,21 +6385,20 @@ namespace libtorrent
 			entry::list_type& up = ret["unfinished"].list();
 
 			// info for each unfinished piece
-			for (std::vector<piece_picker::downloading_piece>::const_iterator i
-				= q.begin(); i != q.end(); ++i)
+			for (piece_picker::downloading_piece const& dp : q)
 			{
-				if (i->finished == 0) continue;
+				if (dp.finished == 0) continue;
 
 				entry piece_struct(entry::dictionary_t);
 
 				// the unfinished piece's index
-				piece_struct["piece"] = i->index;
+				piece_struct["piece"] = dp.index;
 
 				std::string bitmask;
 				const int num_bitmask_bytes
 					= (std::max)(num_blocks_per_piece / 8, 1);
 
-				piece_picker::block_info const* info = m_picker->blocks_for_piece(*i);
+				piece_picker::block_info const* info = m_picker->blocks_for_piece(dp);
 				for (int j = 0; j < num_bitmask_bytes; ++j)
 				{
 					unsigned char v = 0;
@@ -6417,22 +6419,21 @@ namespace libtorrent
 		entry::list_type& tr_list = ret["trackers"].list();
 		tr_list.push_back(entry::list_type());
 		int tier = 0;
-		for (std::vector<announce_entry>::const_iterator i = m_trackers.begin()
-			, end(m_trackers.end()); i != end; ++i)
+		for (announce_entry const& tr : m_trackers)
 		{
 			// don't save trackers we can't trust
 			// TODO: 1 save the send_stats state instead of throwing them away
 			// it may pose an issue when downgrading though
-			if (i->send_stats == false) continue;
-			if (i->tier == tier)
+			if (tr.send_stats == false) continue;
+			if (tr.tier == tier)
 			{
-				tr_list.back().list().push_back(i->url);
+				tr_list.back().list().push_back(tr.url);
 			}
 			else
 			{
 				tr_list.push_back(entry::list_t);
-				tr_list.back().list().push_back(i->url);
-				tier = i->tier;
+				tr_list.back().list().push_back(tr.url);
+				tier = tr.tier;
 			}
 		}
 
@@ -6441,14 +6442,13 @@ namespace libtorrent
 		{
 			entry::list_type& url_list = ret["url-list"].list();
 			entry::list_type& httpseed_list = ret["httpseeds"].list();
-			for (std::list<web_seed_t>::const_iterator i = m_web_seeds.begin()
-				, end(m_web_seeds.end()); i != end; ++i)
+			for (web_seed_t const& ws : m_web_seeds)
 			{
-				if (i->removed) continue;
-				if (i->type == web_seed_entry::url_seed)
-					url_list.push_back(i->url);
-				else if (i->type == web_seed_entry::http_seed)
-					httpseed_list.push_back(i->url);
+				if (ws.removed) continue;
+				if (ws.type == web_seed_entry::url_seed)
+					url_list.push_back(ws.url);
+				else if (ws.type == web_seed_entry::http_seed)
+					httpseed_list.push_back(ws.url);
 			}
 		}
 
@@ -6474,7 +6474,7 @@ namespace libtorrent
 		{
 			entry::string_type& pieces = ret["pieces"].string();
 			pieces.resize(max_piece);
-			if (!has_picker())
+			if (is_seed())
 			{
 				std::memset(&pieces[0], m_have_all, pieces.size());
 			}