From 2c3d7ee0ef3c80a36477b8b6b9b7690c100a07af Mon Sep 17 00:00:00 2001 From: arvidn Date: Sat, 28 Nov 2015 14:06:40 -0500 Subject: [PATCH] fix a bug where the torrent's invariant was not always maintained as well as a shutdown issue (now covered by test as well) --- include/libtorrent/performance_counters.hpp | 3 +- simulation/swarm_suite.cpp | 10 ++- simulation/swarm_suite.hpp | 3 +- simulation/test_swarm.cpp | 5 ++ src/session_impl.cpp | 54 ++++++++++---- src/torrent.cpp | 78 +++++++++++---------- 6 files changed, 98 insertions(+), 55 deletions(-) diff --git a/include/libtorrent/performance_counters.hpp b/include/libtorrent/performance_counters.hpp index f1aa7df6a..d9bd72696 100644 --- a/include/libtorrent/performance_counters.hpp +++ b/include/libtorrent/performance_counters.hpp @@ -319,8 +319,7 @@ namespace libtorrent { num_checking_torrents = num_stats_counters, num_stopped_torrents, - // upload_only means finished - num_upload_only_torrents, + num_upload_only_torrents, // upload_only means finished num_downloading_torrents, num_seeding_torrents, num_queued_seeding_torrents, diff --git a/simulation/swarm_suite.cpp b/simulation/swarm_suite.cpp index 4a7aed4ba..ad25aaae7 100644 --- a/simulation/swarm_suite.cpp +++ b/simulation/swarm_suite.cpp @@ -53,9 +53,17 @@ struct test_swarm_config : swarm_config , m_resumed_once(false) {} + virtual bool tick(int t) override + { + return (m_flags & early_shutdown) ? t >= 1 : t > 200; + } + virtual void on_exit(std::vector const& torrents) override { - swarm_config::on_exit(torrents); + if ((m_flags & early_shutdown) == 0) + { + swarm_config::on_exit(torrents); + } // if we stopped and started again, we loose some time and need a bit // more slack for completion diff --git a/simulation/swarm_suite.hpp b/simulation/swarm_suite.hpp index 0021c89ca..efd1a7c84 100644 --- a/simulation/swarm_suite.hpp +++ b/simulation/swarm_suite.hpp @@ -44,7 +44,8 @@ enum test_flags_t stop_start_download = 128, stop_start_seed = 256, graceful_pause = 1024, - add_extra_peers = 2048 + add_extra_peers = 2048, + early_shutdown = 4096 }; void EXPORT simulate_swarm(int flags = 0); diff --git a/simulation/test_swarm.cpp b/simulation/test_swarm.cpp index 3548ab590..7839df799 100644 --- a/simulation/test_swarm.cpp +++ b/simulation/test_swarm.cpp @@ -80,3 +80,8 @@ TORRENT_TEST(explicit_cache) simulate_swarm(suggest_read_cache | explicit_cache); } +TORRENT_TEST(shutdown) +{ + simulate_swarm(early_shutdown); +} + diff --git a/src/session_impl.cpp b/src/session_impl.cpp index 0dac584fe..ef3cfc2b3 100644 --- a/src/session_impl.cpp +++ b/src/session_impl.cpp @@ -1123,11 +1123,17 @@ namespace aux { m_ssl_udp_socket.close(); #endif - m_undead_peers.clear(); - - // give all the sockets an opportunity to actually have their handlers - // called and cancelled before we continue the shutdown - m_io_service.post(boost::bind(&session_impl::abort_stage2, this)); + // we need to give all the sockets an opportunity to actually have their handlers + // called and cancelled before we continue the shutdown. This is a bit + // complicated, if there are no "undead" peers, it's safe tor resume the + // shutdown, but if there are, we have to wait for them to be cleared out + // first. In session_impl::on_tick() we check them periodically. If we're + // shutting down and we remove the last one, we'll initiate + // shutdown_stage2 from there. + if (m_undead_peers.empty()) + { + m_io_service.post(boost::bind(&session_impl::abort_stage2, this)); + } } void session_impl::abort_stage2() @@ -2932,6 +2938,25 @@ retry: aux::update_time_now(); time_point now = aux::time_now(); + // remove undead peers that only have this list as their reference keeping them alive + if (!m_undead_peers.empty()) + { + std::vector >::iterator remove_it + = std::remove_if(m_undead_peers.begin(), m_undead_peers.end() + , boost::bind(&boost::shared_ptr::unique, _1)); + m_undead_peers.erase(remove_it, m_undead_peers.end()); + if (m_undead_peers.empty()) + { + // we just removed our last "undead" peer (i.e. a peer connection + // that had some external reference to it). It's now safe to + // shut-down + if (m_abort) + { + m_io_service.post(boost::bind(&session_impl::abort_stage2, this)); + } + } + } + // too expensive // INVARIANT_CHECK; @@ -2939,10 +2964,13 @@ retry: // until they're all closed if (m_abort) { - if (m_utp_socket_manager.num_sockets() == 0) + if (m_utp_socket_manager.num_sockets() == 0 + && m_undead_peers.empty()) return; #if defined TORRENT_ASIO_DEBUGGING - fprintf(stderr, "uTP sockets left: %d\n", m_utp_socket_manager.num_sockets()); + fprintf(stderr, "uTP sockets left: %d undead-peers left: %d\n" + , m_utp_socket_manager.num_sockets() + , int(m_undead_peers.size())); #endif } @@ -2980,12 +3008,6 @@ retry: update_dht_announce_interval(); #endif - // remove undead peers that only have this list as their reference keeping them alive - std::vector >::iterator remove_it - = std::remove_if(m_undead_peers.begin(), m_undead_peers.end() - , boost::bind(&boost::shared_ptr::unique, _1)); - m_undead_peers.erase(remove_it, m_undead_peers.end()); - int tick_interval_ms = int(total_milliseconds(now - m_last_second_tick)); m_last_second_tick = now; m_tick_residual += tick_interval_ms - 1000; @@ -3604,6 +3626,8 @@ retry: t->log_to_all_peers("auto manager starting (inactive) torrent"); #endif t->set_allow_peers(true); + t->update_gauge(); + t->update_want_peers(); continue; } @@ -3620,6 +3644,8 @@ retry: t->log_to_all_peers("auto manager starting torrent"); #endif t->set_allow_peers(true); + t->update_gauge(); + t->update_want_peers(); continue; } @@ -3632,6 +3658,8 @@ retry: t->set_announce_to_dht(false); t->set_announce_to_trackers(false); t->set_announce_to_lsd(false); + t->update_gauge(); + t->update_want_peers(); } } diff --git a/src/torrent.cpp b/src/torrent.cpp index f13bd3a06..95931433c 100644 --- a/src/torrent.cpp +++ b/src/torrent.cpp @@ -516,44 +516,6 @@ namespace libtorrent } #else // if 0 - int torrent::current_stats_state() const - { - if (m_abort) return counters::num_checking_torrents + no_gauge_state; - - if (has_error()) return counters::num_error_torrents; - if (!m_allow_peers || m_graceful_pause_mode) - { - if (!is_auto_managed()) return counters::num_stopped_torrents; - if (is_seed()) return counters::num_queued_seeding_torrents; - return counters::num_queued_download_torrents; - } - if (state() == torrent_status::checking_files -#ifndef TORRENT_NO_DEPRECATE - || state() == torrent_status::queued_for_checking -#endif - ) - return counters::num_checking_torrents; - else if (is_seed()) return counters::num_seeding_torrents; - else if (is_upload_only()) return counters::num_upload_only_torrents; - return counters::num_downloading_torrents; - } - - void torrent::update_gauge() - { - int new_gauge_state = current_stats_state() - counters::num_checking_torrents; - TORRENT_ASSERT(new_gauge_state >= 0); - TORRENT_ASSERT(new_gauge_state <= no_gauge_state); - - if (new_gauge_state == m_current_gauge_state) return; - - if (m_current_gauge_state != no_gauge_state) - inc_stats_counter(m_current_gauge_state + counters::num_checking_torrents, -1); - if (new_gauge_state != no_gauge_state) - inc_stats_counter(new_gauge_state + counters::num_checking_torrents, 1); - - m_current_gauge_state = new_gauge_state; - } - void torrent::on_torrent_download(error_code const& ec , http_parser const& parser, char const* data, int size) { @@ -663,6 +625,45 @@ namespace libtorrent #endif // if 0 + int torrent::current_stats_state() const + { + if (m_abort) return counters::num_checking_torrents + no_gauge_state; + + if (has_error()) return counters::num_error_torrents; + if (!m_allow_peers || m_graceful_pause_mode) + { + if (!is_auto_managed()) return counters::num_stopped_torrents; + if (is_seed()) return counters::num_queued_seeding_torrents; + return counters::num_queued_download_torrents; + } + if (state() == torrent_status::checking_files +#ifndef TORRENT_NO_DEPRECATE + || state() == torrent_status::queued_for_checking +#endif + ) + return counters::num_checking_torrents; + else if (is_seed()) return counters::num_seeding_torrents; + else if (is_upload_only()) return counters::num_upload_only_torrents; + return counters::num_downloading_torrents; + } + + void torrent::update_gauge() + { + int new_gauge_state = current_stats_state() - counters::num_checking_torrents; + TORRENT_ASSERT(new_gauge_state >= 0); + TORRENT_ASSERT(new_gauge_state <= no_gauge_state); + + if (new_gauge_state == m_current_gauge_state) return; + + if (m_current_gauge_state != no_gauge_state) + inc_stats_counter(m_current_gauge_state + counters::num_checking_torrents, -1); + if (new_gauge_state != no_gauge_state) + inc_stats_counter(new_gauge_state + counters::num_checking_torrents, 1); + + m_current_gauge_state = new_gauge_state; + } + + void torrent::leave_seed_mode(bool skip_checking) { if (!m_seed_mode) return; @@ -9787,6 +9788,7 @@ namespace libtorrent && graceful == false) { m_graceful_pause_mode = graceful; + update_gauge(); do_pause(); } return;