diff --git a/ChangeLog b/ChangeLog index b9370be6c..90596a4b1 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,7 @@ 1.1.1 release + * make sure add_torrent_alert is always posted before other alerts for + the torrent * fixed peer-class leak when settings per-torrent rate limits * added a new "preformatted" type to bencode entry variant type * improved Socks5 support and test coverage diff --git a/include/libtorrent/aux_/session_impl.hpp b/include/libtorrent/aux_/session_impl.hpp index 49ff38d3e..fe00f8e9a 100644 --- a/include/libtorrent/aux_/session_impl.hpp +++ b/include/libtorrent/aux_/session_impl.hpp @@ -406,7 +406,7 @@ namespace libtorrent #endif torrent_handle add_torrent(add_torrent_params const&, error_code& ec); - torrent_handle add_torrent_impl(add_torrent_params const&, error_code& ec); + boost::shared_ptr add_torrent_impl(add_torrent_params& p, error_code& ec); void async_add_torrent(add_torrent_params* params); void on_async_load_torrent(disk_io_job const* j); diff --git a/simulation/test_torrent_status.cpp b/simulation/test_torrent_status.cpp index 2addf6905..7b6959015 100644 --- a/simulation/test_torrent_status.cpp +++ b/simulation/test_torrent_status.cpp @@ -34,6 +34,7 @@ POSSIBILITY OF SUCH DAMAGE. #include "setup_swarm.hpp" #include "simulator/simulator.hpp" #include "libtorrent/alert_types.hpp" +#include "libtorrent/settings_pack.hpp" using namespace libtorrent; using namespace sim; @@ -98,3 +99,45 @@ TORRENT_TEST(status_timers) }); } +// This test makes sure that adding a torrent causes no torrent related alert to +// be posted _before_ the add_torrent_alert, which is expected to always be the +// first +TORRENT_TEST(alert_order) +{ + bool received_add_torrent_alert = false; + int num_torrent_alerts = 0; + + lt::torrent_handle handle; + + setup_swarm(1, swarm_test::upload + // add session + , [](lt::settings_pack& sett) { + sett.set_int(settings_pack::alert_mask, alert::all_categories); + } + // add torrent + , [](lt::add_torrent_params& params) {} + // on alert + , [&](lt::alert const* a, lt::session& ses) { + if (auto ta = alert_cast(a)) + { + TEST_EQUAL(received_add_torrent_alert, false); + received_add_torrent_alert = true; + handle = ta->handle; + } + + if (auto ta = dynamic_cast(a)) + { + TEST_EQUAL(received_add_torrent_alert, true); + TEST_CHECK(handle == ta->handle); + ++num_torrent_alerts; + } + } + // terminate + , [&](int ticks, lt::session& ses) -> bool + { return ticks > 10; } + ); + + TEST_EQUAL(received_add_torrent_alert, true); + TEST_CHECK(num_torrent_alerts > 1); +} + diff --git a/src/session_impl.cpp b/src/session_impl.cpp index 6efe287c7..6bfa31b1f 100644 --- a/src/session_impl.cpp +++ b/src/session_impl.cpp @@ -4777,188 +4777,20 @@ retry: torrent_handle session_impl::add_torrent(add_torrent_params const& p , error_code& ec) { - torrent_handle h = add_torrent_impl(p, ec); - m_alerts.emplace_alert(h, p, ec); - return h; - } - - torrent_handle session_impl::add_torrent_impl(add_torrent_params const& p - , error_code& ec) - { - TORRENT_ASSERT(!p.save_path.empty()); - -#ifndef TORRENT_NO_DEPRECATE - p.update_flags(); -#endif - + // params is updated by add_torrent_impl() add_torrent_params params = p; - if (string_begins_no_case("magnet:", params.url.c_str())) - { - parse_magnet_uri(params.url, params, ec); - if (ec) return torrent_handle(); - params.url.clear(); - } + boost::shared_ptr const torrent_ptr = add_torrent_impl(params, ec); - if (string_begins_no_case("file://", params.url.c_str()) && !params.ti) - { - std::string filename = resolve_file_url(params.url); - boost::shared_ptr t = boost::make_shared(filename, boost::ref(ec), 0); - if (ec) return torrent_handle(); - params.url.clear(); - params.ti = t; - } + torrent_handle const handle(torrent_ptr); + m_alerts.emplace_alert(handle, params, ec); - if (params.ti && params.ti->is_valid() && params.ti->num_files() == 0) - { - ec = errors::no_files_in_torrent; - return torrent_handle(); - } + if (!torrent_ptr) return handle; -#ifndef TORRENT_DISABLE_DHT - // add p.dht_nodes to the DHT, if enabled - if (!p.dht_nodes.empty()) - { - for (std::vector >::const_iterator i = p.dht_nodes.begin() - , end(p.dht_nodes.end()); i != end; ++i) - { - add_dht_node_name(*i); - } - } -#endif - - INVARIANT_CHECK; - - if (is_aborted()) - { - ec = errors::session_is_closing; - return torrent_handle(); - } - - // figure out the info hash of the torrent - sha1_hash const* ih = 0; - sha1_hash tmp; - if (params.ti) ih = ¶ms.ti->info_hash(); - else if (!params.url.empty()) - { - // in order to avoid info-hash collisions, for - // torrents where we don't have an info-hash, but - // just a URL, set the temporary info-hash to the - // hash of the URL. This will be changed once we - // have the actual .torrent file - tmp = hasher(¶ms.url[0], params.url.size()).final(); - ih = &tmp; - } - else ih = ¶ms.info_hash; - - // we don't have a torrent file. If the user provided - // resume data, there may be some metadata in there - // TODO: this logic could probably be less spaghetti looking by being - // moved to a function with early exits - if ((!params.ti || !params.ti->is_valid()) - && !params.resume_data.empty()) - { - int pos; - error_code err; - bdecode_node root; - bdecode_node info; -#ifndef TORRENT_DISABLE_LOGGING - session_log("adding magnet link with resume data"); -#endif - if (bdecode(¶ms.resume_data[0], ¶ms.resume_data[0] - + params.resume_data.size(), root, err, &pos) == 0 - && root.type() == bdecode_node::dict_t - && (info = root.dict_find_dict("info"))) - { -#ifndef TORRENT_DISABLE_LOGGING - session_log("found metadata in resume data"); -#endif - // verify the info-hash of the metadata stored in the resume file matches - // the torrent we're loading - - std::pair buf = info.data_section(); - sha1_hash resume_ih = hasher(buf.first, buf.second).final(); - - // if url is set, the info_hash is not actually the info-hash of the - // torrent, but the hash of the URL, until we have the full torrent - // only require the info-hash to match if we actually passed in one - if (resume_ih == params.info_hash - || !params.url.empty() - || params.info_hash.is_all_zeros()) - { -#ifndef TORRENT_DISABLE_LOGGING - session_log("info-hash matched"); -#endif - params.ti = boost::make_shared(resume_ih); - - if (params.ti->parse_info_section(info, err, 0)) - { -#ifndef TORRENT_DISABLE_LOGGING - session_log("successfully loaded metadata from resume file"); -#endif - // make the info-hash be the one in the resume file - params.info_hash = resume_ih; - ih = ¶ms.info_hash; - } - else - { -#ifndef TORRENT_DISABLE_LOGGING - session_log("failed to load metadata from resume file: %s" - , err.message().c_str()); -#endif - } - } -#ifndef TORRENT_DISABLE_LOGGING - else - { - session_log("metadata info-hash failed"); - } -#endif - } -#ifndef TORRENT_DISABLE_LOGGING - else - { - session_log("no metadata found (\"%s\")", err.message().c_str()); - } -#endif - } - - // is the torrent already active? - boost::shared_ptr torrent_ptr = find_torrent(*ih).lock(); - if (!torrent_ptr && !params.uuid.empty()) torrent_ptr = find_torrent(params.uuid).lock(); - // if we still can't find the torrent, look for it by url - if (!torrent_ptr && !params.url.empty()) - { - torrent_map::iterator i = std::find_if(m_torrents.begin() - , m_torrents.end(), boost::bind(&torrent::url, boost::bind(&std::pair >::second, _1)) == params.url); - if (i != m_torrents.end()) - torrent_ptr = i->second; - } - - if (torrent_ptr) - { - if ((params.flags & add_torrent_params::flag_duplicate_is_error) == 0) - { - if (!params.uuid.empty() && torrent_ptr->uuid().empty()) - torrent_ptr->set_uuid(params.uuid); - if (!params.url.empty() && torrent_ptr->url().empty()) - torrent_ptr->set_url(params.url); - if (!params.source_feed_url.empty() && torrent_ptr->source_feed_url().empty()) - torrent_ptr->set_source_feed_url(params.source_feed_url); - return torrent_handle(torrent_ptr); - } - - ec = errors::duplicate_torrent; - return torrent_handle(); - } - - int queue_pos = ++m_max_queue_pos; - - torrent_ptr = boost::make_shared(boost::ref(*this) - , 16 * 1024, queue_pos, boost::cref(params), boost::cref(*ih)); + // params.info_hash should have been initialized by add_torrent_impl() + TORRENT_ASSERT(params.info_hash != sha1_hash(0)); if (m_alerts.should_post()) - m_alerts.emplace_alert(torrent_ptr->get_handle()); + m_alerts.emplace_alert(handle); torrent_ptr->set_ip_filter(m_ip_filter); torrent_ptr->start(params); @@ -4971,8 +4803,7 @@ retry: for (torrent_plugins_t::const_iterator i = params.extensions.begin() , end(params.extensions.end()); i != end; ++i) { - torrent_ptr->add_extension((*i)(torrent_ptr->get_handle(), - params.userdata)); + torrent_ptr->add_extension((*i)(handle, params.userdata)); } add_extensions_to_torrent(torrent_ptr, params.userdata); @@ -5002,14 +4833,14 @@ retry: float load_factor = m_torrents.load_factor(); #endif // TORRENT_HAS_BOOST_UNORDERED - m_torrents.insert(std::make_pair(*ih, torrent_ptr)); + m_torrents.insert(std::make_pair(params.info_hash, torrent_ptr)); TORRENT_ASSERT(m_torrents.size() >= m_torrent_lru.size()); #if !defined(TORRENT_DISABLE_ENCRYPTION) && !defined(TORRENT_DISABLE_EXTENSIONS) hasher h; h.update("req2", 4); - h.update(ih->data(), 20); + h.update(params.info_hash.data(), 20); // this is SHA1("req2" + info-hash), used for // encrypted hand shakes m_obfuscated_torrents.insert(std::make_pair(h.final(), torrent_ptr)); @@ -5067,7 +4898,183 @@ retry: } } - return torrent_handle(torrent_ptr); + return handle; + } + + boost::shared_ptr session_impl::add_torrent_impl( + add_torrent_params& params + , error_code& ec) + { + TORRENT_ASSERT(!params.save_path.empty()); + + typedef boost::shared_ptr ptr_t; + +#ifndef TORRENT_NO_DEPRECATE + params.update_flags(); +#endif + + if (string_begins_no_case("magnet:", params.url.c_str())) + { + parse_magnet_uri(params.url, params, ec); + if (ec) return ptr_t(); + params.url.clear(); + } + + if (string_begins_no_case("file://", params.url.c_str()) && !params.ti) + { + std::string filename = resolve_file_url(params.url); + boost::shared_ptr t = boost::make_shared(filename, boost::ref(ec), 0); + if (ec) return ptr_t(); + params.url.clear(); + params.ti = t; + } + + if (params.ti && params.ti->is_valid() && params.ti->num_files() == 0) + { + ec = errors::no_files_in_torrent; + return ptr_t(); + } + +#ifndef TORRENT_DISABLE_DHT + // add params.dht_nodes to the DHT, if enabled + if (!params.dht_nodes.empty()) + { + for (std::vector >::const_iterator i = params.dht_nodes.begin() + , end(params.dht_nodes.end()); i != end; ++i) + { + add_dht_node_name(*i); + } + } +#endif + + INVARIANT_CHECK; + + if (is_aborted()) + { + ec = errors::session_is_closing; + return ptr_t(); + } + + // figure out the info hash of the torrent and make sure params.info_hash + // is set correctly + if (params.ti) params.info_hash = params.ti->info_hash(); + else if (!params.url.empty()) + { + // in order to avoid info-hash collisions, for + // torrents where we don't have an info-hash, but + // just a URL, set the temporary info-hash to the + // hash of the URL. This will be changed once we + // have the actual .torrent file + params.info_hash = hasher(¶ms.url[0], params.url.size()).final(); + } + + // we don't have a torrent file. If the user provided + // resume data, there may be some metadata in there + // TODO: this logic could probably be less spaghetti looking by being + // moved to a function with early exits + if ((!params.ti || !params.ti->is_valid()) + && !params.resume_data.empty()) + { + int pos; + error_code err; + bdecode_node root; + bdecode_node info; +#ifndef TORRENT_DISABLE_LOGGING + session_log("adding magnet link with resume data"); +#endif + if (bdecode(¶ms.resume_data[0], ¶ms.resume_data[0] + + params.resume_data.size(), root, err, &pos) == 0 + && root.type() == bdecode_node::dict_t + && (info = root.dict_find_dict("info"))) + { +#ifndef TORRENT_DISABLE_LOGGING + session_log("found metadata in resume data"); +#endif + // verify the info-hash of the metadata stored in the resume file matches + // the torrent we're loading + + std::pair const buf = info.data_section(); + sha1_hash const resume_ih = hasher(buf.first, buf.second).final(); + + // if url is set, the info_hash is not actually the info-hash of the + // torrent, but the hash of the URL, until we have the full torrent + // only require the info-hash to match if we actually passed in one + if (resume_ih == params.info_hash + || !params.url.empty() + || params.info_hash.is_all_zeros()) + { +#ifndef TORRENT_DISABLE_LOGGING + session_log("info-hash matched"); +#endif + params.ti = boost::make_shared(resume_ih); + + if (params.ti->parse_info_section(info, err, 0)) + { +#ifndef TORRENT_DISABLE_LOGGING + session_log("successfully loaded metadata from resume file"); +#endif + // make the info-hash be the one in the resume file + params.info_hash = resume_ih; + } + else + { +#ifndef TORRENT_DISABLE_LOGGING + session_log("failed to load metadata from resume file: %s" + , err.message().c_str()); +#endif + } + } +#ifndef TORRENT_DISABLE_LOGGING + else + { + session_log("metadata info-hash failed"); + } +#endif + } +#ifndef TORRENT_DISABLE_LOGGING + else + { + session_log("no metadata found (\"%s\")", err.message().c_str()); + } +#endif + } + + // is the torrent already active? + boost::shared_ptr torrent_ptr = find_torrent(params.info_hash).lock(); + if (!torrent_ptr && !params.uuid.empty()) torrent_ptr = find_torrent(params.uuid).lock(); + // if we still can't find the torrent, look for it by url + if (!torrent_ptr && !params.url.empty()) + { + torrent_map::iterator i = std::find_if(m_torrents.begin() + , m_torrents.end(), boost::bind(&torrent::url, boost::bind(&std::pair >::second, _1)) == params.url); + if (i != m_torrents.end()) + torrent_ptr = i->second; + } + + if (torrent_ptr) + { + if ((params.flags & add_torrent_params::flag_duplicate_is_error) == 0) + { + if (!params.uuid.empty() && torrent_ptr->uuid().empty()) + torrent_ptr->set_uuid(params.uuid); + if (!params.url.empty() && torrent_ptr->url().empty()) + torrent_ptr->set_url(params.url); + if (!params.source_feed_url.empty() && torrent_ptr->source_feed_url().empty()) + torrent_ptr->set_source_feed_url(params.source_feed_url); + return torrent_ptr; + } + + ec = errors::duplicate_torrent; + return ptr_t(); + } + + int queue_pos = ++m_max_queue_pos; + + torrent_ptr = boost::make_shared(boost::ref(*this) + , 16 * 1024, queue_pos, boost::cref(params), boost::cref(params.info_hash)); + + return torrent_ptr; } void session_impl::update_outgoing_interfaces()