diff --git a/ChangeLog b/ChangeLog index 55b0c2196..6d814a4fc 100644 --- a/ChangeLog +++ b/ChangeLog @@ -85,6 +85,8 @@ * resume data no longer has timestamps of files * require C++11 to build libtorrent + * fix race condition in part_file + * fix part_file open mode compatibility test * fixed race condition in random number generator * fix race condition in stat_cache (disk storage) * improve error handling of failing to change file priority diff --git a/include/libtorrent/part_file.hpp b/include/libtorrent/part_file.hpp index 9ebd6acc4..012d363ae 100644 --- a/include/libtorrent/part_file.hpp +++ b/include/libtorrent/part_file.hpp @@ -38,6 +38,7 @@ POSSIBILITY OF SUCH DAMAGE. #include #include #include +#include #include "libtorrent/config.hpp" #include "libtorrent/file.hpp" @@ -78,8 +79,11 @@ namespace libtorrent { void open_file(open_mode_t mode, error_code& ec); void flush_metadata_impl(error_code& ec); + std::int64_t slot_offset(slot_index_t const slot) const + { return m_header_size + static_cast(slot) * m_piece_size; } + std::string m_path; - std::string m_name; + std::string const m_name; // allocate a slot and return the slot index slot_index_t allocate_slot(piece_index_t piece); @@ -98,15 +102,15 @@ namespace libtorrent { // the max number of pieces in the torrent this part file is // backing - int m_max_pieces; + int const m_max_pieces; // number of bytes each piece contains - int m_piece_size; + int const m_piece_size; // this is the size of the part_file header, it is added // to offsets when calculating the offset to read and write // payload data from - int m_header_size; + int const m_header_size; // if this is true, the metadata in memory has changed since // we last saved or read it from disk. It means that we @@ -117,7 +121,9 @@ namespace libtorrent { std::unordered_map m_piece_map; // this is the file handle to the part file - file m_file; + // it's allocated on the heap and reference counted, to allow it to be + // closed and re-opened while other threads are still using it + std::shared_ptr m_file; }; } diff --git a/src/part_file.cpp b/src/part_file.cpp index 692217ff3..7febe87e9 100644 --- a/src/part_file.cpp +++ b/src/part_file.cpp @@ -92,13 +92,13 @@ namespace libtorrent { error_code ec; std::string fn = combine_path(m_path, m_name); - m_file.open(fn, open_mode::read_only, ec); + auto f = std::make_shared(fn, open_mode::read_only, ec); if (ec) return; // parse header std::vector header(static_cast(m_header_size)); iovec_t b = header; - int n = int(m_file.readv(0, b, ec)); + int n = int(f->readv(0, b, ec)); if (ec) return; // we don't have a full header. consider the file empty @@ -140,8 +140,7 @@ namespace libtorrent { { if (free_slots[i]) m_free_slots.push_back(i); } - - m_file.close(); + m_file = std::move(f); } part_file::~part_file() @@ -185,10 +184,10 @@ namespace libtorrent { slot_index_t const slot = (i == m_piece_map.end()) ? allocate_slot(piece) : i->second; + auto const f = m_file; l.unlock(); - std::int64_t slot_offset = std::int64_t(m_header_size) + std::int64_t(static_cast(slot)) * m_piece_size; - return int(m_file.writev(slot_offset + offset, bufs, ec)); + return int(f->writev(slot_offset(slot) + offset, bufs, ec)); } int part_file::readv(span bufs @@ -200,29 +199,29 @@ namespace libtorrent { auto const i = m_piece_map.find(piece); if (i == m_piece_map.end()) { - ec = error_code(boost::system::errc::no_such_file_or_directory - , boost::system::generic_category()); + ec = make_error_code(boost::system::errc::no_such_file_or_directory); return -1; } slot_index_t const slot = i->second; - open_file(open_mode::read_write | open_mode::attribute_hidden, ec); + open_file(open_mode::read_only | open_mode::attribute_hidden, ec); if (ec) return -1; + auto const f = m_file; l.unlock(); - std::int64_t slot_offset = std::int64_t(m_header_size) + std::int64_t(static_cast(slot)) * m_piece_size; - return int(m_file.readv(slot_offset + offset, bufs, ec)); + return int(f->readv(slot_offset(slot) + offset, bufs, ec)); } void part_file::open_file(open_mode_t const mode, error_code& ec) { - if (m_file.is_open() - && ((m_file.open_mode() & open_mode::rw_mask) == mode - || mode == open_mode::read_only)) return; + if (m_file && m_file->is_open() + && (mode == open_mode::read_only + || (m_file->open_mode() & open_mode::rw_mask) == open_mode::read_write)) + return; - std::string fn = combine_path(m_path, m_name); - m_file.open(fn, mode, ec); + std::string const fn = combine_path(m_path, m_name); + auto f = std::make_shared(fn, mode, ec); if (((mode & open_mode::rw_mask) != open_mode::read_only) && ec == boost::system::errc::no_such_file_or_directory) { @@ -232,8 +231,9 @@ namespace libtorrent { create_directories(m_path, ec); if (ec) return; - m_file.open(fn, mode, ec); + f = std::make_shared(fn, mode, ec); } + if (!ec) m_file = f; } void part_file::free_piece(piece_index_t const piece) @@ -260,7 +260,10 @@ namespace libtorrent { flush_metadata_impl(ec); if (ec) return; - m_file.close(); + // we're only supposed to move part files from a fence job. i.e. no other + // disk jobs are supposed to be in-flight at this point + TORRENT_ASSERT(!m_file || m_file.unique()); + m_file.reset(); if (!m_piece_map.empty()) { @@ -303,17 +306,15 @@ namespace libtorrent { slot_index_t const slot = i->second; open_file(open_mode::read_only, ec); if (ec) return; + auto const local_file = m_file; if (!buf) buf.reset(new char[std::size_t(m_piece_size)]); - std::int64_t const slot_offset = std::int64_t(m_header_size) - + std::int64_t(static_cast(slot)) * m_piece_size; - // don't hold the lock during disk I/O l.unlock(); iovec_t v = {buf.get(), std::size_t(block_to_copy)}; - auto bytes_read = std::size_t(m_file.readv(slot_offset + piece_offset, v, ec)); + auto bytes_read = std::size_t(local_file->readv(slot_offset(slot) + piece_offset, v, ec)); v = v.first(bytes_read); TORRENT_ASSERT(!ec); if (ec || v.empty()) return; @@ -363,11 +364,11 @@ namespace libtorrent { if (m_piece_map.empty()) { - m_file.close(); + m_file.reset(); // if we don't have any pieces left in the // part file, remove it - std::string p = combine_path(m_path, m_name); + std::string const p = combine_path(m_path, m_name); remove(p, ec); if (ec == boost::system::errc::no_such_file_or_directory) @@ -395,6 +396,6 @@ namespace libtorrent { } std::memset(ptr, 0, std::size_t(m_header_size - (ptr - header.data()))); iovec_t b = header; - m_file.writev(0, b, ec); + m_file->writev(0, b, ec); } }