polish the stop-when-ready flag and improve documentation

include/libtorrent/torrent.hpp

@@ -464,7 +464,7 @@ namespace libtorrent
 		void set_announce_to_trackers(bool b) { m_announce_to_trackers = b; }
 		void set_announce_to_lsd(bool b) { m_announce_to_lsd = b; }
 
-		void stop_when_ready(bool b) { m_stop_when_ready = b; }
+		void stop_when_ready(bool b);
 
 		int started() const { return m_started; }
 		void step_session_time(int seconds);

include/libtorrent/torrent_handle.hpp

@@ -234,6 +234,13 @@ namespace libtorrent
 	// 
 	struct TORRENT_EXPORT torrent_handle
 	{
+		// TODO: 3 consider replacing all the setters and getters for pause,
+		// resume, stop-when-ready, share-mode, upload-mode, super-seeding,
+		// apply-ip-filter, resolve-countries, pinned, sequential-download,
+		// seed-mode
+		// with just set_flags() and clear_flags() using the flags from
+		// add_torrent_params. Perhaps those flags should have a more generic
+		// name.
 		friend class invariant_access;
 		friend struct aux::session_impl;
 		friend class session;
@@ -474,6 +481,9 @@ namespace libtorrent
 		void replace_trackers(std::vector<announce_entry> const&) const;
 		void add_tracker(announce_entry const&) const;
 
+		// TODO: 3 unify url_seed and http_seed with just web_seed, using the
+		// web_seed_entry.
+
 		// ``add_url_seed()`` adds another url to the torrent's list of url
 		// seeds. If the given url already exists in that list, the call has no
 		// effect. The torrent will connect to the server and try to download
@@ -565,6 +575,24 @@ namespace libtorrent
 		// 
 		// *Force stopped* means auto-managed is set to false and it's paused. As
 		// if auto_manage(false) and pause() were called on the torrent.
+		// 
+		// Note that the torrent may transition into a downloading state while
+		// calling this function, and since the logic is edge triggered you may
+		// miss the edge. To avoid this race, if the torrent already is in a
+		// downloading state when this call is made, it will trigger the
+		// stop-when-ready immediately.
+		// 
+		// When the stop-when-ready logic fires, the flag is cleared. Any
+		// subsequent transitions between downloading and non-downloading states
+		// will not be affected, until this function is used to set it again.
+		// 
+		// The behavior is more robust when setting this flag as part of adding
+		// the torrent. See add_torrent_params.
+		// 
+		// The stop-when-ready flag fixes the inherent race condition of waiting
+		// for the state_changed_alert and then call pause(). The download/seeding
+		// will most likely start in between posting the alert and receiving the
+		// call to pause.
 		void stop_when_ready(bool b) const;
 
 		// Explicitly sets the upload mode of the torrent. In upload mode, the

simulation/setup_swarm.cpp

@@ -327,7 +327,8 @@ void setup_swarm(int num_nodes
 					// only print alerts from the session under test
 					lt::time_duration d = a->timestamp() - start_time;
 					boost::uint32_t millis = lt::duration_cast<lt::milliseconds>(d).count();
-					printf("%4d.%03d: %s\n", millis / 1000, millis % 1000
+					printf("%4d.%03d: %-25s %s\n", millis / 1000, millis % 1000
+						, a->what()
 						, a->message().c_str());
 
 					// if a torrent was added save the torrent handle

simulation/test_auto_manage.cpp

@@ -656,6 +656,195 @@ TORRENT_TEST(stop_when_ready)
 			}
 		});
 }
+
+// This test makes sure that the fastresume check will still run, and
+// potentially fail, for stopped torrents. The actual checking of files won't
+// start until the torrent is un-paused/resumed though
+TORRENT_TEST(resume_reject_when_paused)
+{
+	run_test(
+		[](settings_pack& sett) {
+			sett.set_int(settings_pack::alert_mask, alert::all_categories);
+		},
+
+		[](lt::session& ses) {
+			// add torrents
+			lt::add_torrent_params params = create_torrent(0, true);
+
+			// the torrent is not auto managed and paused. Once the resume data
+			// check completes, it will stay paused but the resume_rejected_alert
+			// will be posted
+			params.flags &= ~add_torrent_params::flag_auto_managed;
+			params.flags |= add_torrent_params::flag_paused;
+
+			// an empty dictionary will be rejected
+			params.resume_data = {'d', 'e'};
+			ses.async_add_torrent(params);
+		},
+
+		[](lt::session& ses) {
+			std::vector<lt::alert*> alerts;
+			ses.pop_alerts(&alerts);
+
+			lt::time_point start_time = alerts[0]->timestamp();
+
+			int num_piece_finished = 0;
+			int resume_rejected = 0;
+			int state_changed = 0;
+
+			for (alert* a : alerts)
+			{
+				fprintf(stderr, "%-3d %-25s %s\n", int(duration_cast<lt::seconds>(a->timestamp()
+						- start_time).count())
+					, a->what()
+					, a->message().c_str());
+
+				if (alert_cast<piece_finished_alert>(a))
+					++num_piece_finished;
+
+				if (alert_cast<fastresume_rejected_alert>(a))
+					++resume_rejected;
+
+				if (auto sc = alert_cast<state_changed_alert>(a))
+				{
+					if (sc->state == torrent_status::checking_files)
+						++state_changed;
+				}
+			}
+
+			for (torrent_handle const& h : ses.get_torrents())
+			{
+				// the torrent should have been force-stopped. Force stopped means
+				// not auto-managed and paused.
+				torrent_status st = h.status();
+				TEST_CHECK(!st.auto_managed);
+				TEST_EQUAL(st.paused, true);
+				// it should be checking files, because the resume data should have
+				// failed validation.
+				TEST_EQUAL(st.state, torrent_status::checking_files);
+			}
+
+			TEST_EQUAL(num_piece_finished, 0);
+			TEST_EQUAL(resume_rejected, 1);
+			TEST_EQUAL(state_changed, 1);
+		});
+}
+
+// this test adds the torrent in paused state and no resume data. Expecting the
+// resume check to complete and just transition into checking state, but without
+// actually checking anything
+TORRENT_TEST(no_resume_when_paused)
+{
+	run_test(
+		[](settings_pack& sett) {
+			sett.set_int(settings_pack::alert_mask, alert::all_categories);
+		},
+
+		[](lt::session& ses) {
+			// add torrents
+			lt::add_torrent_params params = create_torrent(0, true);
+
+			// the torrent is not auto managed and paused.
+			params.flags &= ~add_torrent_params::flag_auto_managed;
+			params.flags |= add_torrent_params::flag_paused;
+
+			ses.async_add_torrent(params);
+		},
+
+		[](lt::session& ses) {
+			std::vector<lt::alert*> alerts;
+			ses.pop_alerts(&alerts);
+
+			lt::time_point start_time = alerts[0]->timestamp();
+
+			int num_piece_finished = 0;
+			int resume_rejected = 0;
+			int state_changed = 0;
+
+			for (alert* a : alerts)
+			{
+				fprintf(stderr, "%-3d %-25s %s\n", int(duration_cast<lt::seconds>(a->timestamp()
+						- start_time).count())
+					, a->what()
+					, a->message().c_str());
+
+				if (alert_cast<piece_finished_alert>(a))
+					++num_piece_finished;
+
+				if (alert_cast<fastresume_rejected_alert>(a))
+					++resume_rejected;
+
+				if (auto sc = alert_cast<state_changed_alert>(a))
+				{
+					if (sc->state == torrent_status::checking_files)
+						++state_changed;
+				}
+			}
+
+			for (torrent_handle const& h : ses.get_torrents())
+			{
+				// the torrent should have been force-stopped. Force stopped means
+				// not auto-managed and paused.
+				torrent_status st = h.status();
+				TEST_CHECK(!st.auto_managed);
+				TEST_EQUAL(st.paused, true);
+				// it should be checking files, because the resume data should have
+				// failed validation.
+				TEST_EQUAL(st.state, torrent_status::checking_files);
+			}
+
+			TEST_EQUAL(num_piece_finished, 0);
+			TEST_EQUAL(resume_rejected, 0);
+			TEST_EQUAL(state_changed, 1);
+		});
+}
+
+// this is just asserting that when the files are checked we do in fact get
+// piece_finished_alerts. The other tests rely on this assumption
+TORRENT_TEST(no_resume_when_started)
+{
+	run_test(
+		[](settings_pack& sett) {
+			sett.set_int(settings_pack::alert_mask, alert::all_categories);
+		},
+
+		[](lt::session& ses) {
+			// add torrents
+			lt::add_torrent_params params = create_torrent(0, true);
+			ses.async_add_torrent(params);
+		},
+
+		[](lt::session& ses) {
+			std::vector<lt::alert*> alerts;
+			ses.pop_alerts(&alerts);
+
+			lt::time_point start_time = alerts[0]->timestamp();
+
+			int num_piece_finished = 0;
+			int state_changed = 0;
+
+			for (alert* a : alerts)
+			{
+				fprintf(stderr, "%-3d %-25s %s\n", int(duration_cast<lt::seconds>(a->timestamp()
+						- start_time).count())
+					, a->what()
+					, a->message().c_str());
+
+				if (alert_cast<piece_finished_alert>(a))
+					++num_piece_finished;
+
+				if (auto sc = alert_cast<state_changed_alert>(a))
+				{
+					if (sc->state == torrent_status::checking_files)
+						++state_changed;
+				}
+			}
+
+			TEST_EQUAL(num_piece_finished, 9);
+			TEST_EQUAL(state_changed, 1);
+		});
+}
+
 // TODO: assert that the torrent_paused_alert is posted when pausing
 //       downloading, seeding, checking torrents as well as the graceful pause
 // TODO: test limits of tracker, DHT and LSD announces

src/torrent.cpp

@@ -11695,6 +11695,24 @@ namespace libtorrent
 		}
 	}
 
+	void torrent::stop_when_ready(bool b)
+	{
+		m_stop_when_ready = b;
+
+		// to avoid race condition, if we're already in a downloading state,
+		// trigger the stop-when-ready logic immediately.
+		if (m_stop_when_ready
+			&& is_downloading_state(m_state))
+		{
+#ifndef TORRENT_DISABLE_LOGGING
+			debug_log("stop_when_ready triggered");
+#endif
+			auto_managed(false);
+			pause();
+			m_stop_when_ready = false;
+		}
+	}
+
 	void torrent::set_state(torrent_status::state_t s)
 	{
 		TORRENT_ASSERT(is_single_thread());
@@ -11741,6 +11759,7 @@ namespace libtorrent
 			// either upload or download (for the purpose of this flag).
 			auto_managed(false);
 			pause();
+			m_stop_when_ready = false;
 		}
 
 		m_state = s;