From 11d6a00bece86e5b2230ef043e05aefbc28191a4 Mon Sep 17 00:00:00 2001 From: d-komarov Date: Tue, 11 Jul 2017 07:35:34 +0300 Subject: [PATCH 1/4] fix storage destruction order issue (#2138) back-ported from b553cb32f785775b527bd53ea0153400c488e45e --- include/libtorrent/torrent.hpp | 4 ++++ src/torrent.cpp | 14 ++++++++++---- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/include/libtorrent/torrent.hpp b/include/libtorrent/torrent.hpp index 1f3c2ada7..1fc0329f2 100644 --- a/include/libtorrent/torrent.hpp +++ b/include/libtorrent/torrent.hpp @@ -1168,6 +1168,10 @@ namespace libtorrent void on_file_renamed(disk_io_job const* j); void on_cache_flushed(disk_io_job const* j); + // this is used when a torrent is being removed.It synchronizes with the + // disk thread + void on_torrent_aborted(); + // upload and download rate limits for the torrent void set_limit_impl(int limit, int channel, bool state_update = true); int limit_impl(int channel) const; diff --git a/src/torrent.cpp b/src/torrent.cpp index 1a82dfbae..fa5f8660b 100644 --- a/src/torrent.cpp +++ b/src/torrent.cpp @@ -5003,9 +5003,8 @@ namespace libtorrent // the torrent object from there if (m_storage.get()) { - inc_refcount("release_files"); m_ses.disk_thread().async_stop_torrent(m_storage.get() - , boost::bind(&torrent::on_cache_flushed, shared_from_this(), _1)); + , boost::bind(&torrent::on_torrent_aborted, shared_from_this())); } else { @@ -5014,8 +5013,6 @@ namespace libtorrent alerts().emplace_alert(get_handle()); } - m_storage.reset(); - // TODO: 2 abort lookups this torrent has made via the // session host resolver interface @@ -9821,6 +9818,15 @@ namespace libtorrent alerts().emplace_alert(get_handle()); } + void torrent::on_torrent_aborted() + { + TORRENT_ASSERT(is_single_thread()); + + // there should be no more disk activity for this torrent now, we can + // release the disk io handle + m_storage.reset(); + } + bool torrent::is_paused() const { return !m_allow_peers || m_ses.is_paused() || m_graceful_pause_mode; From 85cf521145e421934406d2d1e4a2108a98f72136 Mon Sep 17 00:00:00 2001 From: d-komarov Date: Wed, 12 Jul 2017 07:35:12 +0300 Subject: [PATCH 2/4] fix bandwith rate limit calculation (#2134) back-port of fc0cbfb7891b34541065d3ae763ade9cc8a95a10 --- include/libtorrent/bandwidth_limit.hpp | 10 ++++----- src/bandwidth_limit.cpp | 28 +++++++++++++++++--------- test/test_bandwidth_limiter.cpp | 1 + 3 files changed, 24 insertions(+), 15 deletions(-) diff --git a/include/libtorrent/bandwidth_limit.hpp b/include/libtorrent/bandwidth_limit.hpp index 3c7a3f8fe..259b2a308 100644 --- a/include/libtorrent/bandwidth_limit.hpp +++ b/include/libtorrent/bandwidth_limit.hpp @@ -47,7 +47,7 @@ namespace libtorrent { // member of peer_connection struct TORRENT_EXTRA_EXPORT bandwidth_channel { - static const int inf = boost::integer_traits::const_max; + static boost::int32_t const inf = boost::integer_traits::const_max; bandwidth_channel(); @@ -55,8 +55,9 @@ struct TORRENT_EXTRA_EXPORT bandwidth_channel void throttle(int limit); int throttle() const { - TORRENT_ASSERT_VAL(m_limit < INT_MAX, m_limit); - return int(m_limit); + TORRENT_ASSERT_VAL(m_limit >= 0, m_limit); + TORRENT_ASSERT_VAL(m_limit < inf, m_limit); + return m_limit; } int quota_left() const; @@ -96,10 +97,9 @@ private: // the limit is the number of bytes // per second we are allowed to use. - boost::int64_t m_limit; + boost::int32_t m_limit; }; } #endif - diff --git a/src/bandwidth_limit.cpp b/src/bandwidth_limit.cpp index 01d6f7f6b..866138765 100644 --- a/src/bandwidth_limit.cpp +++ b/src/bandwidth_limit.cpp @@ -33,8 +33,8 @@ POSSIBILITY OF SUCH DAMAGE. #include "libtorrent/bandwidth_limit.hpp" #include -namespace libtorrent -{ +namespace libtorrent { + bandwidth_channel::bandwidth_channel() : tmp(0) , distribute_quota(0) @@ -45,9 +45,9 @@ namespace libtorrent // 0 means infinite void bandwidth_channel::throttle(int limit) { - TORRENT_ASSERT(limit >= 0); + TORRENT_ASSERT_VAL(limit >= 0, limit); // if the throttle is more than this, we might overflow - TORRENT_ASSERT(limit < INT_MAX); + TORRENT_ASSERT_VAL(limit < inf, limit); m_limit = limit; } @@ -59,19 +59,27 @@ namespace libtorrent void bandwidth_channel::update_quota(int dt_milliseconds) { + TORRENT_ASSERT_VAL(m_limit >= 0, m_limit); + TORRENT_ASSERT_VAL(m_limit < inf, m_limit); + if (m_limit == 0) return; - // avoid integer overflow - if (m_limit >= std::numeric_limits::max() / dt_milliseconds) + // "to_add" should never have int64 overflow: "m_limit" contains < "::max" + boost::int64_t to_add = (boost::int64_t(m_limit) * dt_milliseconds + 500) / 1000; + + if (to_add > inf - m_quota_left) { - m_quota_left = std::numeric_limits::max(); + m_quota_left = inf; } else { - m_quota_left += (m_limit * dt_milliseconds + 500) / 1000; - if (m_quota_left / 3 > m_limit) m_quota_left = m_limit * 3; + m_quota_left += to_add; + if (m_quota_left / 3 > m_limit) m_quota_left = boost::int64_t(m_limit) * 3; + // "m_quota_left" will never have int64 overflow but may exceed "::max" + m_quota_left = std::min(m_quota_left, boost::int64_t(inf)); } - distribute_quota = int((std::max)(m_quota_left, boost::int64_t(0))); + + distribute_quota = int(std::max(m_quota_left, boost::int64_t(0))); } // this is used when connections disconnect with diff --git a/test/test_bandwidth_limiter.cpp b/test/test_bandwidth_limiter.cpp index 4d9763163..a7f4b6eaa 100644 --- a/test/test_bandwidth_limiter.cpp +++ b/test/test_bandwidth_limiter.cpp @@ -467,6 +467,7 @@ TORRENT_TEST(equal_connection) test_equal_connections(33, 60000); test_equal_connections(33, 500000); test_equal_connections( 1, 1000000); + test_equal_connections( 1, 6000000); } TORRENT_TEST(conn_var_rate) From 7eb3cf6bc6dbada3fa7bb7ff4d5981182813a0e2 Mon Sep 17 00:00:00 2001 From: arvidn Date: Tue, 11 Jul 2017 23:16:50 -0700 Subject: [PATCH 3/4] use the official boost.config header --- include/libtorrent/export.hpp | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/include/libtorrent/export.hpp b/include/libtorrent/export.hpp index 87536af2a..503afe27a 100644 --- a/include/libtorrent/export.hpp +++ b/include/libtorrent/export.hpp @@ -33,19 +33,7 @@ POSSIBILITY OF SUCH DAMAGE. #ifndef TORRENT_EXPORT_HPP_INCLUDED #define TORRENT_EXPORT_HPP_INCLUDED -#if !defined(BOOST_COMPILER_CONFIG) && !defined(BOOST_NO_COMPILER_CONFIG) -# include -#endif -#ifdef BOOST_COMPILER_CONFIG -# include BOOST_COMPILER_CONFIG -#endif - -#if !defined(BOOST_PLATFORM_CONFIG) && !defined(BOOST_NO_PLATFORM_CONFIG) -# include -#endif -#ifdef BOOST_PLATFORM_CONFIG -# include BOOST_PLATFORM_CONFIG -#endif +#include // backwards compatibility with older versions of boost #if !defined BOOST_SYMBOL_EXPORT && !defined BOOST_SYMBOL_IMPORT From cc30434c52d8c96922be8de51b4806273d2ea445 Mon Sep 17 00:00:00 2001 From: arvidn Date: Sun, 9 Jul 2017 09:07:28 -0700 Subject: [PATCH 4/4] fix inconsistency in file_priorities and override_resume_data behavior. file_priorities are not subject to the override_resume_data flag --- ChangeLog | 2 ++ include/libtorrent/add_torrent_params.hpp | 7 ++-- src/torrent.cpp | 16 +++++++--- test/test_resume.cpp | 39 +++++++++++++++++++++++ 4 files changed, 56 insertions(+), 8 deletions(-) diff --git a/ChangeLog b/ChangeLog index ed815d253..fd7269b6e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,5 @@ + * fix inconsistency in file_priorities and override_resume_data behavior + 1.1.4 release * corrected missing const qualifiers on bdecode_node diff --git a/include/libtorrent/add_torrent_params.hpp b/include/libtorrent/add_torrent_params.hpp index e86636646..99c004857 100644 --- a/include/libtorrent/add_torrent_params.hpp +++ b/include/libtorrent/add_torrent_params.hpp @@ -157,8 +157,7 @@ namespace libtorrent // add_torrent_params configuring the torrent override the corresponding // configuration from the resume file, with the one exception of save // resume data, which has its own flag (for historic reasons). - // If this flag is set, but file_priorities is empty, file priorities - // are still loaded from the resume data, if present. + // "file_priorities" and "save_path" are not affected by this flag. flag_override_resume_data = 0x002, // If ``flag_upload_mode`` is set, the torrent will be initialized in @@ -347,7 +346,9 @@ namespace libtorrent // can be set to control the initial file priorities when adding a // torrent. The semantics are the same as for - // ``torrent_handle::prioritize_files()``. + // ``torrent_handle::prioritize_files()``. The file priorities specified + // in here take precedence over those specified in the resume data, if + // any. std::vector file_priorities; // torrent extension construction functions can be added to this vector diff --git a/src/torrent.cpp b/src/torrent.cpp index fa5f8660b..54a9dfac7 100644 --- a/src/torrent.cpp +++ b/src/torrent.cpp @@ -5637,7 +5637,7 @@ namespace libtorrent { inc_refcount("file_priority"); m_ses.disk_thread().async_set_file_priority(m_storage.get() - , m_file_priority, boost::bind(&torrent::on_file_priority, this)); + , m_file_priority, boost::bind(&torrent::on_file_priority, shared_from_this())); } update_piece_priorities(); @@ -5676,7 +5676,7 @@ namespace libtorrent { inc_refcount("file_priority"); m_ses.disk_thread().async_set_file_priority(m_storage.get() - , m_file_priority, boost::bind(&torrent::on_file_priority, this)); + , m_file_priority, boost::bind(&torrent::on_file_priority, shared_from_this())); } update_piece_priorities(); } @@ -7041,9 +7041,7 @@ namespace libtorrent if (m_completed_time != 0 && m_completed_time < m_added_time) m_completed_time = m_added_time; - // load file priorities except if the add_torrent_param file was set to - // override resume data - if (!m_override_resume_data || m_file_priority.empty()) + if (m_file_priority.empty()) { bdecode_node file_priority = rd.dict_find_list("file_priority"); if (file_priority) @@ -7071,6 +7069,14 @@ namespace libtorrent m_file_priority[i] = 0; } + // storage may be NULL during shutdown + if (m_storage) + { + inc_refcount("file_priority"); + m_ses.disk_thread().async_set_file_priority(m_storage.get() + , m_file_priority, boost::bind(&torrent::on_file_priority, shared_from_this())); + } + update_piece_priorities(); } } diff --git a/test/test_resume.cpp b/test/test_resume.cpp index de871d33a..d8e1acc7b 100644 --- a/test/test_resume.cpp +++ b/test/test_resume.cpp @@ -308,6 +308,45 @@ TORRENT_TEST(file_priorities_default) TEST_EQUAL(file_priorities[2], 4); } +// As long as the add_torrent_params priorities are empty, the file_priorities +// from the resume data should take effect +TORRENT_TEST(file_priorities_in_resume) +{ + lt::session ses(settings()); + std::vector file_priorities = test_resume_flags(ses, 0, "", "123").file_priorities(); + + TEST_EQUAL(file_priorities.size(), 3); + TEST_EQUAL(file_priorities[0], 1); + TEST_EQUAL(file_priorities[1], 2); + TEST_EQUAL(file_priorities[2], 3); +} + +// if both resume data and add_torrent_params has file_priorities, the +// add_torrent_params one take precedence +TORRENT_TEST(file_priorities_in_resume_and_params) +{ + lt::session ses(settings()); + std::vector file_priorities = test_resume_flags(ses, 0, "456", "123").file_priorities(); + + TEST_EQUAL(file_priorities.size(), 3); + TEST_EQUAL(file_priorities[0], 4); + TEST_EQUAL(file_priorities[1], 5); + TEST_EQUAL(file_priorities[2], 6); +} + +// if we set flag_override_resume_data, it should no affect file priorities +TORRENT_TEST(file_priorities_override_resume) +{ + lt::session ses(settings()); + std::vector file_priorities = test_resume_flags(ses + , add_torrent_params::flag_override_resume_data, "", "123").file_priorities(); + + TEST_EQUAL(file_priorities.size(), 3); + TEST_EQUAL(file_priorities[0], 1); + TEST_EQUAL(file_priorities[1], 2); + TEST_EQUAL(file_priorities[2], 3); +} + TORRENT_TEST(file_priorities_resume_seed_mode) { // in share mode file priorities should always be 0