From b4c160e723439c0f752525c5c7ef717f2a1afeea Mon Sep 17 00:00:00 2001 From: Arvid Norberg Date: Tue, 3 Jun 2008 15:17:09 +0000 Subject: [PATCH] fixed bug in web_peer_connection where an incorrect number of bytes would be reported as downloaded --- ChangeLog | 4 +++ include/libtorrent/web_peer_connection.hpp | 3 ++ src/http_parser.cpp | 14 +++++--- src/web_peer_connection.cpp | 34 +++++++++++++----- test/test_primitives.cpp | 30 +++++++++++----- test/test_web_seed.cpp | 41 ++++++++++++++++++---- 6 files changed, 99 insertions(+), 27 deletions(-) diff --git a/ChangeLog b/ChangeLog index c311f63f1..da772d4c6 100644 --- a/ChangeLog +++ b/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. diff --git a/include/libtorrent/web_peer_connection.hpp b/include/libtorrent/web_peer_connection.hpp index c37608232..f8e7af99f 100755 --- a/include/libtorrent/web_peer_connection.hpp +++ b/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; }; } diff --git a/src/http_parser.cpp b/src/http_parser.cpp index 4c2839178..eb717420f 100755 --- a/src/http_parser.cpp +++ b/src/http_parser.cpp @@ -63,9 +63,10 @@ namespace libtorrent { TORRENT_ASSERT(recv_buffer.left() >= m_recv_buffer.left()); boost::tuple 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) diff --git a/src/web_peer_connection.cpp b/src/web_peer_connection.cpp index a2dc059e2..50402cd34 100755 --- a/src/web_peer_connection.cpp +++ b/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 diff --git a/test/test_primitives.cpp b/test/test_primitives.cpp index e336fee15..391895a9e 100644 --- a/test/test_primitives.cpp +++ b/test/test_primitives.cpp @@ -25,16 +25,28 @@ using boost::bind; tuple feed_bytes(http_parser& parser, char const* str) { tuple ret(0, 0, false); - buffer::const_interval recv_buf(str, str + 1); - for (; *str; ++str) + tuple 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; } diff --git a/test/test_web_seed.cpp b/test/test_web_seed.cpp index 73192f428..2621be9b9 100644 --- a/test/test_web_seed.cpp +++ b/test/test_web_seed.cpp @@ -21,8 +21,12 @@ void test_transfer(boost::intrusive_ptr 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_file, int proxy) std::vector 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 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);