steps towars making file_pool private to disk_io_thread

examples/connection_tester.cpp

@@ -851,10 +851,9 @@ void generate_data(char const* path, torrent_info const& ti)
 	params.files = &const_cast<file_storage&>(fs);
 	params.mapped_files = nullptr;
 	params.path = path;
-	params.pool = &fp;
 	params.mode = storage_mode_sparse;
 
-	std::unique_ptr<storage_interface> st(default_storage_constructor(params));
+	std::unique_ptr<storage_interface> st(default_storage_constructor(params, fp));
 
 	{
 		storage_error error;

include/libtorrent/disk_interface.hpp

@@ -74,7 +74,8 @@ namespace libtorrent
 			volatile_read = 0x10,
 		};
 
-		virtual storage_holder new_torrent(std::unique_ptr<storage_interface> storage) = 0;
+		virtual storage_holder new_torrent(storage_constructor_type sc
+			, storage_params p, std::shared_ptr<void> const&) = 0;
 		virtual void remove_torrent(storage_index_t) = 0;
 		virtual storage_interface* get_torrent(storage_index_t) = 0;
 

include/libtorrent/disk_io_thread.hpp

@@ -293,7 +293,8 @@ namespace libtorrent
 
 		void abort(bool wait);
 
-		storage_holder new_torrent(std::unique_ptr<storage_interface> storage) override;
+		storage_holder new_torrent(storage_constructor_type sc
+			, storage_params p, std::shared_ptr<void> const&) override;
 		void remove_torrent(storage_index_t) override;
 
 		void async_read(storage_index_t storage, peer_request const& r

include/libtorrent/storage.hpp

@@ -72,7 +72,7 @@ POSSIBILITY OF SUCH DAMAGE.
 //
 //	struct temp_storage : storage_interface
 //	{
-//		temp_storage(file_storage const& fs) : m_files(fs) {}
+//		temp_storage(file_storage const& fs) : storage_interface(fs) {}
 //		virtual bool initialize(storage_error& se) { return false; }
 //		virtual bool has_any_file() { return false; }
 //		virtual int read(char* buf, int piece, int offset, int size)
@@ -99,7 +99,7 @@ POSSIBILITY OF SUCH DAMAGE.
 //			, std::vector<std::string> const* links
 //			, storage_error& error) { return false; }
 //		virtual std::int64_t physical_offset(int piece, int offset)
-//		{ return piece * m_files.piece_length() + offset; };
+//		{ return piece * files().piece_length() + offset; };
 //		virtual sha1_hash hash_for_slot(int piece, partial_hash& ph, int piece_size)
 //		{
 //			int left = piece_size - ph.offset;
@@ -120,7 +120,6 @@ POSSIBILITY OF SUCH DAMAGE.
 //		virtual bool delete_files() { return false; }
 //
 //		std::map<int, std::vector<char>> m_file_data;
-//		file_storage m_files;
 //	};
 //
 //	storage_interface* temp_storage_constructor(storage_params const& params)
@@ -169,6 +168,7 @@ namespace libtorrent
 		, public aux::storage_piece_set
 		, boost::noncopyable
 	{
+		explicit storage_interface(file_storage const& fs) : m_files(fs) {}
 
 		// This function is called when the storage is to be initialized. The
 		// default storage will create directories and empty files at this point.
@@ -304,7 +304,7 @@ namespace libtorrent
 		// off again.
 		virtual bool tick() { return false; }
 
-		file_storage const* files() const { return m_files; }
+		file_storage const* files() const { return &m_files; }
 
 		bool set_need_tick()
 		{
@@ -319,7 +319,6 @@ namespace libtorrent
 			tick();
 		}
 
-		void set_files(file_storage const* f) { m_files = f; }
 		void set_owner(std::shared_ptr<void> const& tor) { m_torrent = tor; }
 
 		// access global session_settings
@@ -343,7 +342,7 @@ namespace libtorrent
 	private:
 
 		bool m_need_tick = false;
-		file_storage const* m_files = nullptr;
+		file_storage const& m_files;
 
 		// the reason for this to be a void pointer
 		// is to avoid creating a dependency on the
@@ -378,7 +377,7 @@ namespace libtorrent
 		// an empty vector. Any file whose index is not represented by the vector
 		// (because the vector is too short) are assumed to have priority 1.
 		// this is used to treat files with priority 0 slightly differently.
-		explicit default_storage(storage_params const& params);
+		explicit default_storage(storage_params const& params, file_pool&);
 
 		// hidden
 		~default_storage();
@@ -405,7 +404,10 @@ namespace libtorrent
 
 		// if the files in this storage are mapped, returns the mapped
 		// file_storage, otherwise returns the original file_storage object.
-		file_storage const& files() const { return m_mapped_files ? *m_mapped_files : m_files; }
+		file_storage const& files() const
+		{
+			return m_mapped_files ? *m_mapped_files : *storage_interface::files();
+		}
 
 	private:
 
@@ -414,7 +416,6 @@ namespace libtorrent
 		void need_partfile();
 
 		std::unique_ptr<file_storage> m_mapped_files;
-		file_storage const& m_files;
 
 		// in order to avoid calling stat() on each file multiple times
 		// during startup, cache the results in here, and clear it all
@@ -429,9 +430,8 @@ namespace libtorrent
 		aux::vector<std::uint8_t, file_index_t> m_file_priority;
 		std::string m_save_path;
 		std::string m_part_file_name;
-		// the file pool is typically stored in
+		// the file pool is a member of the disk_io_thread
-		// the session, to make all storage
+		// to make all storage instances share the pool
-		// instances use the same pool
 		file_pool& m_pool;
 
 		// used for skipped files

include/libtorrent/storage_defs.hpp

@@ -92,32 +92,29 @@ namespace libtorrent
 		dont_replace
 	};
 
-	// see default_storage::default_storage()
 	struct TORRENT_EXPORT storage_params
 	{
-		storage_params(): files(nullptr), mapped_files(nullptr), pool(nullptr)
+		file_storage const* files = nullptr;
-			, mode(storage_mode_sparse), priorities(nullptr), info(nullptr) {}
+		file_storage const* mapped_files = nullptr; // optional
-		file_storage const* files;
-		file_storage const* mapped_files; // optional
 		std::string path;
-		file_pool* pool;
+		storage_mode_t mode{storage_mode_sparse};
-		storage_mode_t mode;
+		aux::vector<std::uint8_t, file_index_t> const* priorities = nullptr; // optional
-		aux::vector<std::uint8_t, file_index_t> const* priorities; // optional
+		torrent_info const* info = nullptr; // optional
-		torrent_info const* info; // optional
 	};
 
-	using storage_constructor_type = std::function<storage_interface*(storage_params const& params)>;
+	using storage_constructor_type = std::function<storage_interface*(storage_params const& params, file_pool&)>;
 
 	// the constructor function for the regular file storage. This is the
 	// default value for add_torrent_params::storage.
-	TORRENT_EXPORT storage_interface* default_storage_constructor(storage_params const&);
+	TORRENT_EXPORT storage_interface* default_storage_constructor(storage_params const&
+		, file_pool& p);
 
 	// the constructor function for the disabled storage. This can be used for
 	// testing and benchmarking. It will throw away any data written to
 	// it and return garbage for anything read from it.
-	TORRENT_EXPORT storage_interface* disabled_storage_constructor(storage_params const&);
+	TORRENT_EXPORT storage_interface* disabled_storage_constructor(storage_params const&, file_pool&);
 
-	TORRENT_EXPORT storage_interface* zero_storage_constructor(storage_params const&);
+	TORRENT_EXPORT storage_interface* zero_storage_constructor(storage_params const&, file_pool&);
 }
 
 #endif

src/create_torrent.cpp

@@ -276,12 +276,9 @@ namespace libtorrent
 		params.files = &t.files();
 		params.mapped_files = nullptr;
 		params.path = path;
-		params.pool = &disk_thread.files();
 		params.mode = storage_mode_sparse;
 
-		std::unique_ptr<storage_interface> stor(default_storage_constructor(params));
+		storage_holder storage = disk_thread.new_torrent(default_storage_constructor, std::move(params), std::shared_ptr<void>());
-		stor->set_files(&t.files());
-		storage_holder storage = disk_thread.new_torrent(std::move(stor));
 
 		settings_pack sett;
 		sett.set_int(settings_pack::cache_size, 0);

src/disk_io_thread.cpp

@@ -206,8 +206,12 @@ namespace libtorrent
 		return m_torrents[storage].get();
 	}
 
-	storage_holder disk_io_thread::new_torrent(std::unique_ptr<storage_interface> storage)
+	storage_holder disk_io_thread::new_torrent(storage_constructor_type sc
+		, storage_params p, std::shared_ptr<void> const& owner)
 	{
+		std::unique_ptr<storage_interface> storage(sc(p, m_file_pool));
+		storage->set_owner(owner);
+
 		TORRENT_ASSERT(storage);
 		if (m_free_slots.empty())
 		{

src/storage.cpp

@@ -241,15 +241,16 @@ namespace libtorrent
 		int const m_flags;
 	};
 
-	default_storage::default_storage(storage_params const& params)
+	default_storage::default_storage(storage_params const& params
-		: m_files(*params.files)
+		, file_pool& pool)
-		, m_pool(*params.pool)
+		: storage_interface(*params.files)
+		, m_pool(pool)
 		, m_allocate_files(params.mode == storage_mode_allocate)
 	{
 		if (params.mapped_files) m_mapped_files.reset(new file_storage(*params.mapped_files));
 		if (params.priorities) m_file_priority = *params.priorities;
 
-		TORRENT_ASSERT(m_files.num_files() > 0);
+		TORRENT_ASSERT(files().num_files() > 0);
 		m_save_path = complete(params.path);
 		m_part_file_name = "." + (params.info
 			? aux::to_hex(params.info->info_hash())
@@ -272,7 +273,7 @@ namespace libtorrent
 
 		m_part_file.reset(new part_file(
 			m_save_path, m_part_file_name
-			, m_files.num_pieces(), m_files.piece_length()));
+			, files().num_pieces(), files().piece_length()));
 	}
 
 	void default_storage::set_file_priority(
@@ -544,7 +545,7 @@ namespace libtorrent
 		// in our file_storage, so that when it is created
 		// it will get the new name
 		if (!m_mapped_files)
-		{ m_mapped_files.reset(new file_storage(m_files)); }
+		{ m_mapped_files.reset(new file_storage(files())); }
 		m_mapped_files->rename_file(index, new_filename);
 	}
 
@@ -736,9 +737,10 @@ namespace libtorrent
 		return false;
 	}
 
-	storage_interface* default_storage_constructor(storage_params const& params)
+	storage_interface* default_storage_constructor(storage_params const& params
+		, file_pool& pool)
 	{
-		return new default_storage(params);
+		return new default_storage(params, pool);
 	}
 
 	// -- disabled_storage --------------------------------------------------
@@ -756,6 +758,8 @@ namespace libtorrent
 		class disabled_storage final : public storage_interface
 		{
 		public:
+			explicit disabled_storage(file_storage const& fs) : storage_interface(fs) {}
+
 			bool has_any_file(storage_error&) override { return false; }
 			void set_file_priority(aux::vector<std::uint8_t, file_index_t> const&
 				, storage_error&) override {}
@@ -782,10 +786,9 @@ namespace libtorrent
 		};
 	}
 
-	storage_interface* disabled_storage_constructor(storage_params const& params)
+	storage_interface* disabled_storage_constructor(storage_params const& params, file_pool&)
 	{
-		TORRENT_UNUSED(params);
+		return new disabled_storage(*params.files);
-		return new disabled_storage;
 	}
 
 	// -- zero_storage ------------------------------------------------------
@@ -796,6 +799,7 @@ namespace libtorrent
 		// anything written to it
 		struct zero_storage final : storage_interface
 		{
+			explicit zero_storage(file_storage const& fs) : storage_interface(fs) {}
 			void initialize(storage_error&) override {}
 
 			int readv(span<iovec_t const> bufs
@@ -834,9 +838,9 @@ namespace libtorrent
 		};
 	}
 
-	storage_interface* zero_storage_constructor(storage_params const&)
+	storage_interface* zero_storage_constructor(storage_params const& params, file_pool&)
 	{
-		return new zero_storage;
+		return new zero_storage(*params.files);
 	}
 
 } // namespace libtorrent

src/torrent.cpp

@@ -1593,19 +1593,14 @@ namespace libtorrent
 			params.mapped_files = nullptr;
 		}
 		params.path = m_save_path;
-		params.pool = &m_ses.disk_thread().files();
 		params.mode = static_cast<storage_mode_t>(m_storage_mode);
 		params.priorities = &m_file_priority;
 		params.info = m_torrent_file.get();
 
 		TORRENT_ASSERT(m_storage_constructor);
 
-		std::unique_ptr<storage_interface> storage(m_storage_constructor(params));
+		m_storage = m_ses.disk_thread().new_torrent(m_storage_constructor
-		storage->set_files(&m_torrent_file->files());
+			, params, shared_from_this());
-		// the shared_from_this() will create an intentional
-		// cycle of ownership, se the hpp file for description.
-		storage->set_owner(shared_from_this());
-		m_storage = m_ses.disk_thread().new_torrent(std::move(storage));
 	}
 
 	peer_connection* torrent::find_lowest_ranking_peer() const

test/make_torrent.cpp

@@ -173,9 +173,8 @@ void generate_files(libtorrent::torrent_info const& ti, std::string const& path
 	storage_params params;
 	params.files = &ti.files();
 	params.path = path;
-	params.pool = &fp;
 
-	default_storage st(params);
+	default_storage st(params, fp);
 
 	file_storage const& fs = ti.files();
 	std::vector<char> buffer;

test/test_block_cache.cpp

@@ -46,6 +46,7 @@ using namespace libtorrent;
 
 struct test_storage_impl : storage_interface
 {
+	explicit test_storage_impl(file_storage const& fs) : storage_interface(fs) {}
 	void initialize(storage_error& ec) override {}
 
 	int readv(span<iovec_t const> bufs
@@ -97,8 +98,7 @@ static void nop() {}
 	fs.set_piece_length(0x8000); \
 	fs.set_num_pieces(5); \
 	std::shared_ptr<storage_interface> pm \
-		= std::make_shared<test_storage_impl>(); \
+		= std::make_shared<test_storage_impl>(fs); \
-	pm->set_files(&fs); \
 	bc.set_settings(sett); \
 	pm->m_settings = &sett; \
 	disk_io_job rj; \

test/test_storage.cpp

@@ -157,10 +157,9 @@ std::shared_ptr<default_storage> setup_torrent(file_storage& fs
 
 	storage_params p;
 	p.files = &fs;
-	p.pool = &fp;
 	p.path = test_path;
 	p.mode = storage_mode_allocate;
-	std::shared_ptr<default_storage> s(new default_storage(p));
+	std::shared_ptr<default_storage> s(new default_storage(p, fp));
 	s->m_settings = &set;
 
 	// allocate the files and create the directories
@@ -227,9 +226,8 @@ void run_storage_tests(std::shared_ptr<torrent_info> info
 	storage_params p;
 	p.path = test_path;
 	p.files = &fs;
-	p.pool = &fp;
 	p.mode = storage_mode;
-	std::unique_ptr<storage_interface> s(new default_storage(p));
+	std::unique_ptr<storage_interface> s(new default_storage(p, fp));
 	s->m_settings = &set;
 
 	storage_error ec;
@@ -472,12 +470,10 @@ void test_check_files(std::string const& test_path
 	storage_params p;
 	p.files = &fs;
 	p.path = test_path;
-	p.pool = &fp;
 	p.mode = storage_mode;
 
-	std::unique_ptr<storage_interface> pm(new default_storage(p));
+	auto st = io.new_torrent(default_storage_constructor, std::move(p)
-	pm->set_files(&fs);
+		, std::shared_ptr<void>());
-	auto st = io.new_torrent(std::move(pm));
 	std::mutex lock;
 
 	bool done = false;

test/test_transfer.cpp

@@ -71,8 +71,8 @@ bool on_alert(alert const* a)
 // simulate a full disk
 struct test_storage : default_storage
 {
-	explicit test_storage(storage_params const& params)
+	explicit test_storage(storage_params const& params, file_pool& pool)
-		: default_storage(params)
+		: default_storage(params, pool)
 		, m_written(0)
 		, m_limit(16 * 1024 * 2)
 	{}
@@ -115,9 +115,9 @@ struct test_storage : default_storage
 	std::mutex m_mutex;
 };
 
-storage_interface* test_storage_constructor(storage_params const& params)
+storage_interface* test_storage_constructor(storage_params const& params, file_pool& pool)
 {
-	return new test_storage(params);
+	return new test_storage(params, pool);
 }
 
 void test_transfer(int proxy_type, settings_pack const& sett