defer truncating existing files until the first time we write to them

ChangeLog

@@ -1,4 +1,5 @@
 
+	* 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

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 <fstream>
+
 template <typename Setup, typename Test>
 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 = lt::current_working_directory() + "/" + atp.save_path + "/" + atp.ti->files().file_path(0);
+			std::ofstream f(filename);
+			// create a file that's 100 bytes larger
+			size = atp.ti->files().file_size(0) + 100;
+			std::vector<char> 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(

src/storage.cpp

@@ -583,11 +583,11 @@ namespace libtorrent
 				}
 			}
 
-			// if the file already exists, but is larger than what
-			// it's supposed to be, truncate it
 			// if the file is empty and doesn't already exist, create it
-			if ((!ec && cached_size > files().file_size(file_index))
-				|| (files().file_size(file_index) == 0 && cached_size == stat_cache::no_exist))
+			// 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 && cached_size == stat_cache::no_exist)
 			{
 				std::string file_path = files().file_path(file_index, m_save_path);
 				std::string dir = parent_path(file_path);
@@ -605,20 +605,16 @@ 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, file::read_write
 					| file::random_access, ec);
 				if (ec) return;
 
-				boost::int64_t const size = files().file_size(file_index);
-				f->set_size(size, ec.ec);
-				if (ec)
-				{
-					ec.file = file_index;
-					ec.operation = storage_error::fallocate;
-					break;
-				}
 				size_t const mtime = m_stat_cache.get_filetime(file_index);
-				m_stat_cache.set_cache(file_index, size, mtime);
+				m_stat_cache.set_cache(file_index, 0, mtime);
 			}
 			ec.ec.clear();
 		}
@@ -1544,7 +1540,7 @@ namespace libtorrent
 		}
 		TORRENT_ASSERT(h);
 
-		if (m_allocate_files && (mode & file::rw_mask) != file::read_only)
+		if ((mode & file::rw_mask) != file::read_only)
 		{
 			mutex::scoped_lock l(m_file_created_mutex);
 			if (m_file_created.size() != files().num_files())
@@ -1559,9 +1555,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
 				boost::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;
@@ -1569,7 +1568,19 @@ namespace libtorrent
 					ec.operation = storage_error::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 = storage_error::fallocate;
+						return h;
+					}
+					m_stat_cache.set_dirty(file);
+				}
 			}
 		}
 		return h;

test/test_checking.cpp

@@ -241,30 +241,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)
-				fprintf(stdout, "error: %s\n", st.errc.message().c_str());
-
-			// wait a while to make sure libtorrent survived the error
-			test_sleep(1000);
-
-			st = tor1.status();
-			TEST_CHECK(!st.is_seeding);
-			TEST_CHECK(st.errc);
-			if (st.errc)
-				fprintf(stdout, "error: %s\n", st.errc.message().c_str());
-		}
-		else
-		{
-			TEST_CHECK(!st.errc);
-			if (st.errc)
-				fprintf(stdout, "error: %s\n", st.errc.message().c_str());
-		}
+		TEST_CHECK(!st.errc);
+		if (st.errc)
+			fprintf(stdout, "error: %s\n", st.errc.message().c_str());
 	}
 
 	if ((flags & (incomplete_files | corrupt_files)) == 0)