fixed bug in web_peer_connection where an incorrect number of bytes would be reported as downloaded

ChangeLog

@@ -1,3 +1,7 @@
+	* Fixed bug in statistics from web server peers where it sometimes
+	  could report too many bytes downloaded.
+	* Fixed bug where statistics from the last second was lost when
+	  disconnecting a peer.
 	* receive buffer optimizations (memcpy savings and memory savings)
 	* Support for specifying the TOS byte for peer traffic.
 	* Basic support for queueing of torrents.

include/libtorrent/web_peer_connection.hpp

@@ -173,6 +173,9 @@ namespace libtorrent
 		// response. used to know where in the buffer the
 		// next response starts
 		int m_received_body;
+
+		// position in the current range response
+		int m_range_pos;
 	};
 }
 

src/http_parser.cpp

@@ -63,9 +63,10 @@ namespace libtorrent
 	{
 		TORRENT_ASSERT(recv_buffer.left() >= m_recv_buffer.left());
 		boost::tuple<int, int> ret(0, 0);
+		int start_pos = m_recv_buffer.left();
 
 		// early exit if there's nothing new in the receive buffer
-		if (recv_buffer.left() == m_recv_buffer.left()) return ret;
+		if (start_pos == recv_buffer.left()) return ret;
 		m_recv_buffer = recv_buffer;
 
 		if (m_state == error_state)
@@ -80,7 +81,11 @@ namespace libtorrent
 			TORRENT_ASSERT(!m_finished);
 			char const* newline = std::find(pos, recv_buffer.end, '\n');
 			// if we don't have a full line yet, wait.
-			if (newline == recv_buffer.end) return ret;
+			if (newline == recv_buffer.end)
+			{
+				boost::get<1>(ret) += m_recv_buffer.left() - start_pos;
+				return ret;
+			}
 
 			if (newline == pos)
 			{
@@ -96,7 +101,7 @@ namespace libtorrent
 			++newline;
 			int incoming = (int)std::distance(pos, newline);
 			m_recv_pos += incoming;
-			boost::get<1>(ret) += incoming;
+			boost::get<1>(ret) += newline - (m_recv_buffer.begin + start_pos);
 			pos = newline;
 
 			line >> m_protocol;
@@ -114,6 +119,7 @@ namespace libtorrent
 				m_status_code = 0;
 			}
 			m_state = read_header;
+			start_pos = pos - recv_buffer.begin;
 		}
 
 		if (m_state == read_header)
@@ -131,7 +137,6 @@ namespace libtorrent
 				line.assign(pos, line_end);
 				++newline;
 				m_recv_pos += newline - pos;
-				boost::get<1>(ret) += newline - pos;
 				pos = newline;
 
 				std::string::size_type separator = line.find(':');
@@ -187,6 +192,7 @@ namespace libtorrent
 				TORRENT_ASSERT(m_recv_pos <= (int)recv_buffer.left());
 				newline = std::find(pos, recv_buffer.end, '\n');
 			}
+			boost::get<1>(ret) += newline - (m_recv_buffer.begin + start_pos);
 		}
 
 		if (m_state == read_body)

src/web_peer_connection.cpp

@@ -67,6 +67,7 @@ namespace libtorrent
 		: peer_connection(ses, t, s, remote, peerinfo)
 		, m_url(url)
 		, m_first_request(true)
+		, m_range_pos(0)
 	{
 		INVARIANT_CHECK;
 
@@ -350,7 +351,8 @@ namespace libtorrent
 			{
 				bool error = false;
 				boost::tie(payload, protocol) = m_parser.incoming(recv_buffer, error);
-				m_statistics.received_bytes(payload, protocol);
+				m_statistics.received_bytes(0, protocol);
+				bytes_transferred -= protocol;
 
 				if (error)
 				{
@@ -363,7 +365,12 @@ namespace libtorrent
 				TORRENT_ASSERT(recv_buffer.left() <= packet_size());
 				
 				// this means the entire status line hasn't been received yet
-				if (m_parser.status_code() == -1) break;
+				if (m_parser.status_code() == -1)
+				{
+					TORRENT_ASSERT(payload == 0);
+					TORRENT_ASSERT(bytes_transferred == 0);
+					break;
+				}
 
 				// if the status code is not one of the accepted ones, abort
 				if (m_parser.status_code() != 206 // partial content
@@ -388,15 +395,16 @@ namespace libtorrent
 					disconnect(error_msg.c_str(), 1);
 					return;
 				}
-				if (!m_parser.header_finished()) break;
+				if (!m_parser.header_finished())
+				{
+					TORRENT_ASSERT(payload == 0);
+					TORRENT_ASSERT(bytes_transferred == 0);
+					break;
+				}
 
 				m_body_start = m_parser.body_start();
 				m_received_body = 0;
 			}
-			else
-			{
-				m_statistics.received_bytes(bytes_transferred, 0);
-			}
 
 			// we just completed reading the header
 			if (!header_finished)
@@ -460,9 +468,11 @@ namespace libtorrent
 
 				m_body_start = m_parser.body_start();
 				m_received_body = 0;
+				m_range_pos = 0;
 			}
 
 			recv_buffer.begin += m_body_start;
+
 			// we only received the header, no data
 			if (recv_buffer.left() == 0) break;
 
@@ -499,6 +509,13 @@ namespace libtorrent
 				}
 			}
 
+			int left_in_response = range_end - range_start - m_range_pos;
+			int payload_transferred = (std::min)(left_in_response, int(bytes_transferred));
+			m_statistics.received_bytes(payload_transferred, 0);
+			bytes_transferred -= payload_transferred;
+			m_range_pos += payload_transferred;;
+			if (m_range_pos > range_end - range_start) m_range_pos = range_end - range_start;
+
 //			std::cerr << "REQUESTS: m_requests: " << m_requests.size()
 //				<< " file_requests: " << m_file_requests.size() << std::endl;
 
@@ -641,8 +658,9 @@ namespace libtorrent
 				m_received_body = 0;
 				continue;
 			}
-			break;
+			if (bytes_transferred == 0) break;
 		}
+		TORRENT_ASSERT(bytes_transferred == 0);
 	}
 
 	void web_peer_connection::get_specific_peer_info(peer_info& p) const

test/test_primitives.cpp

@@ -25,16 +25,28 @@ using boost::bind;
 tuple<int, int, bool> feed_bytes(http_parser& parser, char const* str)
 {
 	tuple<int, int, bool> ret(0, 0, false);
-	buffer::const_interval recv_buf(str, str + 1);
-	for (; *str; ++str)
+	tuple<int, int, bool> prev(0, 0, false);
+	for (int chunks = 1; chunks < 70; ++chunks)
 	{
-		recv_buf.end = str + 1;
-		int payload, protocol;
-		bool error = false;
-		tie(payload, protocol) = parser.incoming(recv_buf, error);
-		ret.get<0>() += payload;
-		ret.get<1>() += protocol;
-		ret.get<2>() += error;
+		ret = make_tuple(0, 0, false);
+		parser.reset();
+		buffer::const_interval recv_buf(str, str);
+		for (; *str;)
+		{
+			int chunk_size = (std::min)(chunks, int(strlen(recv_buf.end)));
+			if (chunk_size == 0) break;
+			recv_buf.end += chunk_size;
+			int payload, protocol;
+			bool error = false;
+			tie(payload, protocol) = parser.incoming(recv_buf, error);
+			ret.get<0>() += payload;
+			ret.get<1>() += protocol;
+			ret.get<2>() += error;
+			std::cerr << payload << ", " << protocol << ", " << chunk_size << std::endl;
+			TORRENT_ASSERT(payload + protocol == chunk_size);
+		}
+		TEST_CHECK(prev == make_tuple(0, 0, false) || ret == prev);
+		prev = ret;
 	}
 	return ret;
 }

test/test_web_seed.cpp

@@ -21,8 +21,12 @@ void test_transfer(boost::intrusive_ptr<torrent_info> torrent_file, int proxy)
 	using namespace libtorrent;
 
 	session ses;
+	session_settings settings;
+	settings.ignore_limits_on_local_network = false;
+	ses.set_settings(settings);
 	ses.set_severity_level(alert::debug);
 	ses.listen_on(std::make_pair(51000, 52000));
+	ses.set_download_rate_limit(torrent_file->total_size() / 10);
 	remove_all("./tmp1");
 
 	char const* test_name[] = {"no", "SOCKS4", "SOCKS5", "SOCKS5 password", "HTTP", "HTTP password"};
@@ -46,19 +50,44 @@ void test_transfer(boost::intrusive_ptr<torrent_info> torrent_file, int proxy)
 	std::vector<announce_entry> empty;
 	th.replace_trackers(empty);
 
+	const size_type total_size = torrent_file->total_size();
+
+	float rate_sum = 0.f;
+	float ses_rate_sum = 0.f;
+
 	for (int i = 0; i < 30; ++i)
 	{
 		torrent_status s = th.status();
-		std::cerr << s.progress << " " << (s.download_rate / 1000.f) << std::endl;
-		std::auto_ptr<alert> a;
-		a = ses.pop_alert();
-		if (a.get())
-			std::cerr << a->msg() << "\n";
+		session_status ss = ses.status();
+		std::cerr << (s.progress * 100.f) << " %"
+			<< " torrent rate: " << (s.download_rate / 1000.f) << " kB/s"
+			<< " session rate: " << (ss.download_rate / 1000.f) << " kB/s"
+			<< " session total: " << ss.total_payload_download
+			<< " torrent total: " << s.total_payload_download
+			<< std::endl;
+		rate_sum += s.download_payload_rate;
+		ses_rate_sum += ss.payload_download_rate;
 
-		if (th.is_seed()) break;
+		print_alerts(ses, "ses");
+
+		if (th.is_seed() && ss.download_rate == 0.f)
+		{
+			TEST_CHECK(ses.status().total_payload_download == total_size);
+			TEST_CHECK(th.status().total_payload_download == total_size);
+			break;
+		}
 		test_sleep(1000);
 	}
 
+	std::cerr << "total_size: " << total_size
+		<< " rate_sum: " << rate_sum
+		<< " session_rate_sum: " << ses_rate_sum
+		<< std::endl;
+
+	// the rates for each second should sum up to the total, with a 10% error margin
+	TEST_CHECK(fabs(rate_sum - total_size) < total_size * .1f);
+	TEST_CHECK(fabs(ses_rate_sum - total_size) < total_size * .1f);
+
 	TEST_CHECK(th.is_seed());
 
 	if (proxy) stop_proxy(8002);