From 05f7a95604b736b71c4744936a9f5238b8a37ab3 Mon Sep 17 00:00:00 2001 From: arvidn Date: Wed, 7 Mar 2018 13:19:39 +0100 Subject: [PATCH 1/3] fix reporting &redundant= in tracker announces --- ChangeLog | 1 + src/torrent.cpp | 2 ++ 2 files changed, 3 insertions(+) diff --git a/ChangeLog b/ChangeLog index 0ed5fc3c8..fdab05eb6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,4 +1,5 @@ + * fix reporting &redundant= in tracker announces * fix tie-break in duplicate peer connection disconnect logic * fix issue with SSL tracker connections left in CLOSE_WAIT state * defer truncating existing files until the first time we write to them diff --git a/src/torrent.cpp b/src/torrent.cpp index 488c70698..804ba0072 100644 --- a/src/torrent.cpp +++ b/src/torrent.cpp @@ -3203,6 +3203,8 @@ namespace { // exclude redundant bytes if we should if (!settings().get_bool(settings_pack::report_true_downloaded)) req.downloaded -= m_total_redundant_bytes; + if (settings().get_bool(settings_pack::report_redundant_bytes)) + req.redundant = m_total_redundant_bytes; if (req.downloaded < 0) req.downloaded = 0; req.event = e; From b841860643fb22ae90e52d6d81df185959c59fcc Mon Sep 17 00:00:00 2001 From: arvidn Date: Fri, 9 Mar 2018 12:00:56 +0100 Subject: [PATCH 2/3] improve python binding for torrent_handle::connect_peer --- bindings/python/src/torrent_handle.cpp | 6 +++--- bindings/python/test.py | 4 ++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/bindings/python/src/torrent_handle.cpp b/bindings/python/src/torrent_handle.cpp index 80c7eb52e..c440aa46a 100644 --- a/bindings/python/src/torrent_handle.cpp +++ b/bindings/python/src/torrent_handle.cpp @@ -339,9 +339,9 @@ namespace } } -void connect_peer(torrent_handle& th, tuple ip, int source) +void connect_peer(torrent_handle& th, tuple ip, int source, int flags) { - th.connect_peer(tuple_to_endpoint(ip), source); + th.connect_peer(tuple_to_endpoint(ip), source, flags); } std::vector file_status(torrent_handle const& h) @@ -516,7 +516,7 @@ void bind_torrent_handle() .def("set_ratio", _(&torrent_handle::set_ratio)) .def("save_path", _(&torrent_handle::save_path)) #endif - .def("connect_peer", &connect_peer) + .def("connect_peer", &connect_peer, (arg("ip"), arg("source") = 0, arg("flags") = 0xd)) .def("set_max_uploads", _(&torrent_handle::set_max_uploads)) .def("max_uploads", _(&torrent_handle::max_uploads)) .def("set_max_connections", _(&torrent_handle::set_max_connections)) diff --git a/bindings/python/test.py b/bindings/python/test.py index 13348db0d..1f5b9292c 100644 --- a/bindings/python/test.py +++ b/bindings/python/test.py @@ -58,6 +58,10 @@ class test_torrent_handle(unittest.TestCase): # also test the overload that takes a list of piece->priority mappings self.h.prioritize_pieces([(0, 1)]) self.assertEqual(self.h.piece_priorities(), [1]) + self.h.connect_peer(('127.0.0.1', 6881)) + self.h.connect_peer(('127.0.0.2', 6881), source=4) + self.h.connect_peer(('127.0.0.3', 6881), flags=2) + self.h.connect_peer(('127.0.0.4', 6881), flags=2, source=4) def test_torrent_handle_in_set(self): self.setup() From a06e4f696b9a08cae81a4b6e5fa038a80e51837d Mon Sep 17 00:00:00 2001 From: arvidn Date: Sun, 25 Feb 2018 17:17:42 +0100 Subject: [PATCH 3/3] track whether a file is eligible for using the partfile on a per-file basis. This is intended to improve backwards compatibility, to better support resuming files downloaded with older versions of libtorrent --- ChangeLog | 2 + docs/hunspell/libtorrent.dic | 2 + include/libtorrent/storage.hpp | 19 ++++-- include/libtorrent/torrent_status.hpp | 5 +- simulation/create_torrent.hpp | 4 +- simulation/test_checking.cpp | 94 +++++++++++++++++++++++++++ src/create_torrent.cpp | 16 +++++ src/storage.cpp | 87 ++++++++++++++++--------- src/torrent.cpp | 12 ++-- test/setup_transfer.cpp | 5 +- 10 files changed, 200 insertions(+), 46 deletions(-) diff --git a/ChangeLog b/ChangeLog index fdab05eb6..b1e5fa93e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,4 +1,6 @@ + * fix backwards compatibility to downloads without partfiles + * improve part-file related error messages * fix reporting &redundant= in tracker announces * fix tie-break in duplicate peer connection disconnect logic * fix issue with SSL tracker connections left in CLOSE_WAIT state diff --git a/docs/hunspell/libtorrent.dic b/docs/hunspell/libtorrent.dic index 1fa52c510..773d0df51 100644 --- a/docs/hunspell/libtorrent.dic +++ b/docs/hunspell/libtorrent.dic @@ -189,3 +189,5 @@ src 'fingerprints' 'query' 'ro' +pre-partfile +pre diff --git a/include/libtorrent/storage.hpp b/include/libtorrent/storage.hpp index a5051b25e..962fb74d3 100644 --- a/include/libtorrent/storage.hpp +++ b/include/libtorrent/storage.hpp @@ -475,16 +475,23 @@ namespace libtorrent file_handle open_file(int file, int mode, storage_error& ec) const; file_handle open_file_impl(int file, int mode, error_code& ec) const; + bool use_partfile(int index); + void use_partfile(int index, bool b); + std::vector m_file_priority; std::string m_save_path; std::string m_part_file_name; - // if this is false, we're not using a part file to store priority-0 - // pieces, but we instead look for them under their actual file names - // this defaults to true, but when checking resume data for a torrent - // where we would expect to have a part file, but there isn't one, we set - // this to false. - bool m_use_part_file; + // this this is an array indexed by file-index. Each slot represents + // whether this file has the part-file enabled for it. This is used for + // backwards compatibility with pre-partfile versions of libtorrent. If + // this vector is empty, the default is that files *do* use the partfile. + // on startup, any 0-priority file that's found in it's original location + // is expected to be an old-style (pre-partfile) torrent storage, and + // those files have their slot set to false in this vector. + // note that the vector is *sparse*, it's only allocated if a file has its + // entry set to false, and only indices up to that entry. + std::vector m_use_partfile; // the file pool is typically stored in // the session, to make all storage diff --git a/include/libtorrent/torrent_status.hpp b/include/libtorrent/torrent_status.hpp index 47342debb..5eec35554 100644 --- a/include/libtorrent/torrent_status.hpp +++ b/include/libtorrent/torrent_status.hpp @@ -146,7 +146,10 @@ namespace libtorrent // the error occurred while loading the .torrent file via the user // supplied load function - error_file_metadata = -4 + error_file_metadata = -4, + + // the error occurred with the partfile + error_file_partfile = -5 }; // the path to the directory where this torrent's files are stored. diff --git a/simulation/create_torrent.hpp b/simulation/create_torrent.hpp index e8785f0db..5785a2c37 100644 --- a/simulation/create_torrent.hpp +++ b/simulation/create_torrent.hpp @@ -30,8 +30,8 @@ POSSIBILITY OF SUCH DAMAGE. */ -#ifndef TORRENT_CREATE_TORRENT_HPP_INCLUDED -#define TORRENT_CREATE_TORRENT_HPP_INCLUDED +#ifndef TORRENT_SIM_CREATE_TORRENT_HPP_INCLUDED +#define TORRENT_SIM_CREATE_TORRENT_HPP_INCLUDED #include #include "libtorrent/add_torrent_params.hpp" diff --git a/simulation/test_checking.cpp b/simulation/test_checking.cpp index c5d8426d7..7aecd44a3 100644 --- a/simulation/test_checking.cpp +++ b/simulation/test_checking.cpp @@ -35,11 +35,14 @@ POSSIBILITY OF SUCH DAMAGE. #include "libtorrent/address.hpp" #include "libtorrent/torrent_status.hpp" #include "libtorrent/torrent_info.hpp" +#include "libtorrent/file.hpp" +#include "libtorrent/create_torrent.hpp" #include "simulator/simulator.hpp" #include "simulator/utils.hpp" #include "test.hpp" #include "settings.hpp" +#include "setup_transfer.hpp" // for create_random_files #include "create_torrent.hpp" #include "utils.hpp" @@ -101,6 +104,97 @@ TORRENT_TEST(no_truncate_checking) TEST_EQUAL(f.tellg(), std::fstream::pos_type(size)); } +boost::shared_ptr create_multifile_torrent() +{ + // the two first files are exactly the size of a piece + static const int file_sizes[] = { 0x40000, 0x40000, 4300, 0, 400, 4300, 6, 4}; + const int num_files = sizeof(file_sizes)/sizeof(file_sizes[0]); + + lt::file_storage fs; + create_random_files("test_torrent_dir", file_sizes, num_files, &fs); + lt::create_torrent t(fs, 0x40000, 0x4000); + + // calculate the hash for all pieces + lt::error_code ec; + set_piece_hashes(t, "."); + + std::vector buf; + lt::bencode(std::back_inserter(buf), t.generate()); + return boost::make_shared(&buf[0], buf.size()); +} + +TORRENT_TEST(aligned_zero_priority) +{ + run_test( + [&](lt::add_torrent_params& atp, lt::settings_pack& p) { + atp.file_priorities.push_back(1); + atp.file_priorities.push_back(0); + atp.ti = create_multifile_torrent(); + atp.save_path = "."; + }, + [](lt::session& ses) { + std::vector tor = ses.get_torrents(); + TEST_EQUAL(tor.size(), 1); + TEST_EQUAL(tor[0].status().is_finished, true); + } + ); +} + +// we have a zero-priority file that also does not exist on disk. It does not +// overlap any piece in another file, so we don't need a partfile +TORRENT_TEST(aligned_zero_priority_no_file) +{ + std::string partfile; + run_test( + [&](lt::add_torrent_params& atp, lt::settings_pack& p) { + atp.ti = create_multifile_torrent(); + atp.save_path = "."; + atp.file_priorities.push_back(1); + atp.file_priorities.push_back(0); + std::string filename = lt::current_working_directory() + "/" + atp.save_path + "/" + atp.ti->files().file_path(1); + partfile = lt::current_working_directory() + "/" + atp.save_path + "/." + lt::to_hex(atp.ti->info_hash().to_string()) + ".parts"; + lt::error_code ec; + lt::remove(filename, ec); + }, + [](lt::session& ses) { + std::vector tor = ses.get_torrents(); + TEST_EQUAL(tor.size(), 1); + TEST_EQUAL(tor[0].status().is_finished, true); + } + ); + + // the part file should not have been created. There is no need for a + // partfile + lt::error_code ec; + lt::file_status fs; + stat_file(partfile, &fs, ec); + TEST_EQUAL(ec, boost::system::errc::no_such_file_or_directory); +} + +// we have a file whose priority is 0, we don't have the file on disk nor a +// part-file for it. The checking should complete and enter download state. +TORRENT_TEST(zero_priority_missing_partfile) +{ + boost::shared_ptr ti = create_multifile_torrent(); + run_test( + [&](lt::add_torrent_params& atp, lt::settings_pack& p) { + atp.ti = ti; + atp.save_path = "."; + atp.file_priorities.push_back(1); + atp.file_priorities.push_back(1); + atp.file_priorities.push_back(0); + std::string filename = lt::current_working_directory() + "/" + atp.save_path + "/" + atp.ti->files().file_path(2); + lt::error_code ec; + lt::remove(filename, ec); + }, + [&](lt::session& ses) { + std::vector tor = ses.get_torrents(); + TEST_EQUAL(tor.size(), 1); + TEST_EQUAL(tor[0].status().num_pieces, ti->num_pieces() - 1); + } + ); +} + TORRENT_TEST(cache_after_checking) { run_test( diff --git a/src/create_torrent.cpp b/src/create_torrent.cpp index 1858c4f6d..5363eefc7 100644 --- a/src/create_torrent.cpp +++ b/src/create_torrent.cpp @@ -249,7 +249,13 @@ namespace libtorrent , boost::function const& f, error_code& ec) { // optimized path +#ifdef TORRENT_BUILD_SIMULATOR + sim::default_config conf; + sim::simulation sim{conf}; + io_service ios{sim}; +#else io_service ios; +#endif #if TORRENT_USE_UNC_PATHS std::string path = canonicalize_path(p); @@ -273,7 +279,11 @@ namespace libtorrent boost::shared_ptr dummy; counters cnt; disk_io_thread disk_thread(ios, cnt, 0); +#ifdef TORRENT_BUILD_SIMULATOR + disk_thread.set_num_threads(0); +#else disk_thread.set_num_threads(1); +#endif storage_params params; params.files = &t.files(); @@ -309,7 +319,13 @@ namespace libtorrent if (piece_counter >= t.num_pieces()) break; } disk_thread.submit_jobs(); + +#ifdef TORRENT_BUILD_SIMULATOR + sim.run(); +#else ios.run(ec); +#endif + disk_thread.abort(true); } create_torrent::~create_torrent() {} diff --git a/src/storage.cpp b/src/storage.cpp index 53f733a4b..7d9034b8a 100644 --- a/src/storage.cpp +++ b/src/storage.cpp @@ -244,7 +244,7 @@ namespace libtorrent if (file_index < int(m_storage.m_file_priority.size()) && m_storage.m_file_priority[file_index] == 0 - && m_storage.m_use_part_file) + && m_storage.use_partfile(file_index)) { TORRENT_ASSERT(m_storage.m_part_file); @@ -335,7 +335,7 @@ namespace libtorrent if (file_index < int(m_storage.m_file_priority.size()) && m_storage.m_file_priority[file_index] == 0 - && m_storage.m_use_part_file) + && m_storage.use_partfile(file_index)) { TORRENT_ASSERT(m_storage.m_part_file); @@ -403,7 +403,6 @@ namespace libtorrent default_storage::default_storage(storage_params const& params) : m_files(*params.files) - , m_use_part_file(true) , m_pool(*params.pool) , m_allocate_files(params.mode == storage_mode_allocate) { @@ -417,12 +416,27 @@ namespace libtorrent : params.files->name()) + ".parts"; file_storage const& fs = files(); + + // if some files have priority 0, we need to check if they exist on the + // filesystem, in which case we won't use a partfile for them. + // this is to be backwards compatible with previous versions of + // libtorrent, when part files were not supported. for (int i = 0; i < m_file_priority.size(); ++i) { - if (m_file_priority[i] == 0 && !fs.pad_file_at(i)) + if (m_file_priority[i] != 0 || fs.pad_file_at(i)) + continue; + + file_status s; + std::string const file_path = files().file_path(i, m_save_path); + error_code ec; + stat_file(file_path, &s, ec); + if (!ec) + { + use_partfile(i, false); + } + else { need_partfile(); - break; } } } @@ -456,23 +470,32 @@ namespace libtorrent file_storage const& fs = files(); for (int i = 0; i < int(prio.size()); ++i) { + // pad files always have priority 0. + if (fs.pad_file_at(i)) continue; + int old_prio = m_file_priority[i]; int new_prio = prio[i]; if (old_prio == 0 && new_prio != 0) { // move stuff out of the part file file_handle f = open_file(i, file::read_write, ec); - if (ec) return; - - need_partfile(); - - m_part_file->export_file(*f, fs.file_offset(i), fs.file_size(i), ec.ec); if (ec) { ec.file = i; - ec.operation = storage_error::partfile_write; + ec.operation = storage_error::open; return; } + + if (m_part_file) + { + m_part_file->export_file(*f, fs.file_offset(i), fs.file_size(i), ec.ec); + if (ec) + { + ec.file = i; + ec.operation = storage_error::partfile_write; + return; + } + } } else if (old_prio != 0 && new_prio == 0) { @@ -512,17 +535,32 @@ namespace libtorrent ec.ec.clear(); m_file_priority[i] = new_prio; - if (m_file_priority[i] == 0 && !fs.pad_file_at(i)) + if (m_file_priority[i] == 0 && use_partfile(i)) + { need_partfile(); + } } if (m_part_file) m_part_file->flush_metadata(ec.ec); if (ec) { - ec.file = -1; + ec.file = torrent_status::error_file_partfile; ec.operation = storage_error::partfile_write; } } + bool default_storage::use_partfile(int const index) + { + TORRENT_ASSERT_VAL(index >= 0, index); + if (index >= int(m_use_partfile.size())) return true; + return m_use_partfile[index]; + } + + void default_storage::use_partfile(int const index, bool const b) + { + if (index >= int(m_use_partfile.size())) m_use_partfile.resize(index + 1, true); + m_use_partfile[index] = b; + } + void default_storage::initialize(storage_error& ec) { m_stat_cache.init(files().num_files()); @@ -680,7 +718,7 @@ namespace libtorrent ec.ec.clear(); if (ec) { - ec.file = -1; + ec.file = torrent_status::error_file_partfile; ec.operation = storage_error::stat; return false; } @@ -862,7 +900,7 @@ namespace libtorrent , m_save_path.c_str(), m_part_file_name.c_str(), error.message().c_str()); if (error && error != boost::system::errc::no_such_file_or_directory) { - ec.file = -1; + ec.file = torrent_status::error_file_partfile; ec.ec = error; ec.operation = storage_error::remove; } @@ -1038,21 +1076,6 @@ namespace libtorrent return false; } - // if some files have priority 0, we would expect therer to be a part file - // as well. If there isn't, it implies we're not using part files and the - // original file names are used to store the partial pieces. i.e. 1.0.x - // behavior - for (int i = 0; i < m_file_priority.size(); ++i) - { - if (m_file_priority[i] == 0 && !fs.pad_file_at(i)) - { - file_status s; - stat_file(combine_path(m_save_path, m_part_file_name), &s, ec.ec); - m_use_part_file = !ec; - break; - } - } - for (int i = 0; i < file_sizes_ent.list_size(); ++i) { if (fs.pad_file_at(i)) continue; @@ -1065,7 +1088,7 @@ namespace libtorrent // files they belong in. if (i < int(m_file_priority.size()) && m_file_priority[i] == 0 - && m_use_part_file) + && use_partfile(i)) continue; bdecode_node e = file_sizes_ent.list_at(i); @@ -1323,7 +1346,7 @@ namespace libtorrent if (e) { ec.ec = e; - ec.file = -1; + ec.file = torrent_status::error_file_partfile; ec.operation = storage_error::partfile_move; } } diff --git a/src/torrent.cpp b/src/torrent.cpp index 804ba0072..f853b2c4e 100644 --- a/src/torrent.cpp +++ b/src/torrent.cpp @@ -9508,10 +9508,14 @@ namespace { } std::string torrent::resolve_filename(int file) const { - if (file == torrent_status::error_file_none) return ""; - if (file == torrent_status::error_file_url) return m_url; - if (file == torrent_status::error_file_ssl_ctx) return "SSL Context"; - if (file == torrent_status::error_file_metadata) return "metadata (from user load function)"; + switch (file) + { + case torrent_status::error_file_none: return ""; + case torrent_status::error_file_url: return m_url; + case torrent_status::error_file_ssl_ctx: return "SSL Context"; + case torrent_status::error_file_metadata: return "metadata (from user load function)"; + case torrent_status::error_file_partfile: return "partfile"; + } if (m_storage && file >= 0) { diff --git a/test/setup_transfer.cpp b/test/setup_transfer.cpp index 6b5dd816b..b599e7e41 100644 --- a/test/setup_transfer.cpp +++ b/test/setup_transfer.cpp @@ -597,7 +597,10 @@ void create_random_files(std::string const& path, const int file_sizes[], int nu std::string full_path = combine_path(path, dirname); error_code ec; - create_directory(full_path, ec); + lt::create_directories(full_path, ec); + if (ec) fprintf(stderr, "create_directory(%s) failed: (%d) %s\n" + , full_path.c_str(), ec.value(), ec.message().c_str()); + full_path = combine_path(full_path, filename); int to_write = file_sizes[i];