merge RC_1_1 into master

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

include/libtorrent/part_file.hpp

@@ -38,6 +38,7 @@ POSSIBILITY OF SUCH DAMAGE.
 #include <mutex>
 #include <unordered_map>
 #include <cstdint>
+#include <memory>
 
 #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<int>(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<piece_index_t, slot_index_t> 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<file> m_file;
 	};
 }
 

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<file>(fn, open_mode::read_only, ec);
 		if (ec) return;
 
 		// parse header
 		std::vector<char> header(static_cast<std::size_t>(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<int>(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<iovec_t const> 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<int>(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<file>(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<file>(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<int>(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);
 	}
 }