From 1afc0c67406509a2a7071333a18cb8977d36fc80 Mon Sep 17 00:00:00 2001 From: Arvid Norberg Date: Mon, 19 Aug 2013 03:54:45 +0000 Subject: [PATCH] a bunch of fixes to make test_web_seeds a lot faster, and fail slightly fewer tests --- include/libtorrent/connection_queue.hpp | 2 +- include/libtorrent/torrent.hpp | 4 + include/libtorrent/udp_socket.hpp | 2 + src/connection_queue.cpp | 6 +- src/file_pool.cpp | 4 +- src/peer_connection.cpp | 22 +++-- src/piece_picker.cpp | 9 +- src/torrent.cpp | 44 ++++++---- src/udp_socket.cpp | 24 +++-- test/main.cpp | 9 +- test/setup_transfer.cpp | 36 +++++--- test/test_web_seed.cpp | 112 ++++++++++++++---------- 12 files changed, 172 insertions(+), 102 deletions(-) diff --git a/include/libtorrent/connection_queue.hpp b/include/libtorrent/connection_queue.hpp index 982ab8bbd..084010bbb 100644 --- a/include/libtorrent/connection_queue.hpp +++ b/include/libtorrent/connection_queue.hpp @@ -62,7 +62,7 @@ public: void enqueue(boost::function const& on_connect , boost::function const& on_timeout , time_duration timeout, int priority = 0); - void done(int ticket); + bool done(int ticket); void limit(int limit); int limit() const; void close(); diff --git a/include/libtorrent/torrent.hpp b/include/libtorrent/torrent.hpp index 2df71a252..85aef16b8 100644 --- a/include/libtorrent/torrent.hpp +++ b/include/libtorrent/torrent.hpp @@ -232,6 +232,10 @@ namespace libtorrent void second_tick(stat& accumulator, int tick_interval_ms); + // see if we need to connect to web seeds, and if so, + // connect to them + void maybe_connect_web_seeds(); + std::string name() const; stat statistics() const { return m_stat; } diff --git a/include/libtorrent/udp_socket.hpp b/include/libtorrent/udp_socket.hpp index d25d46ca0..036bea804 100644 --- a/include/libtorrent/udp_socket.hpp +++ b/include/libtorrent/udp_socket.hpp @@ -274,6 +274,8 @@ namespace libtorrent int m_outstanding_resolve; int m_outstanding_connect_queue; int m_outstanding_socks; + + char timeout_stack[2000]; #endif }; diff --git a/src/connection_queue.cpp b/src/connection_queue.cpp index 96aceffab..18fe9cd6d 100644 --- a/src/connection_queue.cpp +++ b/src/connection_queue.cpp @@ -104,7 +104,7 @@ namespace libtorrent &connection_queue::on_try_connect, this)); } - void connection_queue::done(int ticket) + bool connection_queue::done(int ticket) { mutex_t::scoped_lock l(m_mutex); @@ -115,7 +115,8 @@ namespace libtorrent if (i == m_queue.end()) { // this might not be here in case on_timeout calls remove - return; + TORRENT_ASSERT(false); + return false; } if (i->connecting) --m_num_connecting; m_queue.erase(i); @@ -124,6 +125,7 @@ namespace libtorrent || m_half_open_limit == 0) m_timer.get_io_service().post(boost::bind( &connection_queue::on_try_connect, this)); + return true; } void connection_queue::close() diff --git a/src/file_pool.cpp b/src/file_pool.cpp index 91675dcd0..f94ce2981 100644 --- a/src/file_pool.cpp +++ b/src/file_pool.cpp @@ -83,10 +83,10 @@ namespace libtorrent if (i == m_queued_for_close.end()) { l.unlock(); - // none of the files are ready to be closet yet + // none of the files are ready to be closed yet // because they're still in use by other threads // hold off for a while - sleep(1000); + sleep(100); } else { diff --git a/src/peer_connection.cpp b/src/peer_connection.cpp index ac17996db..17d0b5680 100644 --- a/src/peer_connection.cpp +++ b/src/peer_connection.cpp @@ -1727,8 +1727,6 @@ namespace libtorrent peer_log("<== DONT_HAVE [ piece: %d ]", index); #endif - if (is_disconnecting()) return; - // if we got an invalid message, abort if (index >= int(m_have_piece.size()) || index < 0) { @@ -3419,7 +3417,8 @@ namespace libtorrent if (m_connection_ticket != -1) { - m_ses.m_half_open.done(m_connection_ticket); + if (m_ses.m_half_open.done(m_connection_ticket)) + m_connection_ticket = -1; } // a connection attempt using uTP just failed @@ -3585,8 +3584,8 @@ namespace libtorrent } if (m_connection_ticket >= 0) { - m_ses.m_half_open.done(m_connection_ticket); - m_connection_ticket = -1; + if (m_ses.m_half_open.done(m_connection_ticket)) + m_connection_ticket = -1; } torrent_handle handle; @@ -4109,8 +4108,11 @@ namespace libtorrent if (!t || m_disconnecting) { - m_ses.m_half_open.done(m_connection_ticket); - if (m_connection_ticket >= -1) m_connection_ticket = -1; + if (m_connection_ticket != -1) + { + if (m_ses.m_half_open.done(m_connection_ticket)) + m_connection_ticket = -1; + } TORRENT_ASSERT(t || !m_connecting); if (m_connecting && t) { @@ -5554,7 +5556,11 @@ namespace libtorrent t->dec_num_connecting(); m_connecting = false; } - m_ses.m_half_open.done(m_connection_ticket); + if (m_connection_ticket != -1) + { + if (m_ses.m_half_open.done(m_connection_ticket)) + m_connection_ticket = -1; + } if (m_disconnecting) return; m_last_receive = time_now(); diff --git a/src/piece_picker.cpp b/src/piece_picker.cpp index 2dd765585..d0e13ad82 100644 --- a/src/piece_picker.cpp +++ b/src/piece_picker.cpp @@ -2462,7 +2462,14 @@ namespace libtorrent if (info.state == block_info::state_finished) return; TORRENT_ASSERT(info.num_peers == 0); - info.peer = peer; + + // peers may have been disconnected in between mark_as_writing + // and mark_as_finished. When a peer disconnects, its m_peer_info + // pointer is set to NULL. If so, preserve the previous peer + // pointer, instead of forgetting who we downloaded this block from + if (info.state != block_info::state_writing || peer != 0) + info.peer = peer; + TORRENT_ASSERT(info.state == block_info::state_writing || peer == 0); TORRENT_ASSERT(i->writing >= 0); diff --git a/src/torrent.cpp b/src/torrent.cpp index cce09648f..bcafcf3a5 100644 --- a/src/torrent.cpp +++ b/src/torrent.cpp @@ -6518,6 +6518,8 @@ namespace libtorrent m_files_checked = true; start_announcing(); + + maybe_connect_web_seeds(); } alert_manager& torrent::alerts() const @@ -7794,24 +7796,7 @@ namespace libtorrent // ---- WEB SEEDS ---- - // if we have everything we want we don't need to connect to any web-seed - if (!is_finished() && !m_web_seeds.empty() && m_files_checked - && int(m_connections.size()) < m_max_connections - && m_ses.num_connections() < m_ses.settings().connections_limit) - { - // keep trying web-seeds if there are any - // first find out which web seeds we are connected to - for (std::list::iterator i = m_web_seeds.begin(); - i != m_web_seeds.end();) - { - std::list::iterator w = i++; - if (w->peer_info.connection) continue; - if (w->retry > time_now()) continue; - if (w->resolving) continue; - - connect_to_url_seed(w); - } - } + maybe_connect_web_seeds(); m_swarm_last_seen_complete = m_last_seen_complete; for (peer_iterator i = m_connections.begin(); @@ -7854,6 +7839,28 @@ namespace libtorrent state_updated(); } + void torrent::maybe_connect_web_seeds() + { + // if we have everything we want we don't need to connect to any web-seed + if (!is_finished() && !m_web_seeds.empty() && m_files_checked + && int(m_connections.size()) < m_max_connections + && m_ses.num_connections() < m_ses.settings().connections_limit) + { + // keep trying web-seeds if there are any + // first find out which web seeds we are connected to + for (std::list::iterator i = m_web_seeds.begin(); + i != m_web_seeds.end();) + { + std::list::iterator w = i++; + if (w->peer_info.connection) continue; + if (w->retry > time_now()) continue; + if (w->resolving) continue; + + connect_to_url_seed(w); + } + } + } + void torrent::recalc_share_mode() { TORRENT_ASSERT(share_mode()); @@ -8329,6 +8336,7 @@ namespace libtorrent for (std::list::const_iterator i = m_web_seeds.begin() , end(m_web_seeds.end()); i != end; ++i) { + if (i->peer_info.banned) continue; if (i->type != type) continue; ret.insert(i->url); } diff --git a/src/udp_socket.cpp b/src/udp_socket.cpp index 360f431f5..582548d07 100644 --- a/src/udp_socket.cpp +++ b/src/udp_socket.cpp @@ -615,8 +615,8 @@ void udp_socket::close() if (m_connection_ticket >= 0) { - m_cc.done(m_connection_ticket); - m_connection_ticket = -1; + if (m_cc.done(m_connection_ticket)) + m_connection_ticket = -1; // we just called done, which means on_timeout // won't be called. Decrement the outstanding @@ -624,6 +624,8 @@ void udp_socket::close() #if defined TORRENT_DEBUG || TORRENT_RELEASE_ASSERTS TORRENT_ASSERT(m_outstanding_timeout > 0); --m_outstanding_timeout; + + print_backtrace(timeout_stack, sizeof(timeout_stack)); #endif TORRENT_ASSERT(m_outstanding_ops > 0); --m_outstanding_ops; @@ -815,6 +817,7 @@ void udp_socket::on_timeout() #if defined TORRENT_DEBUG || TORRENT_RELEASE_ASSERTS TORRENT_ASSERT(m_outstanding_timeout > 0); --m_outstanding_timeout; + print_backtrace(timeout_stack, sizeof(timeout_stack)); #endif TORRENT_ASSERT(m_outstanding_ops > 0); --m_outstanding_ops; @@ -832,6 +835,7 @@ void udp_socket::on_timeout() error_code ec; m_socks5_sock.close(ec); + TORRENT_ASSERT(m_cc.done(m_connection_ticket) == false); m_connection_ticket = -1; } @@ -857,6 +861,7 @@ void udp_socket::on_connect(int ticket) #if defined TORRENT_DEBUG || TORRENT_RELEASE_ASSERTS TORRENT_ASSERT(m_outstanding_timeout > 0); --m_outstanding_timeout; + print_backtrace(timeout_stack, sizeof(timeout_stack)); #endif TORRENT_ASSERT(m_outstanding_ops > 0); --m_outstanding_ops; @@ -904,14 +909,18 @@ void udp_socket::on_connected(error_code const& e) + m_outstanding_resolve + m_outstanding_connect_queue + m_outstanding_socks); - if (m_abort) return; CHECK_MAGIC; - if (e == asio::error::operation_aborted) return; - TORRENT_ASSERT(is_single_thread()); - m_cc.done(m_connection_ticket); - m_connection_ticket = -1; + if (m_connection_ticket >= 0) + { + if (m_cc.done(m_connection_ticket)) + m_connection_ticket = -1; + } + + if (m_abort) return; + + if (e == asio::error::operation_aborted) return; // we just called done, which means on_timeout // won't be called. Decrement the outstanding @@ -919,6 +928,7 @@ void udp_socket::on_connected(error_code const& e) #if defined TORRENT_DEBUG || TORRENT_RELEASE_ASSERTS TORRENT_ASSERT(m_outstanding_timeout > 0); --m_outstanding_timeout; + print_backtrace(timeout_stack, sizeof(timeout_stack)); #endif TORRENT_ASSERT(m_outstanding_ops > 0); --m_outstanding_ops; diff --git a/test/main.cpp b/test/main.cpp index f89226c21..40fb89dca 100644 --- a/test/main.cpp +++ b/test/main.cpp @@ -158,9 +158,12 @@ int main() fflush(stdout); fflush(stderr); - remove_all(test_dir, ec); - if (ec) - fprintf(stderr, "failed to remove test dir: %s\n", ec.message().c_str()); + if (!tests_failure) + { + remove_all(test_dir, ec); + if (ec) + fprintf(stderr, "failed to remove test dir: %s\n", ec.message().c_str()); + } return tests_failure ? 1 : 0; } diff --git a/test/setup_transfer.cpp b/test/setup_transfer.cpp index c31432cb6..6c6d87758 100644 --- a/test/setup_transfer.cpp +++ b/test/setup_transfer.cpp @@ -263,26 +263,30 @@ void test_sleep(int millisec) libtorrent::sleep(millisec); } -static std::set running_proxies; +// maps port to proxy type +static std::map running_proxies; void stop_proxy(int port) { - char buf[100]; - snprintf(buf, sizeof(buf), "delegated -P%d -Fkill", port); - int ret = system(buf); - if (ret == 0) - perror("system"); - else - running_proxies.erase(port); + // don't shut down proxies until the test is + // completely done. This saves a lot of time. + // they're closed at the end of main() by + // calling stop_all_proxies(). } void stop_all_proxies() { - std::set proxies = running_proxies; - for (std::set::iterator i = proxies.begin() + std::map proxies = running_proxies; + for (std::map::iterator i = proxies.begin() , end(proxies.end()); i != end; ++i) { - stop_proxy(*i); + char buf[100]; + snprintf(buf, sizeof(buf), "delegated -P%d -Fkill", i->first); + int ret = system(buf); + if (ret == 0) + perror("system"); + else + running_proxies.erase(i->first); } } @@ -290,9 +294,13 @@ int start_proxy(int proxy_type) { using namespace libtorrent; - int port = 10000 + (rand() % 50000); + for (std::map::iterator i = running_proxies.begin() + , end(running_proxies.end()); i != end; ++i) + { + if (i->second == proxy_type) return i->first; + } - stop_proxy(port); + int port = 10000 + (rand() % 50000); char const* type = ""; char const* auth = ""; @@ -332,7 +340,7 @@ int start_proxy(int proxy_type) fprintf(stderr, "failed (%d) %s\n", errno, strerror(errno)); exit(1); } - running_proxies.insert(port); + running_proxies.insert(std::make_pair(port, proxy_type)); fprintf(stderr, "%s launched\n", time_now_string()); // apparently delegate takes a while to open its listen port test_sleep(500); diff --git a/test/test_web_seed.cpp b/test/test_web_seed.cpp index f19b6388e..b607f0934 100644 --- a/test/test_web_seed.cpp +++ b/test/test_web_seed.cpp @@ -37,6 +37,7 @@ POSSIBILITY OF SUCH DAMAGE. #include "libtorrent/bencode.hpp" #include "libtorrent/create_torrent.hpp" #include "libtorrent/thread.hpp" +#include "libtorrent/alert_types.hpp" #include #include #include @@ -46,31 +47,36 @@ POSSIBILITY OF SUCH DAMAGE. using namespace libtorrent; +int peer_disconnects = 0; + +bool on_alert(alert* a) +{ + if (alert_cast(a)) + ++peer_disconnects; + else if (alert_cast(a)) + ++peer_disconnects; + + return false; +} + // proxy: 0=none, 1=socks4, 2=socks5, 3=socks5_pw 4=http 5=http_pw -void test_transfer(boost::intrusive_ptr torrent_file +void test_transfer(session& ses, boost::intrusive_ptr torrent_file , int proxy, int port, char const* protocol, bool url_seed, bool chunked_encoding, bool test_ban) { using namespace libtorrent; - session ses(fingerprint(" ", 0,0,0,0), 0); - session_settings settings; - settings.max_queued_disk_bytes = 256 * 1024; - ses.set_settings(settings); - ses.set_alert_mask(~(alert::progress_notification | alert::stats_notification)); error_code ec; - ses.listen_on(std::make_pair(51000, 52000), ec); - if (ec) fprintf(stderr, "listen_on failed: %s\n", ec.message().c_str()); - remove_all("tmp2_web_seed", ec); char const* test_name[] = {"no", "SOCKS4", "SOCKS5", "SOCKS5 password", "HTTP", "HTTP password"}; fprintf(stderr, "\n\n ==== TESTING === proxy: %s ==== protocol: %s ==== seed: %s === transfer-encoding: %s === corruption: %s\n\n\n" , test_name[proxy], protocol, url_seed ? "URL seed" : "HTTP seed", chunked_encoding ? "chunked": "none", test_ban ? "yes" : "no"); + + proxy_settings ps; if (proxy) { - proxy_settings ps; ps.port = start_proxy(proxy); ps.hostname = "127.0.0.1"; ps.username = "testuser"; @@ -107,7 +113,9 @@ void test_transfer(boost::intrusive_ptr torrent_file if (f.pad_file) pad_file_size += f.size; } - for (int i = 0; i < 30; ++i) + peer_disconnects = 0; + + for (int i = 0; i < 40; ++i) { torrent_status s = th.status(); session_status ss = ses.status(); @@ -117,41 +125,35 @@ void test_transfer(boost::intrusive_ptr torrent_file cs = ses.get_cache_status(); if (cs.blocks_read < 1) cs.blocks_read = 1; if (cs.blocks_written < 1) cs.blocks_written = 1; -/* - 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 - << " rate sum:" << ses_rate_sum - << " cache: " << cs.cache_size - << " rcache: " << cs.read_cache_size - << " buffers: " << cs.total_used_buffers - << std::endl; -*/ - print_alerts(ses, " >> ses", test_ban, false, false, 0, true); - if (test_ban && th.url_seeds().empty()) + print_ses_rate(i / 10.f, &s, NULL); + + print_alerts(ses, " >> ses", test_ban, false, false, &on_alert); + + if (test_ban && th.url_seeds().empty() && th.http_seeds().empty()) { // when we don't have any web seeds left, we know we successfully banned it break; } +// if (test_ban && peer_disconnects >= 1) break; + if (s.is_seeding /* && ss.download_rate == 0.f*/) { TEST_EQUAL(s.total_payload_download - s.total_redundant_bytes, total_size - pad_file_size); // we need to sleep here a bit to let the session sync with the torrent stats - test_sleep(1000); - TEST_EQUAL(ses.status().total_payload_download - ses.status().total_redundant_bytes - , total_size - pad_file_size); + // commented out because it takes such a long time +// test_sleep(1000); +// TEST_EQUAL(ses.status().total_payload_download - ses.status().total_redundant_bytes +// , total_size - pad_file_size); break; } - test_sleep(500); + test_sleep(100); } // for test_ban tests, make sure we removed // the url seed (i.e. banned it) - TEST_CHECK(!test_ban || th.url_seeds().empty()); + TEST_CHECK(!test_ban || (th.url_seeds().empty() && th.http_seeds().empty())); TEST_EQUAL(cs.cache_size, 0); TEST_EQUAL(cs.total_used_buffers, 0); @@ -172,7 +174,14 @@ void test_transfer(boost::intrusive_ptr torrent_file // otherwise, we are supposed to have TEST_CHECK(th.status().is_seeding == !test_ban); - if (proxy) stop_proxy(8002); + if (proxy) stop_proxy(ps.port); + + ses.remove_torrent(th); + + // call this to synchronize with the network thread + ses.status(); + + print_alerts(ses, " >> ses", true, true, false, &on_alert, true); TEST_CHECK(exists(combine_path("tmp2_web_seed", torrent_file->files().file_path(0))) || test_ban); remove_all("tmp2_web_seed", ec); @@ -209,6 +218,8 @@ sha1_hash file_hash(std::string const& name) return h.final(); } +const int num_pieces = 9; + // test_url_seed determines whether to use url-seed or http-seed int run_suite(char const* protocol, bool test_url_seed, bool chunked_encoding, bool test_ban) { @@ -232,10 +243,10 @@ int run_suite(char const* protocol, bool test_url_seed, bool chunked_encoding, b else { piece_size = 64 * 1024; - char* random_data = (char*)malloc(64 * 1024 * 25); - std::generate(random_data, random_data + 64 * 1024 * 25, &std::rand); - save_file("tmp1_web_seed/seed", random_data, 64 * 1024 * 25); - fs.add_file("seed", 64 * 1024 * 25); + char* random_data = (char*)malloc(64 * 1024 * num_pieces); + std::generate(random_data, random_data + 64 * 1024 * num_pieces, &std::rand); + save_file("tmp1_web_seed/seed", random_data, 64 * 1024 * num_pieces); + fs.add_file("seed", 64 * 1024 * num_pieces); free(random_data); } @@ -265,8 +276,6 @@ int run_suite(char const* protocol, bool test_url_seed, bool chunked_encoding, b fprintf(stderr, " %04x: %d %s\n", int(f.offset), f.pad_file, f.path.c_str()); } -// for (int i = 0; i < 1000; ++i) sleep(1000); - // calculate the hash for all pieces set_piece_hashes(t, "tmp1_web_seed", ec); @@ -288,9 +297,9 @@ int run_suite(char const* protocol, bool test_url_seed, bool chunked_encoding, b else { piece_size = 64 * 1024; - char* random_data = (char*)malloc(64 * 1024 * 25); - std::generate(random_data, random_data + 64 * 1024 * 25, &std::rand); - save_file("tmp1_web_seed/seed", random_data, 64 * 1024 * 25); + char* random_data = (char*)malloc(64 * 1024 * num_pieces); + std::generate(random_data, random_data + 64 * 1024 * num_pieces, &std::rand); + save_file("tmp1_web_seed/seed", random_data, 64 * 1024 * num_pieces); free(random_data); } } @@ -315,13 +324,24 @@ int run_suite(char const* protocol, bool test_url_seed, bool chunked_encoding, b } } - for (int i = 0; i < 6; ++i) - test_transfer(torrent_file, i, port, protocol, test_url_seed, chunked_encoding, test_ban); - - if (test_url_seed) { - torrent_file->rename_file(0, "tmp2_web_seed/test_torrent_dir/renamed_test1"); - test_transfer(torrent_file, 0, port, protocol, test_url_seed, chunked_encoding, test_ban); + session ses(fingerprint(" ", 0,0,0,0), 0); + session_settings settings; + settings.max_queued_disk_bytes = 256 * 1024; + ses.set_settings(settings); + ses.set_alert_mask(~(alert::progress_notification | alert::stats_notification)); + error_code ec; + ses.listen_on(std::make_pair(51000, 52000), ec); + if (ec) fprintf(stderr, "listen_on failed: %s\n", ec.message().c_str()); + + for (int i = 0; i < 6; ++i) + test_transfer(ses, torrent_file, i, port, protocol, test_url_seed, chunked_encoding, test_ban); + + if (test_url_seed) + { + torrent_file->rename_file(0, "tmp2_web_seed/test_torrent_dir/renamed_test1"); + test_transfer(ses, torrent_file, 0, port, protocol, test_url_seed, chunked_encoding, test_ban); + } } stop_web_server();