diff --git a/ChangeLog b/ChangeLog index 139164601..963bf2261 100644 --- a/ChangeLog +++ b/ChangeLog @@ -82,6 +82,8 @@ * resume data no longer has timestamps of files * require C++11 to build libtorrent + * defer truncating existing files until the first time we write to them + * fix issue when receiving a torrent with 0-sized padfiles as magnet link * fix issue resuming 1.0.x downloads with a file priority 0 * fix torrent_status::next_announce * fix pad-file scalability issue diff --git a/simulation/test_checking.cpp b/simulation/test_checking.cpp index 8f0f5896d..da023b6c4 100644 --- a/simulation/test_checking.cpp +++ b/simulation/test_checking.cpp @@ -34,6 +34,7 @@ POSSIBILITY OF SUCH DAMAGE. #include "libtorrent/deadline_timer.hpp" #include "libtorrent/address.hpp" #include "libtorrent/torrent_status.hpp" +#include "libtorrent/torrent_info.hpp" #include "simulator/simulator.hpp" #include "simulator/utils.hpp" @@ -42,6 +43,8 @@ POSSIBILITY OF SUCH DAMAGE. #include "create_torrent.hpp" #include "utils.hpp" +#include + template void run_test(Setup const& setup, Test const& test) { @@ -76,6 +79,28 @@ void run_test(Setup const& setup, Test const& test) sim.run(); } +TORRENT_TEST(no_truncate_checking) +{ + std::string filename; + int size = 0; + run_test( + [&](lt::add_torrent_params& atp, lt::settings_pack& p) { + filename = atp.save_path + "/" + atp.ti->files().file_path(lt::file_index_t{0}); + std::ofstream f(filename); + // create a file that's 100 bytes larger + size = int(atp.ti->files().file_size(lt::file_index_t{0}) + 100); + std::vector dummy(size); + f.write(dummy.data(), dummy.size()); + }, + [](lt::session& ses) {} + ); + + // file should not have been truncated just by checking + std::ifstream f(filename); + f.seekg(0, std::ios_base::end); + TEST_EQUAL(f.tellg(), std::fstream::pos_type(size)); +} + TORRENT_TEST(cache_after_checking) { run_test( diff --git a/src/storage.cpp b/src/storage.cpp index 87ca994d7..d0f2daaf0 100644 --- a/src/storage.cpp +++ b/src/storage.cpp @@ -239,9 +239,9 @@ namespace libtorrent { // ignore pad files if (files().pad_file_at(file_index)) continue; + // this is just to see if the file exists error_code err; - std::int64_t size = m_stat_cache.get_filesize(file_index, files() - , m_save_path, err); + m_stat_cache.get_filesize(file_index, files(), m_save_path, err); if (err && err != boost::system::errc::no_such_file_or_directory) { @@ -251,11 +251,12 @@ namespace libtorrent { break; } - // if the file already exists, but is larger than what - // it's supposed to be, truncate it - // if the file is empty, just create it either way. - if ((!err && size > files().file_size(file_index)) - || (files().file_size(file_index) == 0 && err == boost::system::errc::no_such_file_or_directory)) + // if the file is empty and doesn't already exist, create it + // deliberately don't truncate files that already exist + // if a file is supposed to have size 0, but already exists, we will + // never truncate it to 0. + if (files().file_size(file_index) == 0 + && err == boost::system::errc::no_such_file_or_directory) { std::string file_path = files().file_path(file_index, m_save_path); std::string dir = parent_path(file_path); @@ -273,23 +274,12 @@ namespace libtorrent { } } ec.ec.clear(); + // just creating the file is enough to make it zero-sized. If + // there's a race here and some other process truncates the file, + // it's not a problem, we won't access empty files ever again file_handle f = open_file(file_index, open_mode::read_write | open_mode::random_access, ec); - if (ec) - { - ec.file(file_index); - ec.operation = operation_t::file_fallocate; - return; - } - - size = files().file_size(file_index); - f->set_size(size, ec.ec); - if (ec) - { - ec.file(file_index); - ec.operation = operation_t::file_fallocate; - break; - } + if (ec) return; } ec.ec.clear(); } @@ -617,7 +607,7 @@ namespace libtorrent { } TORRENT_ASSERT(h); - if (m_allocate_files && (mode & open_mode::rw_mask) != open_mode::read_only) + if ((mode & open_mode::rw_mask) != open_mode::read_only) { std::unique_lock l(m_file_created_mutex); if (m_file_created.size() != files().num_files()) @@ -632,9 +622,12 @@ namespace libtorrent { { m_file_created.set_bit(file); l.unlock(); - error_code e; + + // if we're allocating files or if the file exists and is greater + // than what it's supposed to be, truncate it to its correct size std::int64_t const size = files().file_size(file); - h->set_size(size, e); + error_code e; + bool const need_truncate = h->get_size(e) > size; if (e) { ec.ec = e; @@ -642,7 +635,19 @@ namespace libtorrent { ec.operation = operation_t::file_fallocate; return h; } - m_stat_cache.set_dirty(file); + + if (m_allocate_files || need_truncate) + { + h->set_size(size, e); + if (e) + { + ec.ec = e; + ec.file(file); + ec.operation = operation_t::file_fallocate; + return h; + } + m_stat_cache.set_dirty(file); + } } } return h; diff --git a/src/torrent.cpp b/src/torrent.cpp index e7522644e..1f3d23801 100644 --- a/src/torrent.cpp +++ b/src/torrent.cpp @@ -1776,6 +1776,8 @@ namespace libtorrent { } else { + need_picker(); + int num_pad_files = 0; TORRENT_ASSERT(block_size() > 0); @@ -1786,10 +1788,6 @@ namespace libtorrent { if (!fs.pad_file_at(i) || fs.file_size(i) == 0) continue; m_padding += std::uint32_t(fs.file_size(i)); - // TODO: instead of creating the picker up front here, - // maybe this whole section should move to need_picker() - need_picker(); - peer_request pr = m_torrent_file->map_file(i, 0, int(fs.file_size(i))); int off = pr.start & (block_size() - 1); if (off != 0) { pr.length -= block_size() - off; pr.start += block_size() - off; } diff --git a/test/test_checking.cpp b/test/test_checking.cpp index 61034820c..455b6d4d3 100644 --- a/test/test_checking.cpp +++ b/test/test_checking.cpp @@ -243,30 +243,9 @@ void test_checking(int flags = read_only_files) { TEST_CHECK(!st.is_seeding); - if (flags & read_only_files) - { - // we expect our checking of the files to trigger - // attempts to truncate them, since the files are - // read-only here, we expect the checking to fail. - TEST_CHECK(st.errc); - if (st.errc) - std::printf("error: %s\n", st.errc.message().c_str()); - - // wait a while to make sure libtorrent survived the error - std::this_thread::sleep_for(lt::milliseconds(1000)); - - st = tor1.status(); - TEST_CHECK(!st.is_seeding); - TEST_CHECK(st.errc); - if (st.errc) - std::printf("error: %s\n", st.errc.message().c_str()); - } - else - { - TEST_CHECK(!st.errc); - if (st.errc) - std::printf("error: %s\n", st.errc.message().c_str()); - } + TEST_CHECK(!st.errc); + if (st.errc) + std::printf("error: %s\n", st.errc.message().c_str()); } if ((flags & (incomplete_files | corrupt_files)) == 0)