diff --git a/ChangeLog b/ChangeLog index 5a17e743b..18397dbf4 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,4 @@ + * fix issue of rapid calls to file_priority() clobbering each other * clear tracker errors on success * optimize setting with unlimited unchoke slots * fixed restoring of trackers, comment, creation date and created-by in resume data diff --git a/include/libtorrent/torrent.hpp b/include/libtorrent/torrent.hpp index 8a22bf7b8..c1aa3ea7e 100644 --- a/include/libtorrent/torrent.hpp +++ b/include/libtorrent/torrent.hpp @@ -575,7 +575,7 @@ namespace libtorrent { download_priority_t file_priority(file_index_t index) const; void on_file_priority(storage_error const& err, aux::vector prios); - void prioritize_files(aux::vector const& files); + void prioritize_files(aux::vector files); void file_priorities(aux::vector*) const; void cancel_non_critical(); @@ -1273,6 +1273,12 @@ namespace libtorrent { // TODO: this wastes 5 bits per file aux::vector m_file_priority; + // any file priority updateds attempted while another file priority update + // in in-progress/outstanding with the disk I/O thread, are queued up in + // this dictionary. Once the outstanding update comes back, all of these + // are applied in one batch + std::map m_deferred_file_priorities; + // this object is used to track download progress of individual files aux::file_progress m_file_progress; @@ -1704,6 +1710,9 @@ namespace libtorrent { // the torrent error bool m_torrent_initialized:1; + // this is set to true while waiting for an async_set_file_priority + bool m_outstanding_file_priority:1; + #if TORRENT_USE_ASSERTS // set to true when torrent is start()ed. It may only be started once bool m_was_started = false; diff --git a/src/torrent.cpp b/src/torrent.cpp index eabd52d10..a4e0f77ab 100644 --- a/src/torrent.cpp +++ b/src/torrent.cpp @@ -227,6 +227,7 @@ bool is_downloading_state(int const st) , m_downloaded(0xffffff) , m_progress_ppm(0) , m_torrent_initialized(false) + , m_outstanding_file_priority(false) { // we cannot log in the constructor, because it relies on shared_from_this // being initialized, which happens after the constructor returns. @@ -4963,29 +4964,28 @@ bool is_downloading_state(int const st) namespace { aux::vector fix_priorities( - aux::vector const& input + aux::vector input , file_storage const* fs) { - aux::vector files(input.begin(), input.end()); + if (fs) input.resize(fs->num_files(), default_priority); - if (fs) files.resize(fs->num_files(), default_priority); - - for (file_index_t i : files.range()) + for (file_index_t i : input.range()) { // initialize pad files to priority 0 - if (files[i] > dont_download && fs && fs->pad_file_at(i)) - files[i] = dont_download; - else if (files[i] > top_priority) - files[i] = top_priority; + if (input[i] > dont_download && fs && fs->pad_file_at(i)) + input[i] = dont_download; + else if (input[i] > top_priority) + input[i] = top_priority; } - return files; + return input; } } void torrent::on_file_priority(storage_error const& err , aux::vector prios) { + m_outstanding_file_priority = false; COMPLETE_ASYNC("file_priority"); if (m_file_priority != prios) { @@ -4994,23 +4994,43 @@ bool is_downloading_state(int const st) recalc_share_mode(); } - if (!err) return; + if (err) + { + // in this case, some file priorities failed to get set + if (alerts().should_post()) + alerts().emplace_alert(err.ec + , resolve_filename(err.file()), err.operation, get_handle()); - // in this case, some file priorities failed to get set - - if (alerts().should_post()) - alerts().emplace_alert(err.ec - , resolve_filename(err.file()), err.operation, get_handle()); - - set_error(err.ec, err.file()); - pause(); + set_error(err.ec, err.file()); + pause(); + } + else if (!m_deferred_file_priorities.empty() && !m_abort) + { + auto new_priority = m_file_priority; + // resize the vector if we have to. The last item in the map has the + // highest file index. + auto const max_idx = std::prev(m_deferred_file_priorities.end())->first; + if (new_priority.end_index() <= max_idx) + { + // any unallocated slot is assumed to have the default priority + new_priority.resize(static_cast(max_idx) + 1, default_priority); + } + for (auto const& p : m_deferred_file_priorities) + { + file_index_t const index = p.first; + download_priority_t const prio = p.second; + new_priority[index] = prio; + } + m_deferred_file_priorities.clear(); + prioritize_files(std::move(new_priority)); + } } - void torrent::prioritize_files(aux::vector const& files) + void torrent::prioritize_files(aux::vector files) { INVARIANT_CHECK; - auto new_priority = fix_priorities(files + auto new_priority = fix_priorities(std::move(files) , valid_metadata() ? &m_torrent_file->files() : nullptr); // storage may be NULL during shutdown @@ -5023,6 +5043,7 @@ bool is_downloading_state(int const st) // possibly not fully updated. update_piece_priorities(new_priority); + m_outstanding_file_priority = true; ADD_OUTSTANDING_ASYNC("file_priority"); m_ses.disk_thread().async_set_file_priority(m_storage , std::move(new_priority), std::bind(&torrent::on_file_priority, shared_from_this(), _1, _2)); @@ -5048,6 +5069,13 @@ bool is_downloading_state(int const st) } prio = aux::clamp(prio, dont_download, top_priority); + + if (m_outstanding_file_priority) + { + m_deferred_file_priorities[index] = prio; + return; + } + auto new_priority = m_file_priority; if (new_priority.end_index() <= index) { @@ -5066,6 +5094,7 @@ bool is_downloading_state(int const st) // piece priorities still stay the same, but the file priorities are // possibly not fully updated. update_piece_priorities(new_priority); + m_outstanding_file_priority = true; ADD_OUTSTANDING_ASYNC("file_priority"); m_ses.disk_thread().async_set_file_priority(m_storage , std::move(new_priority), std::bind(&torrent::on_file_priority, shared_from_this(), _1, _2)); diff --git a/src/torrent_handle.cpp b/src/torrent_handle.cpp index 4f29e9bb6..70d9b7ed6 100644 --- a/src/torrent_handle.cpp +++ b/src/torrent_handle.cpp @@ -520,6 +520,7 @@ namespace libtorrent { return sync_call_ret(dont_download, &torrent::file_priority, index); } + // TODO: support moving files into this call void torrent_handle::prioritize_files(std::vector const& files) const { async_call(&torrent::prioritize_files diff --git a/test/test_priority.cpp b/test/test_priority.cpp index 4bc19008e..4d5098578 100644 --- a/test/test_priority.cpp +++ b/test/test_priority.cpp @@ -484,6 +484,35 @@ TORRENT_TEST(no_metadata_piece_prio) ses.remove_torrent(h); } +TORRENT_TEST(file_priority_multiple_calls) +{ + settings_pack pack = settings(); + lt::session ses(pack); + + error_code ec; + auto t = ::generate_torrent(true); + + add_torrent_params addp; + addp.flags &= ~torrent_flags::paused; + addp.flags &= ~torrent_flags::auto_managed; + addp.save_path = "."; + addp.ti = t; + torrent_handle h = ses.add_torrent(addp); + + for (file_index_t const i : t->files().file_range()) + h.file_priority(i, lt::low_priority); + + std::vector const expected( + std::size_t(t->files().num_files()), lt::low_priority); + for (int i = 0; i < 10; ++i) + { + auto const p = h.get_file_priorities(); + if (p == expected) return; + std::this_thread::sleep_for(milliseconds(500)); + } + TEST_CHECK(false); +} + TORRENT_TEST(export_file_while_seed) { settings_pack pack = settings(); diff --git a/test/test_torrent.cpp b/test/test_torrent.cpp index 1d0d4e779..a1e866840 100644 --- a/test/test_torrent.cpp +++ b/test/test_torrent.cpp @@ -254,7 +254,6 @@ TORRENT_TEST(total_wanted) // the last set priority h.file_priority(file_index_t{1}, default_priority); h.file_priority(file_index_t{1}, dont_download); - TEST_EQUAL(h.status({}).total_wanted, 0); TEST_CHECK(wait_priority(h, aux::vector(static_cast(fs.num_files())))); TEST_EQUAL(h.status({}).total_wanted, 0); }