fix pre-allocation when changing priority

ChangeLog

@@ -1,6 +1,7 @@
 
 2.0.11 not released
 
+	* fix file pre-allocation when changing file priority (HanabishiRecca)
 	* fix uTP issue where closing the connection could corrupt the payload
 	* apply DSCP/TOS to sockets before initiating the TCP connection
 	* assume copy_file_range() exists on linux (unless old glibc)

include/libtorrent/aux_/posix_storage.hpp

@@ -68,7 +68,8 @@ namespace aux {
 			, storage_error& error);
 
 		bool has_any_file(storage_error& error);
-		void set_file_priority(aux::vector<download_priority_t, file_index_t>& prio
+		void set_file_priority(settings_interface const&
+			, aux::vector<download_priority_t, file_index_t>& prio
 			, storage_error& ec);
 		bool verify_resume_data(add_torrent_params const& rd
 			, aux::vector<std::string, file_index_t> const& links

src/mmap_storage.cpp

@@ -63,6 +63,7 @@ POSSIBILITY OF SUCH DAMAGE.
 #include "libtorrent/aux_/drive_info.hpp"
 #include "libtorrent/stat_cache.hpp"
 #include "libtorrent/hex.hpp" // to_hex
+#include "libtorrent/aux_/scope_end.hpp"
 
 #if TORRENT_HAVE_MMAP || TORRENT_HAVE_MAP_VIEW_OF_FILE
 
@@ -161,15 +162,22 @@ error_code translate_error(std::error_code const& err, bool const write)
 
 			download_priority_t const old_prio = m_file_priority[i];
 			download_priority_t new_prio = prio[i];
+
+			m_file_priority[i] = new_prio;
+
+			// in case there's an error, we make sure m_file_priority is only
+			// updated for the successful files. By leaving failed files as
+			// priority 0, we allow re-trying them.
+			auto restore_prio = aux::scope_end([&] {
+				m_file_priority[i] = old_prio;
+				prio = m_file_priority;
+			});
+
 			if (old_prio == dont_download && new_prio != dont_download)
 			{
 				// move stuff out of the part file
 				std::shared_ptr<aux::file_mapping> f = open_file(sett, i, aux::open_mode::write, ec);
-				if (ec)
-				{
-					prio = m_file_priority;
-					return;
-				}
+				if (ec) return;
 
 				if (m_part_file && use_partfile(i))
 				{
@@ -196,7 +204,6 @@ error_code translate_error(std::error_code const& err, bool const write)
 						{
 							ec.file(i);
 							ec.operation = operation_t::partfile_write;
-							prio = m_file_priority;
 							return;
 						}
 					}
@@ -228,19 +235,14 @@ error_code translate_error(std::error_code const& err, bool const write)
 				{
 					ec.file(i);
 					ec.operation = operation_t::file_stat;
-					prio = m_file_priority;
 					return;
 				}
 				use_partfile(i, !file_exists);
 /*
 				auto f = open_file(sett, i, aux::open_mode::read_only, ec);
 				if (ec.ec != boost::system::errc::no_such_file_or_directory)
 				{
-					if (ec)
-					{
-						prio = m_file_priority;
-						return;
-					}
+					if (ec) return;
 
 					need_partfile();
 
@@ -249,7 +251,6 @@ error_code translate_error(std::error_code const& err, bool const write)
 					{
 						ec.file(i);
 						ec.operation = operation_t::partfile_read;
-						prio = m_file_priority;
 						return;
 					}
 					// remove the file
@@ -259,14 +260,13 @@ error_code translate_error(std::error_code const& err, bool const write)
 					{
 						ec.file(i);
 						ec.operation = operation_t::file_remove;
-						prio = m_file_priority;
 						return;
 					}
 				}
 */
 			}
 			ec.ec.clear();
-			m_file_priority[i] = new_prio;
+			restore_prio.disarm();
 
 			if (m_file_priority[i] == dont_download && use_partfile(i))
 			{

src/part_file.cpp

@@ -370,7 +370,6 @@ namespace libtorrent {
 				span<char> v = {buf.get(), block_to_copy};
 				auto bytes_read = aux::pread_all(file.fd(), v, slot_offset(slot) + piece_offset, ec);
 				v = v.first(static_cast<std::ptrdiff_t>(bytes_read));
-				TORRENT_ASSERT(!ec);
 				if (ec || v.empty()) return;
 
 				f(file_offset, {buf.get(), block_to_copy});

src/posix_disk_io.cpp

@@ -358,7 +358,7 @@ namespace {
 		{
 			posix_storage* st = m_torrents[storage].get();
 			storage_error error;
-			st->set_file_priority(prio, error);
+			st->set_file_priority(m_settings, prio, error);
 			post(m_ios, [p = std::move(prio), h = std::move(handler), error] () mutable
 				{ h(error, std::move(p)); });
 		}

src/posix_part_file.cpp

@@ -406,7 +406,6 @@ namespace aux {
 				if (int(bytes_read) != block_to_copy)
 					ec.assign(errno, generic_category());
 
-				TORRENT_ASSERT(!ec);
 				if (ec) return;
 
 				f(file_offset, {buf.get(), block_to_copy});

src/posix_storage.cpp

@@ -84,7 +84,8 @@ namespace aux {
 			, files().num_pieces(), files().piece_length());
 	}
 
-	void posix_storage::set_file_priority(aux::vector<download_priority_t, file_index_t>& prio
+	void posix_storage::set_file_priority(settings_interface const&
+		, aux::vector<download_priority_t, file_index_t>& prio
 		, storage_error& ec)
 	{
 		// extend our file priorities in case it's truncated

test/test_storage.cpp

@@ -539,6 +539,120 @@
 	TEST_EQUAL(s->files().file_path(0_file), "new_filename");
 }
 
+namespace {
+std::int64_t file_size_on_disk(std::string const& path)
+{
+#ifdef TORRENT_WINDOWS
+	native_path_string f = convert_to_native_path_string(path);
+	// in order to open a directory, we need the FILE_FLAG_BACKUP_SEMANTICS
+	HANDLE h = CreateFileW(f.c_str(), 0, FILE_SHARE_DELETE | FILE_SHARE_READ
+		| FILE_SHARE_WRITE, nullptr, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, nullptr);
+	TEST_CHECK(h != INVALID_HANDLE_VALUE);
+	FILE_STANDARD_INFO Standard;
+	TEST_CHECK(GetFileInformationByHandleEx(h, FILE_INFO_BY_HANDLE_CLASS::FileStandardInfo, &Standard, sizeof(FILE_STANDARD_INFO)));
+	CloseHandle(h);
+	return Standard.AllocationSize.QuadPart;
+#else
+	struct ::stat st{};
+	TEST_EQUAL(::stat(path.c_str(), &st), 0);
+	return std::int64_t(st.st_blocks) * 512;
+#endif
+}
+}
+
+template <typename StorageType>
+void test_pre_allocate()
+{
+	std::string const test_path = complete("pre_allocate_test_path");
+	delete_dirs(combine_path(test_path, "temp_storage"));
+
+	file_storage fs;
+	std::vector<char> buf;
+	typename file_pool_type<StorageType>::type fp;
+	io_context ios;
+
+	aux::session_settings set;
+	std::shared_ptr<torrent_info> info = setup_torrent_info(fs, buf);
+
+	aux::vector<download_priority_t, file_index_t> priorities{
+		lt::dont_download,
+		lt::default_priority,
+		lt::default_priority,
+		lt::default_priority,
+		lt::default_priority,
+	};
+	sha1_hash info_hash;
+	storage_params p{
+		fs,
+		nullptr,
+		test_path,
+		storage_mode_allocate,
+		priorities,
+		info_hash
+	};
+	auto s = make_storage<StorageType>(p, fp);
+
+	// allocate the files and create the directories
+	storage_error se;
+	s->initialize(set, se);
+	if (se)
+	{
+		TEST_ERROR(se.ec.message().c_str());
+		std::printf("storage::initialize %s: %d\n"
+			, se.ec.message().c_str(), static_cast<int>(se.file()));
+		throw system_error(se.ec);
+	}
+
+	std::vector<char> piece1 = new_piece(0x4000);
+	span<char> iov = span<char>(piece1);
+
+	// ensure all files, except the first one, have been allocated
+	for (auto i : fs.file_range())
+	{
+		if (fs.file_size(i) > 0)
+		{
+			int ret = write(s, set, iov, fs.piece_index_at_file(i), 0, aux::open_mode::write, se);
+			TEST_EQUAL(ret, int(iov.size()));
+			TEST_CHECK(!se.ec);
+		}
+
+		error_code ec;
+		file_status st;
+		std::string const path = fs.file_path(i, test_path);
+		stat_file(path, &st, ec);
+		if (i == file_index_t{0})
+		{
+			// the first file has priority 0, and so should not be created
+			TEST_EQUAL(ec, boost::system::errc::no_such_file_or_directory);
+		}
+		else
+		{
+			TEST_CHECK(!ec);
+			std::cerr << "error: " << ec.message() << std::endl;
+			TEST_EQUAL(st.file_size, fs.file_size(i));
+			TEST_CHECK(file_size_on_disk(path) >= fs.file_size(i));
+		}
+	}
+
+	std::cerr << "set file priority" << std::endl;
+	// set priority of file 0 to non-zero, and make sure we create the file now
+	priorities[0_file] = lt::default_priority;
+	s->set_file_priority(set, priorities, se);
+	TEST_CHECK(!se.ec);
+
+	for (auto i : fs.file_range())
+	{
+		error_code ec;
+		file_status st;
+		std::string const path = fs.file_path(i, test_path);
+		stat_file(path, &st, ec);
+		std::cerr<< "error: " << ec.message() << std::endl;
+		TEST_CHECK(!ec);
+
+		TEST_CHECK(file_size_on_disk(path) >= fs.file_size(i));
+	}
+}
+
 using lt::operator""_bit;
 using check_files_flag_t = lt::flags::bitfield_flag<std::uint64_t, struct check_files_flag_type_tag>;
 
@@ -709,11 +823,15 @@
 	test_check_files(sparse | test_oversized, lt::mmap_disk_io_constructor);
 }
 
-
 TORRENT_TEST(check_files_allocate_mmap)
 {
 	test_check_files(zero_prio, lt::mmap_disk_io_constructor);
 }
+
+TORRENT_TEST(test_pre_allocate_mmap)
+{
+	test_pre_allocate<mmap_storage>();
+}
 #endif
 TORRENT_TEST(check_files_sparse_posix)
 {
@@ -736,6 +854,14 @@
 	test_check_files(zero_prio, lt::posix_disk_io_constructor);
 }
 
+// posix_storage doesn't support pre-allocating files on non-windows
+/*
+TORRENT_TEST(test_pre_allocate_posix)
+{
+	test_pre_allocate<posix_storage>();
+}
+*/
+
 #if TORRENT_HAVE_MMAP || TORRENT_HAVE_MAP_VIEW_OF_FILE
 TORRENT_TEST(rename_mmap_disk_io)
 {