deprecate lock_files (#2344)

File locking interacts poorly with the path to replace a read only file handle
with a read/write one. For performance reasons the old handle is not closed
until the new one is put in place. With file locking this obviously cannot
work because the first handle is holding a lock on the file. As a result,
file_pool::open_file fails in this case. Even if we dropped the reference to
the read only file handle before attempting to re-open it, the open could still
fail if another thread is holding a reference to the read only handle.

There doesn't seem to be a good way around this. The simple solution would be
to always open files in read/write mode, but that has obvious safety downsides.
Any other solution would significantly complicate the file pool code.
Deprecating file locking seems to be the least bad option.

bindings/python/src/torrent_handle.cpp

@@ -596,7 +596,9 @@ void bind_torrent_handle()
     s.attr("sparse") = file_open_mode::sparse;
     s.attr("no_atime") = file_open_mode::no_atime;
     s.attr("random_access") = file_open_mode::random_access;
-    s.attr("locked") = file_open_mode::locked;
+#ifndef TORRENT_NO_DEPRECATE
+    s.attr("locked") = 0;
+#endif
     }
 
     enum_<torrent_handle::file_progress_flags_t>("file_progress_flags")

examples/client_test.cpp

@@ -1843,7 +1843,6 @@ COLUMN OPTIONS
 						else if ((f->open_mode & lt::file_open_mode::rw_mask) == lt::file_open_mode::read_only) title += "read ";
 						else if ((f->open_mode & lt::file_open_mode::rw_mask) == lt::file_open_mode::write_only) title += "write ";
 						if (f->open_mode & lt::file_open_mode::random_access) title += "random_access ";
-						if (f->open_mode & lt::file_open_mode::locked) title += "locked ";
 						if (f->open_mode & lt::file_open_mode::sparse) title += "sparse ";
 						title += "]";
 						++f;

include/libtorrent/disk_interface.hpp

@@ -94,9 +94,11 @@ namespace libtorrent {
 		// logic
 		constexpr file_open_mode_t random_access = 5_bit;
 
+#ifndef TORRENT_NO_DEPRECATE
 		// prevent the file from being opened by another process
 		// while it's still being held open by this handle
-		constexpr file_open_mode_t locked = 6_bit;
+		constexpr file_open_mode_t TORRENT_DEPRECATED locked = 6_bit;
+#endif
 	}
 
 	// this contains information about a file that's currently open by the

include/libtorrent/file.hpp

@@ -159,26 +159,22 @@ namespace libtorrent {
 		// logic
 		constexpr open_mode_t random_access = 4_bit;
 
-		// prevent the file from being opened by another process
-		// while it's still being held open by this handle
-		constexpr open_mode_t lock_file = 5_bit;
-
 		// don't put any pressure on the OS disk cache
 		// because of access to this file. We expect our
 		// files to be fairly large, and there is already
 		// a cache at the bittorrent block level. This
 		// may improve overall system performance by
 		// leaving running applications in the page cache
-		constexpr open_mode_t no_cache = 6_bit;
+		constexpr open_mode_t no_cache = 5_bit;
 
 		// this is only used for readv/writev flags
-		constexpr open_mode_t coalesce_buffers = 7_bit;
+		constexpr open_mode_t coalesce_buffers = 6_bit;
 
 		// when creating a file, set the hidden attribute (windows only)
-		constexpr open_mode_t attribute_hidden = 8_bit;
+		constexpr open_mode_t attribute_hidden = 7_bit;
 
 		// when creating a file, set the executable attribute
-		constexpr open_mode_t attribute_executable = 9_bit;
+		constexpr open_mode_t attribute_executable = 8_bit;
 
 		// the mask of all attribute bits
 		constexpr open_mode_t attribute_mask = attribute_hidden | attribute_executable;

include/libtorrent/settings_pack.hpp

@@ -582,6 +582,7 @@ namespace libtorrent {
 			deprecated19,
 #endif
 
+#ifndef TORRENT_NO_DEPRECATE
 			// ``lock_files`` determines whether or not to lock files which
 			// libtorrent is downloading to or seeding from. This is implemented
 			// using ``fcntl(F_SETLK)`` on unix systems and by not passing in
@@ -589,6 +590,9 @@ namespace libtorrent {
 			// 3rd party processes from corrupting the files under libtorrent's
 			// feet.
 			lock_files,
+#else
+			deprecated26,
+#endif
 
 #ifndef TORRENT_NO_DEPRECATE
 			// ``contiguous_recv_buffer`` determines whether or not libtorrent

src/file.cpp

@@ -542,7 +542,7 @@ static_assert(!(open_mode::sparse & open_mode::attribute_mask), "internal flags
 			| ((mode & open_mode::no_cache) ? FILE_FLAG_WRITE_THROUGH : 0);
 
 		handle_type handle = CreateFileW(file_path.c_str(), m.rw_mode
-			, (mode & open_mode::lock_file) ? FILE_SHARE_READ : FILE_SHARE_READ | FILE_SHARE_WRITE
+			, FILE_SHARE_READ | FILE_SHARE_WRITE
 			, 0, m.create_mode, flags, 0);
 
 		if (handle == INVALID_HANDLE_VALUE)
@@ -618,12 +618,6 @@ static_assert(!(open_mode::sparse & open_mode::attribute_mask), "internal flags
 
 		m_file_handle = handle;
 
-		// The purpose of the lock_file flag is primarily to prevent other
-		// processes from corrupting files that are being used by libtorrent.
-		// the posix file locking mechanism does not prevent others from
-		// accessing files, unless they also attempt to lock the file. That's
-		// why the SETLK mechanism is not used here.
-
 #ifdef DIRECTIO_ON
 		// for solaris
 		if ((mode & open_mode::no_cache))

src/file_pool.cpp

@@ -198,7 +198,6 @@ namespace libtorrent {
 		if (mode & open_mode::sparse) ret |= file_open_mode::sparse;
 		if (mode & open_mode::no_atime) ret |= file_open_mode::no_atime;
 		if (mode & open_mode::random_access) ret |= file_open_mode::random_access;
-		if (mode & open_mode::lock_file) ret |= file_open_mode::locked;
 		return ret;
 	}
 

src/settings_pack.cpp

@@ -179,7 +179,7 @@ constexpr int CLOSE_FILE_INTERVAL = 0;
 		SET(always_send_user_agent, false, nullptr),
 		SET(apply_ip_filter_to_trackers, true, nullptr),
 		DEPRECATED_SET(use_disk_read_ahead, true, nullptr),
-		SET(lock_files, false, nullptr),
+		DEPRECATED_SET(lock_files, false, nullptr),
 		DEPRECATED_SET(contiguous_recv_buffer, true, nullptr),
 		SET(ban_web_seeds, true, nullptr),
 		SET(allow_partial_disk_writes, true, nullptr),

src/storage.cpp

@@ -637,9 +637,6 @@ namespace libtorrent {
 	file_handle default_storage::open_file_impl(file_index_t file, open_mode_t mode
 		, error_code& ec) const
 	{
-		bool const lock_files = m_settings ? settings().get_bool(settings_pack::lock_files) : false;
-		if (lock_files) mode |= open_mode::lock_file;
-
 		if (!m_allocate_files) mode |= open_mode::sparse;
 
 		// files with priority 0 should always be sparse
@@ -658,16 +655,6 @@ namespace libtorrent {
 
 		file_handle ret = m_pool.open_file(storage_index(), m_save_path, file
 			, files(), mode, ec);
-		if (ec && (mode & open_mode::lock_file))
-		{
-			// we failed to open the file and we're trying to lock it. It's
-			// possible we're failing because we have another handle to this
-			// file in use (but waiting to be closed). Just retry to open it
-			// without locking.
-			mode &= ~open_mode::lock_file;
-			ret = m_pool.open_file(storage_index(), m_save_path, file, files()
-				, mode, ec);
-		}
 		return ret;
 	}