minor cleanup and modernization of torrent_peer

include/libtorrent/aux_/session_impl.hpp

@@ -249,8 +249,8 @@ namespace aux {
 #if TORRENT_USE_INVARIANT_CHECKS
 			friend class libtorrent::invariant_access;
 #endif
-			typedef std::set<std::shared_ptr<peer_connection>> connection_map;
-			typedef std::unordered_map<sha1_hash, std::shared_ptr<torrent>> torrent_map;
+			using connection_map = std::set<std::shared_ptr<peer_connection>>;
+			using torrent_map = std::unordered_map<sha1_hash, std::shared_ptr<torrent>>;
 
 			explicit session_impl(io_service& ios);
 			~session_impl() override;
@@ -834,6 +834,10 @@ namespace aux {
 			resolver m_host_resolver;
 
 			tracker_manager m_tracker_manager;
+
+			// the torrents must be destructed after the torrent_peer_allocator,
+			// since the torrents hold the peer lists that own the torrent_peers
+			// (which are allocated in the torrent_peer_allocator)
 			torrent_map m_torrents;
 
 			// all torrents that are downloading or queued,

include/libtorrent/peer_list.hpp

@@ -61,41 +61,30 @@ namespace libtorrent {
 	// the peer_list type not depend on the torrent type directly.
 	struct torrent_state
 	{
-		torrent_state()
-			: is_paused(false)
-			, is_finished(false)
-			, allow_multiple_connections_per_ip(false)
-			, first_time_seen(false)
-			, max_peerlist_size(1000)
-			, min_reconnect_time(60)
-			, loop_counter(0)
-			, port(0)
-			, max_failcount(3)
-		{}
-		bool is_paused;
-		bool is_finished;
-		bool allow_multiple_connections_per_ip;
+		bool is_paused = false;
+		bool is_finished = false;
+		bool allow_multiple_connections_per_ip = false;
 
 		// this is set by peer_list::add_peer to either true or false
 		// true means the peer we just added was new, false means
 		// we already knew about the peer
-		bool first_time_seen;
+		bool first_time_seen = false;
 
-		int max_peerlist_size;
-		int min_reconnect_time;
+		int max_peerlist_size = 1000;
+		int min_reconnect_time = 60;
 
 		// the number of iterations over the peer list for this operation
-		int loop_counter;
+		int loop_counter = 0;
 
 		// these are used only by find_connect_candidates in order
 		// to implement peer ranking. See:
 		// http://blog.libtorrent.org/2012/12/swarm-connectivity/
 		external_ip ip;
-		int port;
+		int port = 0;
 
 		// the number of times a peer must fail before it's no longer considered
 		// a connect candidate
-		int max_failcount;
+		int max_failcount = 3;
 
 		// if any peer were removed during this call, they are returned in
 		// this vector. The caller would want to make sure there are no
@@ -110,6 +99,10 @@ namespace libtorrent {
 		explicit peer_list(torrent_peer_allocator_interface& alloc);
 		~peer_list();
 
+		// not copyable
+		peer_list(peer_list const&) = delete;
+		peer_list& operator=(peer_list const&) = delete;
+
 #if TORRENT_USE_I2P
 		torrent_peer* add_i2p_peer(string_view destination
 			, peer_source_flags_t src, char flags
@@ -206,10 +199,6 @@ namespace libtorrent {
 
 	private:
 
-		// not copyable
-		peer_list(peer_list const&);
-		peer_list& operator=(peer_list const&);
-
 		void recalculate_connect_candidates(torrent_state* state);
 
 		void update_connect_candidates(int delta);

include/libtorrent/torrent_peer.hpp

@@ -54,6 +54,11 @@ namespace libtorrent {
 	struct TORRENT_EXTRA_EXPORT torrent_peer
 	{
 		torrent_peer(std::uint16_t port, bool connectable, peer_source_flags_t src);
+#if TORRENT_USE_ASSERTS
+		torrent_peer(torrent_peer const&) = default;
+		torrent_peer& operator=(torrent_peer const&) = default;
+		~torrent_peer() { in_use = false; }
+#endif
 
 		std::int64_t total_download() const;
 		std::int64_t total_upload() const;
@@ -199,7 +204,7 @@ namespace libtorrent {
 		// never considered a connect candidate
 		bool web_seed:1;
 #if TORRENT_USE_ASSERTS
-		bool in_use:1;
+		bool in_use = true;
 #endif
 	};
 

include/libtorrent/torrent_peer_allocator.hpp

@@ -62,7 +62,11 @@ namespace libtorrent {
 	struct TORRENT_EXTRA_EXPORT torrent_peer_allocator final
 		: torrent_peer_allocator_interface
 	{
-		torrent_peer_allocator();
+#if TORRENT_USE_ASSERTS
+		~torrent_peer_allocator() {
+			m_in_use = false;
+		}
+#endif
 
 		torrent_peer* allocate_peer_entry(int type) override;
 		void free_peer_entry(torrent_peer* p) override;
@@ -79,22 +83,25 @@ namespace libtorrent {
 		// to have tens of thousands of peers, and a pool
 		// saves significant overhead
 
-		boost::pool<> m_ipv4_peer_pool;
+		boost::pool<> m_ipv4_peer_pool{sizeof(libtorrent::ipv4_peer), 500};
 #if TORRENT_USE_IPV6
-		boost::pool<> m_ipv6_peer_pool;
+		boost::pool<> m_ipv6_peer_pool{sizeof(libtorrent::ipv6_peer), 500};
 #endif
 #if TORRENT_USE_I2P
-		boost::pool<> m_i2p_peer_pool;
+		boost::pool<> m_i2p_peer_pool{sizeof(libtorrent::i2p_peer), 500};
 #endif
 
 		// the total number of bytes allocated (cumulative)
-		std::uint64_t m_total_bytes;
+		std::uint64_t m_total_bytes = 0;
 		// the total number of allocations (cumulative)
-		std::uint64_t m_total_allocations;
+		std::uint64_t m_total_allocations = 0;
 		// the number of currently live bytes
-		int m_live_bytes;
+		int m_live_bytes = 0;
 		// the number of currently live allocations
-		int m_live_allocations;
+		int m_live_allocations = 0;
+#if TORRENT_USE_ASSERTS
+		bool m_in_use = true;
+#endif
 	};
 }
 

src/peer_list.cpp

@@ -125,11 +125,8 @@ namespace libtorrent {
 
 	peer_list::~peer_list()
 	{
-		for (peers_t::iterator i = m_peers.begin()
-			, end(m_peers.end()); i != end; ++i)
-		{
-			m_peer_allocator.free_peer_entry(*i);
-		}
+		for (auto const p : m_peers)
+			m_peer_allocator.free_peer_entry(p);
 	}
 
 	void peer_list::set_max_failcount(torrent_state* state)
@@ -295,11 +292,6 @@ namespace libtorrent {
 		std::vector<torrent_peer*>::iterator ci = std::find(m_candidate_cache.begin(), m_candidate_cache.end(), *i);
 		if (ci != m_candidate_cache.end()) m_candidate_cache.erase(ci);
 
-#if TORRENT_USE_ASSERTS
-		TORRENT_ASSERT((*i)->in_use);
-		(*i)->in_use = false;
-#endif
-
 		m_peer_allocator.free_peer_entry(*i);
 		m_peers.erase(i);
 	}
@@ -765,10 +757,6 @@ namespace libtorrent {
 #endif
 				new (p) ipv4_peer(c.remote(), false, {});
 
-#if TORRENT_USE_ASSERTS
-			p->in_use = true;
-#endif
-
 			iter = m_peers.insert(iter, p);
 
 			if (m_round_robin >= iter - m_peers.begin()) ++m_round_robin;
@@ -1036,16 +1024,8 @@ namespace libtorrent {
 		if (p == nullptr) return nullptr;
 		new (p) i2p_peer(destination, true, src);
 
-#if TORRENT_USE_ASSERTS
-		p->in_use = true;
-#endif
-
 		if (!insert_peer(p, iter, flags, state))
 		{
-#if TORRENT_USE_ASSERTS
-			p->in_use = false;
-#endif
-
 			m_peer_allocator.free_peer_entry(p);
 			return nullptr;
 		}
@@ -1113,16 +1093,16 @@ namespace libtorrent {
 #endif
 				new (p) ipv4_peer(remote, true, src);
 
-#if TORRENT_USE_ASSERTS
-			p->in_use = true;
-#endif
-
-			if (!insert_peer(p, iter, flags, state))
+			try
+			{
+				if (!insert_peer(p, iter, flags, state))
+				{
+					m_peer_allocator.free_peer_entry(p);
+					return nullptr;
+				}
+			}
+			catch (std::exception const&)
 			{
-#if TORRENT_USE_ASSERTS
-				p->in_use = false;
-#endif
-// TODO: 3 this is not exception safe!
 				m_peer_allocator.free_peer_entry(p);
 				return nullptr;
 			}

src/torrent_peer.cpp

@@ -177,13 +177,11 @@ namespace libtorrent {
 		, confirmed_supports_utp(false)
 		, supports_holepunch(false)
 		, web_seed(false)
-#if TORRENT_USE_ASSERTS
-		, in_use(false)
-#endif
 	{}
 
 	std::uint32_t torrent_peer::rank(external_ip const& external, int external_port) const
 	{
+		TORRENT_ASSERT(in_use);
 //TODO: how do we deal with our external address changing?
 		if (peer_rank == 0)
 			peer_rank = peer_priority(
@@ -195,6 +193,7 @@ namespace libtorrent {
 #ifndef TORRENT_DISABLE_LOGGING
 	std::string torrent_peer::to_string() const
 	{
+		TORRENT_ASSERT(in_use);
 #if TORRENT_USE_I2P
 		if (is_i2p_addr) return dest().to_string();
 #endif // TORRENT_USE_I2P
@@ -205,6 +204,7 @@ namespace libtorrent {
 
 	std::int64_t torrent_peer::total_download() const
 	{
+		TORRENT_ASSERT(in_use);
 		if (connection != nullptr)
 		{
 			TORRENT_ASSERT(prev_amount_download == 0);
@@ -218,6 +218,7 @@ namespace libtorrent {
 
 	std::int64_t torrent_peer::total_upload() const
 	{
+		TORRENT_ASSERT(in_use);
 		if (connection != nullptr)
 		{
 			TORRENT_ASSERT(prev_amount_upload == 0);

src/torrent_peer_allocator.cpp

@@ -37,23 +37,9 @@ POSSIBILITY OF SUCH DAMAGE.
 
 namespace libtorrent {
 
-	torrent_peer_allocator::torrent_peer_allocator()
-		: m_ipv4_peer_pool(sizeof(libtorrent::ipv4_peer), 500)
-#if TORRENT_USE_IPV6
-		, m_ipv6_peer_pool(sizeof(libtorrent::ipv6_peer), 500)
-#endif
-#if TORRENT_USE_I2P
-		, m_i2p_peer_pool(sizeof(libtorrent::i2p_peer), 500)
-#endif
-		, m_total_bytes(0)
-		, m_total_allocations(0)
-		, m_live_bytes(0)
-		, m_live_allocations(0)
-	{
-	}
-
 	torrent_peer* torrent_peer_allocator::allocate_peer_entry(int type)
 	{
+		TORRENT_ASSERT(m_in_use);
 		torrent_peer* p = nullptr;
 		switch(type)
 		{
@@ -94,6 +80,8 @@ namespace libtorrent {
 
 	void torrent_peer_allocator::free_peer_entry(torrent_peer* p)
 	{
+		TORRENT_ASSERT(m_in_use);
+		TORRENT_ASSERT(p->in_use);
 #if TORRENT_USE_IPV6
 		if (p->is_v6_addr)
 		{