Merge pull request #170 from arvidn/active-time-fix

fix bug in time keeping of active_time, seeding_time, finished_time etc.

include/libtorrent/torrent.hpp

@@ -1643,7 +1643,7 @@ namespace libtorrent
 
 		// the timestamp of the last byte uploaded from this torrent specified in
 		// session_time. This is signed because it must be able to represent time
-		// before the session started
+		// before the session started.
 		boost::int16_t m_last_upload;
 
 		// this is a second count-down to when we should tick the

include/libtorrent/torrent_status.hpp

@@ -368,7 +368,11 @@ namespace libtorrent
 
 		// the number of seconds since any peer last uploaded from this torrent
 		// and the last time a downloaded piece passed the hash check,
-		// respectively.
+		// respectively. Note, when starting up a torrent that needs its files
+		// checked, piece may pass and that will be considered downloading for the
+		// purpose of this counter. -1 means there either hasn't been any
+		// uploading/downloading, or it was too long ago for libtorrent to
+		// remember (currently forgetting happens after about 18 hours)
 		int time_since_upload;
 		int time_since_download;
 

simulation/Jamfile

@@ -23,6 +23,7 @@ project
 	;
 
 alias libtorrent-sims :
+	[ run test_torrent_status.cpp ]
 	[ run test_swarm.cpp ]
 	[ run test_super_seeding.cpp ]
 	[ run test_utp.cpp ]

simulation/test_torrent_status.cpp

@@ -0,0 +1,111 @@
+/*
+
+Copyright (c) 2015, 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 "test.hpp"
+#include "swarm_config.hpp"
+#include "simulator/simulator.hpp"
+#include "libtorrent/alert_types.hpp"
+
+using namespace libtorrent;
+using namespace sim;
+namespace lt = libtorrent;
+
+struct test_swarm_config : swarm_config
+{
+	test_swarm_config()
+		: swarm_config()
+	{}
+
+	bool on_alert(libtorrent::alert const* alert
+		, int session_idx
+		, std::vector<libtorrent::torrent_handle> const& handles
+		, libtorrent::session& ses) override
+	{
+		if (torrent_added_alert const* ta = alert_cast<torrent_added_alert>(alert))
+		{
+			TEST_CHECK(!m_handle.is_valid());
+			m_start_time = lt::clock_type::now();
+			m_handle = ta->handle;
+		}
+		return false;
+	}
+
+	virtual void on_exit(std::vector<torrent_handle> const& torrents) override {}
+
+	virtual bool tick(int t) override
+	{
+		// once an hour, verify that the timers seem correct
+		if ((t % 3600) == 0)
+		{
+			lt::time_point now = lt::clock_type::now();
+			int since_start = total_seconds(now - m_start_time) - 1;
+			torrent_status st = m_handle.status();
+			TEST_EQUAL(st.active_time, since_start);
+			TEST_EQUAL(st.seeding_time, since_start);
+			TEST_EQUAL(st.finished_time, since_start);
+
+			TEST_EQUAL(st.last_scrape, -1);
+			TEST_EQUAL(st.time_since_upload, -1);
+
+			// checking the torrent counts as downloading
+			// eventually though, we've forgotten about it and go back to -1
+			if (since_start > 65000)
+			{
+				TEST_EQUAL(st.time_since_download, -1);
+			}
+			else
+			{
+				TEST_EQUAL(st.time_since_download, since_start);
+			}
+		}
+
+		// simulate 20 hours of uptime. Currently, the session_time and internal
+		// peer timestamps are 16 bits counting seconds, so they can only
+		// represent about 18 hours. The clock steps forward in 4 hour increments
+		// to stay within that range
+		return t > 20 * 60 * 60;
+	}
+
+private:
+	lt::time_point m_start_time;
+	lt::torrent_handle m_handle;
+};
+
+TORRENT_TEST(status_timers)
+{
+	sim::default_config network_cfg;
+	sim::simulation sim{network_cfg};
+
+	test_swarm_config cfg;
+	setup_swarm(1, sim, cfg);
+}
+

src/torrent.cpp

@@ -9254,6 +9254,13 @@ namespace libtorrent
 		return a - b;
 	}
 
+	int clamped_subtract_s16(int a, int b)
+	{
+		if (a + (std::numeric_limits<boost::int16_t>::min)() < b)
+			return (std::numeric_limits<boost::int16_t>::min)();
+		return a - b;
+	}
+
 	} // anonymous namespace
 
 	// this is called every time the session timer takes a step back. Since the
@@ -9278,28 +9285,40 @@ namespace libtorrent
 			}
 		}
 
-		if (m_started < seconds)
+		// m_active_time, m_seeding_time and m_finished_time are absolute cunters
+		// of the historical time we've spent in each state. The current time
+		// we've spent in those states (this session) is calculated by
+		// session_time() - m_started
+		// session_time() - m_became_seed
+		// session_time() - m_became_finished respectively. If any of the
+		// comparison points were pulled back to the oldest representable value (0)
+		// the left-over time must be transferred into the m_*_time counters.
+
+		if (m_started < seconds && !is_paused())
 		{
-			// the started time just got shifted out of the valid window of
-			// session time. Record this "lost time" by incrementing the
-			// counters that are supposed to keep track of the total time we've
-			// been in certain states
-			int lost_seconds = m_started - seconds;
-			if (!is_paused())
-				m_active_time += lost_seconds;
-
-			if (is_seed())
-				m_seeding_time += lost_seconds;
-
-			if (is_finished())
-				m_finished_time += lost_seconds;
+			int lost_seconds = seconds - m_started;
+			m_active_time += lost_seconds;
 		}
-
 		m_started = clamped_subtract(m_started, seconds);
 
-		m_last_upload = clamped_subtract(m_last_upload, seconds);
-		m_last_download = clamped_subtract(m_last_download, seconds);
-		m_last_scrape = clamped_subtract(m_last_scrape, seconds);
+		if (m_became_seed < seconds && is_seed())
+		{
+			int lost_seconds = seconds - m_became_seed;
+			m_seeding_time += lost_seconds;
+		}
+		m_became_seed = clamped_subtract(m_became_seed, seconds);
+
+		if (m_finished_time < seconds && is_finished())
+		{
+			int lost_seconds = seconds - m_became_finished;
+			m_finished_time += lost_seconds;
+		}
+		m_became_finished = clamped_subtract(m_became_finished, seconds);
+
+		m_last_upload = clamped_subtract_s16(m_last_upload, seconds);
+		m_last_download = clamped_subtract_s16(m_last_download, seconds);
+		m_last_scrape = clamped_subtract_s16(m_last_scrape, seconds);
+
 		m_last_saved_resume = clamped_subtract(m_last_saved_resume, seconds);
 		m_upload_mode_time = clamped_subtract(m_upload_mode_time, seconds);
 	}