From ae7058e1198923b85351f4ddf40f3f8e25c71560 Mon Sep 17 00:00:00 2001 From: arvidn Date: Sun, 6 Dec 2015 12:45:20 -0500 Subject: [PATCH] remove the timestamps and file sizes from the resume data. This makes saving resume data alot cheaper, since it doesn't have to go via the disk thread. It also removes an old-standing API usage issue where there was easily a race condition introduced between saving resume data and pausing a torrent. --- ChangeLog | 1 + include/libtorrent/alert_manager.hpp | 7 +- include/libtorrent/aux_/session_impl.hpp | 16 - include/libtorrent/aux_/session_interface.hpp | 2 - include/libtorrent/disk_interface.hpp | 2 - include/libtorrent/disk_io_job.hpp | 3 - include/libtorrent/disk_io_thread.hpp | 3 - include/libtorrent/settings_pack.hpp | 5 + include/libtorrent/stat_cache.hpp | 2 +- include/libtorrent/storage.hpp | 16 - include/libtorrent/torrent.hpp | 2 - include/libtorrent/torrent_info.hpp | 1 + src/alert_manager.cpp | 18 +- src/block_cache.cpp | 1 - src/disk_buffer_holder.cpp | 2 - src/disk_io_job.cpp | 2 - src/disk_io_thread.cpp | 34 -- src/session_impl.cpp | 68 +--- src/stat_cache.cpp | 4 + src/storage.cpp | 302 ++++-------------- src/torrent.cpp | 120 ++----- src/torrent_handle.cpp | 13 - test/test_alert_manager.cpp | 48 +-- test/test_storage.cpp | 4 +- 24 files changed, 106 insertions(+), 570 deletions(-) diff --git a/ChangeLog b/ChangeLog index 2ee6d0665..d80308ace 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,4 @@ + * resume data no longer has timestamps of files 1.1.0 release diff --git a/include/libtorrent/alert_manager.hpp b/include/libtorrent/alert_manager.hpp index 41e883a41..e90cecd55 100644 --- a/include/libtorrent/alert_manager.hpp +++ b/include/libtorrent/alert_manager.hpp @@ -110,7 +110,7 @@ namespace libtorrent { #endif bool pending() const; - void get_all(std::vector& alerts, int& num_resume); + void get_all(std::vector& alerts); template bool should_post() const @@ -147,8 +147,6 @@ namespace libtorrent { void set_dispatch_function(boost::function)> const&); #endif - int num_queued_resume() const; - #ifndef TORRENT_DISABLE_EXTENSIONS void add_extension(boost::shared_ptr ext); #endif @@ -179,9 +177,6 @@ namespace libtorrent { // posted to the queue boost::function m_notify; - // the number of resume data alerts in the alert queue - int m_num_queued_resume; - // this is either 0 or 1, it indicates which m_alerts and m_allocations // the alert_manager is allowed to use right now. This is swapped when // the client calls get_all(), at which point all of the alert objects diff --git a/include/libtorrent/aux_/session_impl.hpp b/include/libtorrent/aux_/session_impl.hpp index 84a775107..2a4e2fcd6 100644 --- a/include/libtorrent/aux_/session_impl.hpp +++ b/include/libtorrent/aux_/session_impl.hpp @@ -781,16 +781,6 @@ namespace libtorrent std::map > m_uuids; - // when saving resume data for many torrents, torrents are - // queued up in this list in order to not have too many of them - // outstanding at any given time, since the resume data may use - // a lot of memory. - std::list > m_save_resume_queue; - - // the number of save resume data disk jobs that are currently - // outstanding - int m_num_save_resume; - // peer connections are put here when disconnected to avoid // race conditions with the disk thread. It's important that // peer connections are destructed from the network thread, @@ -1162,12 +1152,6 @@ namespace libtorrent std::list > m_tracker_loggers; #endif - // TODO: 2 the throttling of saving resume data could probably be - // factored out into a separate class - virtual void queue_async_resume_data(boost::shared_ptr const& t) TORRENT_OVERRIDE; - virtual void done_async_resume() TORRENT_OVERRIDE; - void async_resume_dispatched(); - // state for keeping track of external IPs external_ip m_external_ip; diff --git a/include/libtorrent/aux_/session_interface.hpp b/include/libtorrent/aux_/session_interface.hpp index a46b825e6..74b8b180c 100644 --- a/include/libtorrent/aux_/session_interface.hpp +++ b/include/libtorrent/aux_/session_interface.hpp @@ -166,8 +166,6 @@ namespace libtorrent { namespace aux virtual bool has_connection(peer_connection* p) const = 0; virtual void insert_peer(boost::shared_ptr const& c) = 0; - virtual void queue_async_resume_data(boost::shared_ptr const& t) = 0; - virtual void done_async_resume() = 0; virtual void evict_torrent(torrent* t) = 0; virtual void remove_torrent(torrent_handle const& h, int options = 0) = 0; diff --git a/include/libtorrent/disk_interface.hpp b/include/libtorrent/disk_interface.hpp index eb6cd8f51..35b8b4075 100644 --- a/include/libtorrent/disk_interface.hpp +++ b/include/libtorrent/disk_interface.hpp @@ -86,8 +86,6 @@ namespace libtorrent , boost::function const& handler) = 0; virtual void async_delete_files(piece_manager* storage , boost::function const& handler) = 0; - virtual void async_save_resume_data(piece_manager* storage - , boost::function const& handler) = 0; virtual void async_set_file_priority(piece_manager* storage , std::vector const& prio , boost::function const& handler) = 0; diff --git a/include/libtorrent/disk_io_job.hpp b/include/libtorrent/disk_io_job.hpp index fdb693f2c..f5d7b6811 100644 --- a/include/libtorrent/disk_io_job.hpp +++ b/include/libtorrent/disk_io_job.hpp @@ -89,7 +89,6 @@ namespace libtorrent , release_files , delete_files , check_fastresume - , save_resume_data , rename_file , stop_torrent , cache_piece @@ -151,8 +150,6 @@ namespace libtorrent // for other jobs, it may point to other job-specific types // for move_storage and rename_file this is a string allocated // with malloc() - // an entry* for save_resume_data - // for aiocb_complete this points to the aiocb that completed // for get_cache_info this points to a cache_status object which // is filled in union diff --git a/include/libtorrent/disk_io_thread.hpp b/include/libtorrent/disk_io_thread.hpp index 2bd64d2d8..ad5cdd789 100644 --- a/include/libtorrent/disk_io_thread.hpp +++ b/include/libtorrent/disk_io_thread.hpp @@ -317,8 +317,6 @@ namespace libtorrent , bdecode_node const* resume_data , std::vector& links , boost::function const& handler); - void async_save_resume_data(piece_manager* storage - , boost::function const& handler); void async_rename_file(piece_manager* storage, int index, std::string const& name , boost::function const& handler); void async_stop_torrent(piece_manager* storage @@ -414,7 +412,6 @@ namespace libtorrent int do_release_files(disk_io_job* j, jobqueue_t& completed_jobs); int do_delete_files(disk_io_job* j, jobqueue_t& completed_jobs); int do_check_fastresume(disk_io_job* j, jobqueue_t& completed_jobs); - int do_save_resume_data(disk_io_job* j, jobqueue_t& completed_jobs); int do_rename_file(disk_io_job* j, jobqueue_t& completed_jobs); int do_stop_torrent(disk_io_job* j, jobqueue_t& completed_jobs); int do_read_and_hash(disk_io_job* j, jobqueue_t& completed_jobs); diff --git a/include/libtorrent/settings_pack.hpp b/include/libtorrent/settings_pack.hpp index d6aae949d..9dc94204d 100644 --- a/include/libtorrent/settings_pack.hpp +++ b/include/libtorrent/settings_pack.hpp @@ -437,6 +437,7 @@ namespace libtorrent enable_outgoing_tcp, enable_incoming_tcp, +#ifndef TORRENT_NO_DEPRECATE // ``ignore_resume_timestamps`` determines if the storage, when // loading resume data files, should verify that the file modification // time with the timestamps in the resume data. This defaults to @@ -447,6 +448,10 @@ namespace libtorrent // redownload potentially missed pieces than to go through the whole // storage to look for them. ignore_resume_timestamps, +#else + // hidden + deprecated8, +#endif // ``no_recheck_incomplete_resume`` determines if the storage should // check the whole files when resume data is incomplete or missing or diff --git a/include/libtorrent/stat_cache.hpp b/include/libtorrent/stat_cache.hpp index e260e4dd5..a7d898b62 100644 --- a/include/libtorrent/stat_cache.hpp +++ b/include/libtorrent/stat_cache.hpp @@ -51,7 +51,7 @@ namespace libtorrent ~stat_cache(); void init(int num_files); - + enum { cache_error = -1, diff --git a/include/libtorrent/storage.hpp b/include/libtorrent/storage.hpp index ccd8e94aa..447179041 100644 --- a/include/libtorrent/storage.hpp +++ b/include/libtorrent/storage.hpp @@ -113,7 +113,6 @@ POSSIBILITY OF SUCH DAMAGE. // virtual bool verify_resume_data(bdecode_node const& rd // , std::vector const* links // , storage_error& error) { return false; } -// virtual bool write_resume_data(entry& rd) const { return false; } // virtual boost::int64_t physical_offset(int piece, int offset) // { return piece * m_files.piece_length() + offset; }; // virtual sha1_hash hash_for_slot(int piece, partial_hash& ph, int piece_size) @@ -314,16 +313,6 @@ namespace libtorrent , std::vector const* links , storage_error& ec) = 0; - // This function should fill in resume data, the current state of the - // storage, in ``rd``. The default storage adds file timestamps and - // sizes. - // - // Returning ``true`` indicates an error occurred. - // - // If an error occurs, ``storage_error`` should be set to reflect it. - // - virtual void write_resume_data(entry& rd, storage_error& ec) const = 0; - // This function should release all the file handles that it keeps open // to files belonging to this storage. The default implementation just // calls file_pool::release_files(). @@ -433,7 +422,6 @@ namespace libtorrent virtual bool verify_resume_data(bdecode_node const& rd , std::vector const* links , storage_error& error) TORRENT_OVERRIDE; - virtual void write_resume_data(entry& rd, storage_error& ec) const TORRENT_OVERRIDE; virtual bool tick() TORRENT_OVERRIDE; int readv(file::iovec_t const* bufs, int num_bufs @@ -518,7 +506,6 @@ namespace libtorrent virtual bool verify_resume_data(bdecode_node const& , std::vector const* , storage_error&) TORRENT_OVERRIDE { return false; } - virtual void write_resume_data(entry&, storage_error&) const TORRENT_OVERRIDE {} }; // this storage implementation always reads zeroes, and always discards @@ -541,7 +528,6 @@ namespace libtorrent , std::vector const* /* links */ , storage_error&) TORRENT_OVERRIDE { return false; } - virtual void write_resume_data(entry&, storage_error&) const TORRENT_OVERRIDE {} virtual void release_files(storage_error&) TORRENT_OVERRIDE {} virtual void rename_file(int /* index */ , std::string const& /* new_filenamem */, storage_error&) TORRENT_OVERRIDE {} @@ -661,8 +647,6 @@ namespace libtorrent storage_interface* get_storage_impl() { return m_storage.get(); } - void write_resume_data(entry& rd, storage_error& ec) const; - #ifdef TORRENT_DEBUG void assert_torrent_refcount() const; #endif diff --git a/include/libtorrent/torrent.hpp b/include/libtorrent/torrent.hpp index 2c9639363..38f6a552e 100644 --- a/include/libtorrent/torrent.hpp +++ b/include/libtorrent/torrent.hpp @@ -480,7 +480,6 @@ namespace libtorrent bool is_torrent_paused() const { return !m_allow_peers || m_graceful_pause_mode; } void force_recheck(); void save_resume_data(int flags); - bool do_async_save_resume_data(); bool need_save_resume_data() const { @@ -1138,7 +1137,6 @@ namespace libtorrent void on_files_deleted(disk_io_job const* j); void on_torrent_paused(disk_io_job const* j); void on_storage_moved(disk_io_job const* j); - void on_save_resume_data(disk_io_job const* j); void on_file_renamed(disk_io_job const* j); void on_cache_flushed(disk_io_job const* j); diff --git a/include/libtorrent/torrent_info.hpp b/include/libtorrent/torrent_info.hpp index 4e4fa5ab9..9a8de81cc 100644 --- a/include/libtorrent/torrent_info.hpp +++ b/include/libtorrent/torrent_info.hpp @@ -219,6 +219,7 @@ namespace libtorrent void rename_file(int index, std::string const& new_filename) { TORRENT_ASSERT(is_loaded()); + if (m_files.file_name(index) == new_filename) return; copy_on_write(); m_files.rename_file(index, new_filename); } diff --git a/src/alert_manager.cpp b/src/alert_manager.cpp index ddb661986..77473aa5e 100644 --- a/src/alert_manager.cpp +++ b/src/alert_manager.cpp @@ -44,18 +44,11 @@ namespace libtorrent alert_manager::alert_manager(int queue_limit, boost::uint32_t alert_mask) : m_alert_mask(alert_mask) , m_queue_size_limit(queue_limit) - , m_num_queued_resume(0) , m_generation(0) {} alert_manager::~alert_manager() {} - int alert_manager::num_queued_resume() const - { - mutex::scoped_lock lock(m_mutex); - return m_num_queued_resume; - } - alert* alert_manager::wait_for_alert(time_duration max_wait) { mutex::scoped_lock lock(m_mutex); @@ -73,10 +66,6 @@ namespace libtorrent void alert_manager::maybe_notify(alert* a, mutex::scoped_lock& lock) { - if (a->type() == save_resume_data_failed_alert::alert_type - || a->type() == save_resume_data_alert::alert_type) - ++m_num_queued_resume; - if (m_alerts[m_generation].size() == 1) { lock.unlock(); @@ -102,6 +91,8 @@ namespace libtorrent { (*i)->on_alert(a); } +#else + TORRENT_UNUSED(a); #endif } @@ -168,17 +159,14 @@ namespace libtorrent } #endif - void alert_manager::get_all(std::vector& alerts, int& num_resume) + void alert_manager::get_all(std::vector& alerts) { mutex::scoped_lock lock(m_mutex); - TORRENT_ASSERT(m_num_queued_resume <= m_alerts[m_generation].size()); alerts.clear(); if (m_alerts[m_generation].empty()) return; m_alerts[m_generation].get_pointers(alerts); - num_resume = m_num_queued_resume; - m_num_queued_resume = 0; // swap buffers m_generation = (m_generation + 1) & 1; diff --git a/src/block_cache.cpp b/src/block_cache.cpp index b51f9a923..1cc7b49dd 100644 --- a/src/block_cache.cpp +++ b/src/block_cache.cpp @@ -214,7 +214,6 @@ const char* const job_action_name[] = "release_files", "delete_files", "check_fastresume", - "save_resume_data", "rename_file", "stop_torrent", "cache_piece", diff --git a/src/disk_buffer_holder.cpp b/src/disk_buffer_holder.cpp index 83e784369..497e22897 100644 --- a/src/disk_buffer_holder.cpp +++ b/src/disk_buffer_holder.cpp @@ -52,7 +52,6 @@ namespace libtorrent TORRENT_ASSERT(m_ref.storage == 0 || m_ref.block >= 0); TORRENT_ASSERT(m_ref.storage == 0 || m_ref.piece < static_cast(m_ref.storage)->files()->num_pieces()); TORRENT_ASSERT(m_ref.storage == 0 || m_ref.block <= static_cast(m_ref.storage)->files()->piece_length() / 0x4000); - TORRENT_ASSERT(j.action != disk_io_job::save_resume_data); TORRENT_ASSERT(j.action != disk_io_job::rename_file); TORRENT_ASSERT(j.action != disk_io_job::move_storage); } @@ -69,7 +68,6 @@ namespace libtorrent TORRENT_ASSERT(m_ref.block >= 0); TORRENT_ASSERT(m_ref.piece < static_cast(m_ref.storage)->files()->num_pieces()); TORRENT_ASSERT(m_ref.block <= static_cast(m_ref.storage)->files()->piece_length() / 0x4000); - TORRENT_ASSERT(j.action != disk_io_job::save_resume_data); TORRENT_ASSERT(j.action != disk_io_job::rename_file); TORRENT_ASSERT(j.action != disk_io_job::move_storage); } diff --git a/src/disk_io_job.cpp b/src/disk_io_job.cpp index 0b11605a0..d1b737e77 100644 --- a/src/disk_io_job.cpp +++ b/src/disk_io_job.cpp @@ -62,8 +62,6 @@ namespace libtorrent { if (action == rename_file || action == move_storage) free(buffer.string); - else if (action == save_resume_data) - delete static_cast(buffer.resume_data); } bool disk_io_job::completed(cached_piece_entry const* pe, int block_size) diff --git a/src/disk_io_thread.cpp b/src/disk_io_thread.cpp index fce117f05..1e36852e5 100644 --- a/src/disk_io_thread.cpp +++ b/src/disk_io_thread.cpp @@ -1038,7 +1038,6 @@ namespace libtorrent &disk_io_thread::do_release_files, &disk_io_thread::do_delete_files, &disk_io_thread::do_check_fastresume, - &disk_io_thread::do_save_resume_data, &disk_io_thread::do_rename_file, &disk_io_thread::do_stop_torrent, &disk_io_thread::do_cache_piece, @@ -1904,23 +1903,6 @@ namespace libtorrent add_fence_job(storage, j); } - void disk_io_thread::async_save_resume_data(piece_manager* storage - , boost::function const& handler) - { -#ifdef TORRENT_DEBUG - // the caller must increment the torrent refcount before - // issuing an async disk request - storage->assert_torrent_refcount(); -#endif - - disk_io_job* j = allocate_job(disk_io_job::save_resume_data); - j->storage = storage->shared_from_this(); - j->buffer.resume_data = NULL; - j->callback = handler; - - add_fence_job(storage, j); - } - void disk_io_thread::async_rename_file(piece_manager* storage, int index, std::string const& name , boost::function const& handler) { @@ -2601,22 +2583,6 @@ namespace libtorrent return j->storage->check_fastresume(*rd, links.get(), j->error); } - int disk_io_thread::do_save_resume_data(disk_io_job* j, jobqueue_t& completed_jobs) - { - // if this assert fails, something's wrong with the fence logic - TORRENT_ASSERT(j->storage->num_outstanding_jobs() == 1); - - mutex::scoped_lock l(m_cache_mutex); - flush_cache(j->storage.get(), flush_write_cache, completed_jobs, l); - l.unlock(); - - entry* resume_data = new entry(entry::dictionary_t); - j->storage->get_storage_impl()->write_resume_data(*resume_data, j->error); - TORRENT_ASSERT(j->buffer.resume_data == 0); - j->buffer.resume_data = resume_data; - return j->error ? -1 : 0; - } - int disk_io_thread::do_rename_file(disk_io_job* j, jobqueue_t& /* completed_jobs */ ) { // if this assert fails, something's wrong with the fence logic diff --git a/src/session_impl.cpp b/src/session_impl.cpp index 8f5b872a8..208c628d2 100644 --- a/src/session_impl.cpp +++ b/src/session_impl.cpp @@ -379,7 +379,6 @@ namespace aux { , *this #endif ) - , m_num_save_resume(0) , m_work(io_service::work(m_io_service)) , m_max_queue_pos(-1) , m_key(0) @@ -642,58 +641,6 @@ namespace aux { m_host_resolver.async_resolve(host, flags, h); } - void session_impl::queue_async_resume_data(boost::shared_ptr const& t) - { - INVARIANT_CHECK; - - int loaded_limit = m_settings.get_int(settings_pack::active_loaded_limit); - - if (m_num_save_resume + m_alerts.num_queued_resume() >= loaded_limit - && m_user_load_torrent - && loaded_limit > 0) - { - TORRENT_ASSERT(t); - // do loaded torrents first, otherwise they'll just be - // evicted and have to be loaded again - if (t->is_loaded()) - m_save_resume_queue.push_front(t); - else - m_save_resume_queue.push_back(t); - return; - } - - if (t->do_async_save_resume_data()) - ++m_num_save_resume; - } - - // this is called whenever a save_resume_data comes back - // from the disk thread - void session_impl::done_async_resume() - { - TORRENT_ASSERT(m_num_save_resume > 0); - --m_num_save_resume; - } - - // this is called when one or all save resume alerts are - // popped off the alert queue - void session_impl::async_resume_dispatched() - { - INVARIANT_CHECK; - - int num_queued_resume = m_alerts.num_queued_resume(); - - int loaded_limit = m_settings.get_int(settings_pack::active_loaded_limit); - while (!m_save_resume_queue.empty() - && (m_num_save_resume + num_queued_resume < loaded_limit - || loaded_limit == 0)) - { - boost::shared_ptr t = m_save_resume_queue.front(); - m_save_resume_queue.erase(m_save_resume_queue.begin()); - if (t->do_async_save_resume_data()) - ++m_num_save_resume; - } - } - void session_impl::save_state(entry* eptr, boost::uint32_t flags) const { TORRENT_ASSERT(is_single_thread()); @@ -6452,15 +6399,7 @@ retry: void session_impl::pop_alerts(std::vector* alerts) { - int num_resume = 0; - m_alerts.get_all(*alerts, num_resume); - if (num_resume > 0) - { - // we can only issue more resume data jobs from - // the network thread - m_io_service.post(boost::bind(&session_impl::async_resume_dispatched - , this)); - } + m_alerts.get_all(*alerts); } #ifndef TORRENT_NO_DEPRECATE @@ -6892,11 +6831,6 @@ retry: { TORRENT_ASSERT(is_single_thread()); - int loaded_limit = m_settings.get_int(settings_pack::active_loaded_limit); - TORRENT_ASSERT(m_num_save_resume <= loaded_limit); -// if (m_num_save_resume < loaded_limit) -// TORRENT_ASSERT(m_save_resume_queue.empty()); - TORRENT_ASSERT(m_torrents.size() >= m_torrent_lru.size()); if (m_settings.get_int(settings_pack::unchoke_slots_limit) < 0 diff --git a/src/stat_cache.cpp b/src/stat_cache.cpp index 289cbf97e..35845b890 100644 --- a/src/stat_cache.cpp +++ b/src/stat_cache.cpp @@ -38,6 +38,9 @@ namespace libtorrent stat_cache::stat_cache() {} stat_cache::~stat_cache() {} + // TODO: 4 improve this interface by fall back to the actual stat() call internally. Don't + // expose low-level functions to query the cache and set cache state. Also, we should + // probably cache the error_code too void stat_cache::set_cache(int i, boost::int64_t size, time_t time) { TORRENT_ASSERT(i >= 0); @@ -76,6 +79,7 @@ namespace libtorrent return m_stat_cache[i].file_size; } + // TODO: 4 file_time can probably be removed from the cache now time_t stat_cache::get_filetime(int i) const { if (i >= int(m_stat_cache.size())) return not_in_cache; diff --git a/src/storage.cpp b/src/storage.cpp index 2ba8bbc6d..047e521c4 100644 --- a/src/storage.cpp +++ b/src/storage.cpp @@ -831,244 +831,12 @@ namespace libtorrent #endif } - void default_storage::write_resume_data(entry& rd, storage_error& ec) const - { - TORRENT_ASSERT(rd.type() == entry::dictionary_t); - - entry::list_type& fl = rd["file sizes"].list(); - - if (m_part_file) - { - error_code ignore; - const_cast(*m_part_file).flush_metadata(ignore); - } - - file_storage const& fs = files(); - for (int i = 0; i < fs.num_files(); ++i) - { - boost::int64_t file_size = 0; - time_t file_time = 0; - boost::int64_t cache_state = m_stat_cache.get_filesize(i); - if (cache_state != stat_cache::not_in_cache) - { - if (cache_state >= 0) - { - file_size = cache_state; - file_time = m_stat_cache.get_filetime(i); - } - } - else - { - file_status s; - error_code error; - stat_file(fs.file_path(i, m_save_path), &s, error); - if (!error) - { - file_size = s.file_size; - file_time = s.mtime; - } - else if (error == error_code(boost::system::errc::no_such_file_or_directory - , generic_category())) - { - m_stat_cache.set_noexist(i); - } - else - { - ec.ec = error; - ec.file = i; - ec.operation = storage_error::stat; - m_stat_cache.set_error(i); - } - } - - fl.push_back(entry(entry::list_t)); - entry::list_type& p = fl.back().list(); - p.push_back(entry(file_size)); - p.push_back(entry(file_time)); - } - } - bool default_storage::verify_resume_data(bdecode_node const& rd , std::vector const* links , storage_error& ec) { - // TODO: make this more generic to not just work if files have been - // renamed, but also if they have been merged into a single file for instance - // maybe use the same format as .torrent files and reuse some code from torrent_info - bdecode_node mapped_files = rd.dict_find_list("mapped_files"); - if (mapped_files && mapped_files.list_size() == m_files.num_files()) - { - m_mapped_files.reset(new file_storage(m_files)); - for (int i = 0; i < m_files.num_files(); ++i) - { - std::string new_filename = mapped_files.list_string_value_at(i); - if (new_filename.empty()) continue; - m_mapped_files->rename_file(i, new_filename); - } - } - - bdecode_node file_priority = rd.dict_find_list("file_priority"); - if (file_priority && file_priority.list_size() - == files().num_files()) - { - m_file_priority.resize(file_priority.list_size()); - for (int i = 0; i < file_priority.list_size(); ++i) - m_file_priority[i] = boost::uint8_t(file_priority.list_int_value_at(i, 1)); - } - - bdecode_node file_sizes_ent = rd.dict_find_list("file sizes"); - if (file_sizes_ent == 0) - { - ec.ec = errors::missing_file_sizes; - ec.file = -1; - ec.operation = storage_error::check_resume; - return false; - } - - if (file_sizes_ent.list_size() == 0) - { - ec.ec = errors::no_files_in_resume_data; - return false; - } - file_storage const& fs = files(); - if (file_sizes_ent.list_size() != fs.num_files()) - { - ec.ec = errors::mismatching_number_of_files; - ec.file = -1; - ec.operation = storage_error::check_resume; - return false; - } - bool seed = false; - bdecode_node slots = rd.dict_find_list("slots"); - if (slots) - { - if (int(slots.list_size()) == m_files.num_pieces()) - { - seed = true; - for (int i = 0; i < slots.list_size(); ++i) - { - if (slots.list_int_value_at(i, -1) >= 0) continue; - seed = false; - break; - } - } - } - else if (bdecode_node pieces = rd.dict_find_string("pieces")) - { - if (int(pieces.string_length()) == m_files.num_pieces()) - { - seed = true; - char const* p = pieces.string_ptr(); - for (int i = 0; i < pieces.string_length(); ++i) - { - if ((p[i] & 1) == 1) continue; - seed = false; - break; - } - } - } - else - { - ec.ec = errors::missing_pieces; - ec.file = -1; - ec.operation = storage_error::check_resume; - return false; - } - - for (int i = 0; i < file_sizes_ent.list_size(); ++i) - { - if (fs.pad_file_at(i)) continue; - bdecode_node e = file_sizes_ent.list_at(i); - if (e.type() != bdecode_node::list_t - || e.list_size() < 2 - || e.list_at(0).type() != bdecode_node::int_t - || e.list_at(1).type() != bdecode_node::int_t) - { - ec.ec = errors::missing_file_sizes; - ec.file = i; - ec.operation = storage_error::check_resume; - return false; - } - - boost::int64_t expected_size = e.list_int_value_at(0); - time_t expected_time = e.list_int_value_at(1); - - // if we're a seed, the expected size should match - // the actual full size according to the torrent - if (seed && expected_size < fs.file_size(i)) - { - ec.ec = errors::mismatching_file_size; - ec.file = i; - ec.operation = storage_error::check_resume; - return false; - } - - boost::int64_t file_size = m_stat_cache.get_filesize(i); - time_t file_time; - if (file_size >= 0) - { - file_time = m_stat_cache.get_filetime(i); - } - else - { - file_status s; - error_code error; - std::string file_path = fs.file_path(i, m_save_path); - stat_file(file_path, &s, error); - if (error) - { - if (error != boost::system::errc::no_such_file_or_directory) - { - m_stat_cache.set_error(i); - ec.ec = error; - ec.file = i; - ec.operation = storage_error::stat; - return false; - } - m_stat_cache.set_noexist(i); - if (expected_size != 0) - { - ec.ec = errors::mismatching_file_size; - ec.file = i; - ec.operation = storage_error::none; - return false; - } - file_size = 0; - file_time = 0; - } - else - { - file_size = s.file_size; - file_time = s.mtime; - } - } - - if (expected_size > file_size) - { - ec.ec = errors::mismatching_file_size; - ec.file = i; - ec.operation = storage_error::none; - return false; - } - - if (settings().get_bool(settings_pack::ignore_resume_timestamps)) continue; - - // allow some slack, because of FAT volumes - if (expected_time != 0 && - (file_time > expected_time + 5 * 60 || file_time < expected_time - 5)) - { - ec.ec = errors::mismatching_file_timestamp; - ec.file = i; - ec.operation = storage_error::stat; - return false; - } - } - - // TODO: 2 we probably need to do this unconditionally in this function. - // Even if the resume data file appears stale, we need to create these - // hard links, right? #ifndef TORRENT_DISABLE_MUTABLE_TORRENTS if (links) { @@ -1104,6 +872,67 @@ namespace libtorrent } #endif // TORRENT_DISABLE_MUTABLE_TORRENTS + if (rd && rd.type() == bdecode_node::dict_t) + { + bdecode_node pieces = rd.dict_find_string("pieces"); + if (pieces && pieces.type() == bdecode_node::string_t + && int(pieces.string_length()) == fs.num_pieces()) + { + char const* pieces_str = pieces.string_ptr(); + // parse have bitmask. Verify that the files we expect to have + // actually do exist + for (int i = 0; i < fs.num_pieces(); ++i) + { + if ((pieces_str[i] & 1) == 0) continue; + + std::vector f = fs.map_block(i, 0, 1); + TORRENT_ASSERT(!f.empty()); + + const int file_index = f[0].file_index; + boost::int64_t size = m_stat_cache.get_filesize(f[0].file_index); + + if (size == stat_cache::not_in_cache) + { + file_status s; + error_code error; + std::string file_path = fs.file_path(file_index, m_save_path); + stat_file(file_path, &s, error); + size = s.file_size; + if (error) + { + if (error != boost::system::errc::no_such_file_or_directory) + { + m_stat_cache.set_error(i); + ec.ec = error; + ec.file = i; + ec.operation = storage_error::stat; + return false; + } + m_stat_cache.set_noexist(i); + ec.ec = errors::mismatching_file_size; + ec.file = i; + ec.operation = storage_error::stat; + return false; + } + } + if (size < 0) + { + ec.ec = errors::mismatching_file_size; + ec.file = i; + ec.operation = storage_error::check_resume; + return false; + } + + // OK, this file existed, good. Now, skip all remaining pieces in + // this file. We're just sanity-checking whether the files exist + // or not. + peer_request pr = fs.map_file(file_index, 0 + , fs.file_size(file_index) + 1); + i = (std::max)(i + 1, pr.piece); + } + } + } + return true; } @@ -1597,12 +1426,6 @@ namespace libtorrent } #endif - // used in torrent_handle.cpp - void piece_manager::write_resume_data(entry& rd, storage_error& ec) const - { - m_storage->write_resume_data(rd, ec); - } - int piece_manager::check_no_fastresume(storage_error& ec) { bool has_files = false; @@ -1631,13 +1454,14 @@ namespace libtorrent int piece_manager::check_init_storage(storage_error& ec) { storage_error se; + // initialize may clear the error we pass in and it's important to + // preserve the error code in ec, even when initialize() is successful m_storage->initialize(se); if (se) { ec = se; return fatal_disk_error; } - return no_error; } diff --git a/src/torrent.cpp b/src/torrent.cpp index 17c3c5fdd..caa8b8650 100644 --- a/src/torrent.cpp +++ b/src/torrent.cpp @@ -5036,35 +5036,6 @@ namespace libtorrent } } - void torrent::on_save_resume_data(disk_io_job const* j) - { - TORRENT_ASSERT(is_single_thread()); - torrent_ref_holder h(this, "save_resume"); - dec_refcount("save_resume"); - m_ses.done_async_resume(); - - if (!j->buffer.resume_data) - { - alerts().emplace_alert(get_handle(), j->error.ec); - return; - } - - if (!need_loaded()) - { - alerts().emplace_alert(get_handle() - , m_error); - return; - } - - m_need_save_resume_data = false; - m_last_saved_resume = m_ses.session_time(); - write_resume_data(*j->buffer.resume_data); - alerts().emplace_alert( - boost::shared_ptr(j->buffer.resume_data), get_handle()); - const_cast(j)->buffer.resume_data = 0; - state_updated(); - } - void torrent::on_file_renamed(disk_io_job const* j) { TORRENT_ASSERT(is_single_thread()); @@ -5521,10 +5492,6 @@ namespace libtorrent // this call is only valid on torrents with metadata if (!valid_metadata() || is_seed()) return; - // the vector need to have exactly one element for every file - // in the torrent - TORRENT_ASSERT(int(files.size()) == m_torrent_file->num_files()); - int limit = int(files.size()); if (valid_metadata() && limit > m_torrent_file->num_files()) limit = m_torrent_file->num_files(); @@ -5545,7 +5512,7 @@ namespace libtorrent m_file_priority[i] = 0; } - // storage may be NULL during shutdown + // storage may be NULL during construction and shutdown if (m_torrent_file->num_pieces() > 0 && m_storage) { inc_refcount("file_priority"); @@ -6916,9 +6883,6 @@ namespace libtorrent m_ses.insert_uuid_torrent(m_uuid.empty() ? m_url : m_uuid, me); } - // TODO: make this more generic to not just work if files have been - // renamed, but also if they have been merged into a single file for instance - // maybe use the same format as .torrent files and reuse some code from torrent_info // The mapped_files needs to be read both in the network thread // and in the disk thread, since they both have their own mapped files structures // which are kept in sync @@ -6947,28 +6911,15 @@ namespace libtorrent { const int num_files = (std::min)(file_priority.list_size() , m_torrent_file->num_files()); - m_file_priority.resize(num_files, 4); + std::vector file_prio(num_files); for (int i = 0; i < num_files; ++i) { - m_file_priority[i] = file_priority.list_int_value_at(i, 1); + file_prio[i] = file_priority.list_int_value_at(i, 1); // this is suspicious, leave seed mode - if (m_file_priority[i] == 0) m_seed_mode = false; - } - // unallocated slots are assumed to be priority 1, so cut off any - // trailing ones - int end_range = num_files - 1; - for (; end_range >= 0; --end_range) if (m_file_priority[end_range] != 1) break; - m_file_priority.resize(end_range + 1, 4); - - // initialize pad files to priority 0 - file_storage const& fs = m_torrent_file->files(); - for (int i = 0; i < (std::min)(fs.num_files(), end_range + 1); ++i) - { - if (!fs.pad_file_at(i)) continue; - m_file_priority[i] = 0; + if (file_prio[i] == 0) m_seed_mode = false; } - update_piece_priorities(); + prioritize_files(file_prio); } } @@ -7275,9 +7226,6 @@ namespace libtorrent } // write renamed files - // TODO: 0 make this more generic to not just work if files have been - // renamed, but also if they have been merged into a single file for instance. - // using file_base if (&m_torrent_file->files() != &m_torrent_file->orig_files() && m_torrent_file->files().num_files() == m_torrent_file->orig_files().num_files()) { @@ -8766,7 +8714,7 @@ namespace libtorrent TORRENT_ASSERT(index >= 0); TORRENT_ASSERT(index < m_torrent_file->num_files()); - // stoage may be NULL during shutdown + // storage may be NULL during shutdown if (!m_storage.get()) { if (alerts().should_post()) @@ -9558,58 +9506,30 @@ namespace libtorrent m_save_resume_flags = boost::uint8_t(flags); state_updated(); - if (m_state == torrent_status::checking_files - || m_state == torrent_status::checking_resume_data) - { - if (!need_loaded()) - { - alerts().emplace_alert(get_handle() - , m_error); - return; - } - - // storage may be NULL during shutdown - if (!m_storage) - { - TORRENT_ASSERT(m_abort); - alerts().emplace_alert(get_handle() - , boost::asio::error::operation_aborted); - return; - } - - boost::shared_ptr rd(new entry); - write_resume_data(*rd); - alerts().emplace_alert(rd, get_handle()); - return; - } - - // TODO: 3 this really needs to be moved to do_async_save_resume_data. - // flags need to be passed on - if ((flags & torrent_handle::flush_disk_cache) && m_storage.get()) - m_ses.disk_thread().async_release_files(m_storage.get()); - - m_ses.queue_async_resume_data(shared_from_this()); - } - - bool torrent::do_async_save_resume_data() - { if (!need_loaded()) { - alerts().emplace_alert(get_handle(), m_error); - return false; + alerts().emplace_alert(get_handle() + , m_error); + return; } +/* // storage may be NULL during shutdown if (!m_storage) { TORRENT_ASSERT(m_abort); alerts().emplace_alert(get_handle() , boost::asio::error::operation_aborted); - return false; + return; } - inc_refcount("save_resume"); - m_ses.disk_thread().async_save_resume_data(m_storage.get() - , boost::bind(&torrent::on_save_resume_data, shared_from_this(), _1)); - return true; +*/ + if ((flags & torrent_handle::flush_disk_cache) && m_storage.get()) + m_ses.disk_thread().async_release_files(m_storage.get()); + + state_updated(); + + boost::shared_ptr rd(new entry); + write_resume_data(*rd); + alerts().emplace_alert(rd, get_handle()); } bool torrent::should_check_files() const diff --git a/src/torrent_handle.cpp b/src/torrent_handle.cpp index 33feaa2e5..c33875f80 100644 --- a/src/torrent_handle.cpp +++ b/src/torrent_handle.cpp @@ -734,19 +734,6 @@ namespace libtorrent { entry ret(entry::dictionary_t); TORRENT_SYNC_CALL1(write_resume_data, boost::ref(ret)); - t = m_torrent.lock(); - if (t) - { - bool done = false; - session_impl& ses = static_cast(t->session()); - storage_error ec; - ses.get_io_service().dispatch(boost::bind(&aux::fun_wrap, boost::ref(done), boost::ref(ses.cond) - , boost::ref(ses.mut), boost::function(boost::bind( - &piece_manager::write_resume_data, &t->storage(), boost::ref(ret), boost::ref(ec))))); - t.reset(); - aux::torrent_wait(done, ses); - } - return ret; } diff --git a/test/test_alert_manager.cpp b/test/test_alert_manager.cpp index 343233698..b6ca7a95d 100644 --- a/test/test_alert_manager.cpp +++ b/test/test_alert_manager.cpp @@ -59,8 +59,7 @@ TORRENT_TEST(limit) TEST_EQUAL(mgr.pending(), true); std::vector alerts; - int num_resume; - mgr.get_all(alerts, num_resume); + mgr.get_all(alerts); // even though we posted 600, the limit was 500 TEST_EQUAL(alerts.size(), 500); @@ -75,7 +74,7 @@ TORRENT_TEST(limit) TEST_EQUAL(mgr.pending(), true); - mgr.get_all(alerts, num_resume); + mgr.get_all(alerts); // even though we posted 600, the limit was 200 TEST_EQUAL(alerts.size(), 200); @@ -96,8 +95,7 @@ TORRENT_TEST(priority_limit) mgr.emplace_alert(torrent_handle(), i, error_code()); std::vector alerts; - int num_resume; - mgr.get_all(alerts, num_resume); + mgr.get_all(alerts); // even though we posted 400, the limit was 100 for half of them and // 200 for the other half, meaning we should have 200 alerts now @@ -173,8 +171,7 @@ TORRENT_TEST(notify_function) // however, if we pop all the alerts and post new ones, there will be // and edge triggering the notify call std::vector alerts; - int num_resume; - mgr.get_all(alerts, num_resume); + mgr.get_all(alerts); TEST_EQUAL(mgr.pending(), false); @@ -258,8 +255,7 @@ TORRENT_TEST(wait_for_alert) TEST_CHECK(a->type() == torrent_added_alert::alert_type); std::vector alerts; - int num_resume = 0; - mgr.get_all(alerts, num_resume); + mgr.get_all(alerts); start = clock_type::now(); thread posting_thread(boost::bind(&post_torrent_added, &mgr)); @@ -274,40 +270,6 @@ TORRENT_TEST(wait_for_alert) posting_thread.join(); } -TORRENT_TEST(queued_resume) -{ - alert_manager mgr(100, 0xffffffff); - - TEST_EQUAL(mgr.num_queued_resume(), 0); - - for (int i = 0; i < 17; ++i) - mgr.emplace_alert(torrent_handle()); - - TEST_EQUAL(mgr.num_queued_resume(), 0); - - std::vector alerts; - int num_resume = 0; - mgr.get_all(alerts, num_resume); - TEST_EQUAL(num_resume, 0); - TEST_EQUAL(alerts.size(), 17); - - TEST_EQUAL(mgr.num_queued_resume(), 0); - - error_code ec(boost::system::errc::no_such_file_or_directory - , generic_category()); - - for (int i = 0; i < 2; ++i) - mgr.emplace_alert(torrent_handle(), ec); - - TEST_EQUAL(mgr.num_queued_resume(), 2); - - mgr.get_all(alerts, num_resume); - TEST_EQUAL(num_resume, 2); - TEST_EQUAL(alerts.size(), 2); - - TEST_EQUAL(mgr.num_queued_resume(), 0); -} - TORRENT_TEST(alert_mask) { alert_manager mgr(100, 0xffffffff); diff --git a/test/test_storage.cpp b/test/test_storage.cpp index 06e3699d7..f4e875812 100644 --- a/test/test_storage.cpp +++ b/test/test_storage.cpp @@ -664,7 +664,7 @@ TORRENT_TEST(fastresume) { print_alerts(ses, "ses"); s = h.status(); - if (s.progress == 1.0f) + if (s.progress == 1.0f) { std::cout << "progress: 1.0f" << std::endl; break; @@ -692,7 +692,6 @@ TORRENT_TEST(fastresume) return; std::cerr << resume.to_string() << "\n"; - TEST_CHECK(resume.dict().find("file sizes") != resume.dict().end()); // make sure the fast resume check fails! since we removed the file { @@ -795,7 +794,6 @@ TORRENT_TEST(rename_file_fastresume) TEST_CHECK(!exists(combine_path(test_path, "tmp2/temporary"))); TEST_CHECK(exists(combine_path(test_path, "tmp2/testing_renamed_files"))); TEST_CHECK(resume.dict().find("mapped_files") != resume.dict().end()); - TEST_CHECK(resume.dict().find("file sizes") != resume.dict().end()); std::cerr << resume.to_string() << "\n";