only post alerts for newly opened listen sockets, and only attempt to map ports for newly opened sockets. track has_incoming_connections per listen socket

ChangeLog

@@ -1,3 +1,4 @@
+	* when updating listen sockets, only post alerts for new ones
 	* deprecate anonymous_mode_alert
 	* deprecated force_proxy setting (when set, the proxy is always used)
 	* add support for Port Control Protocol (PCP)

include/libtorrent/aux_/session_impl.hpp

@@ -207,6 +207,10 @@ namespace aux {
 		// the key is an id that is used to identify the
 		// client with the tracker only.
 		std::uint32_t tracker_key = 0;
+
+		// set to true when we receive an incoming connection from this listen
+		// socket
+		bool incoming_connection = false;
 	};
 
 		struct TORRENT_EXTRA_EXPORT listen_endpoint_t

src/session_impl.cpp

@@ -1706,7 +1706,7 @@ namespace aux {
 		if (ec || m_abort || !m_ip_notifier) return;
 		m_ip_notifier->async_wait([this] (error_code const& e)
 			{ this->wrap(&session_impl::on_ip_change, e); });
-		reopen_network_sockets(session_handle::reopen_map_ports);
+		reopen_network_sockets({});
 	}
 
 	void session_impl::interface_to_endpoints(std::string const& device, int const port
@@ -1768,7 +1768,6 @@ namespace aux {
 
 		TORRENT_ASSERT(!m_abort);
 
-		m_stats_counters.set_value(counters::has_incoming_connections, 0);
 		error_code ec;
 
 		if (m_abort) return;
@@ -1856,6 +1855,15 @@ namespace aux {
 			remove_iter = m_listen_sockets.erase(remove_iter);
 		}
 
+		// all sockets in there stayed the same. Only sockets after this point are
+		// new and should post alerts
+		auto const existing_sockets = m_listen_sockets.size();
+
+		m_stats_counters.set_value(counters::has_incoming_connections
+			, std::any_of(m_listen_sockets.begin(), m_listen_sockets.end()
+				, [](std::shared_ptr<listen_socket_t> const& l)
+				{ return l->incoming_connection; }));
+
 		// open new sockets on any endpoints that didn't match with
 		// an existing socket
 		for (auto const& ep : eps)
@@ -1873,11 +1881,6 @@ namespace aux {
 
 				TORRENT_ASSERT((s->incoming == duplex::accept_incoming) == bool(s->sock));
 				if (s->sock) async_accept(s->sock, s->ssl);
-				if (m_settings.get_bool(settings_pack::enable_natpmp))
-					start_natpmp(*s);
-				// since this is a new socket it needs to map ports
-				// even if the caller did not request re-mapping
-				if (!map_ports) remap_ports(remap_upnp, *s);
 			}
 		}
 
@@ -1889,12 +1892,14 @@ namespace aux {
 			return;
 		}
 
+		auto const new_sockets = span<std::shared_ptr<listen_socket_t>>(
+			m_listen_sockets).subspan(existing_sockets);
+
 		// now, send out listen_succeeded_alert for the listen sockets we are
 		// listening on
-		// TODO only post alerts for new sockets?
 		if (m_alerts.should_post<listen_succeeded_alert>())
 		{
-			for (auto const& l : m_listen_sockets)
+			for (auto const& l : new_sockets)
 			{
 				error_code err;
 				if (l->sock)
@@ -1936,12 +1941,23 @@ namespace aux {
 
 		ec.clear();
 
+		if (m_settings.get_bool(settings_pack::enable_natpmp))
+		{
+			for (auto const& s : new_sockets)
+				start_natpmp(*s);
+		}
+
 		if (map_ports)
 		{
 			for (auto const& s : m_listen_sockets)
-			{
 				remap_ports(remap_natpmp_and_upnp, *s);
-			}
+		}
+		else
+		{
+			// new sockets need to map ports even if the caller did not request
+			// re-mapping
+			for (auto const& s : new_sockets)
+				remap_ports(remap_natpmp_and_upnp, *s);
 		}
 
 #if TORRENT_USE_I2P
@@ -2368,6 +2384,10 @@ namespace aux {
 #endif
 			m_utp_socket_manager;
 
+		auto listen_socket = ls.lock();
+		if (listen_socket)
+			listen_socket->incoming_connection = true;
+
 		for (;;)
 		{
 			aux::array<udp_socket::packet, 50> p;
@@ -2404,7 +2424,6 @@ namespace aux {
 #ifndef TORRENT_DISABLE_DHT
 					if (m_dht && buf.size() > 20 && buf.front() == 'd' && buf.back() == 'e')
 					{
-						auto listen_socket = ls.lock();
 						if (listen_socket)
 							handled = m_dht->incoming_packet(listen_socket, packet.from, buf);
 					}
@@ -2531,7 +2550,7 @@ namespace aux {
 		error_code ec;
 		if (e)
 		{
-			tcp::endpoint ep = listener->local_endpoint(ec);
+			tcp::endpoint const ep = listener->local_endpoint(ec);
 #ifndef TORRENT_DISABLE_LOGGING
 			if (should_log())
 			{
@@ -2601,6 +2620,12 @@ namespace aux {
 		if (m_settings.get_int(settings_pack::proxy_type) != settings_pack::none)
 			return;
 
+		auto listen = std::find_if(m_listen_sockets.begin(), m_listen_sockets.end()
+			, [&listener](std::shared_ptr<listen_socket_t> const& l)
+		{ return l->sock == listener; });
+		if (listen != m_listen_sockets.end())
+			(*listen)->incoming_connection = true;
+
 #ifdef TORRENT_USE_OPENSSL
 		if (ssl == transport::ssl)
 		{
@@ -5513,7 +5538,6 @@ namespace aux {
 			s.natpmp_mapper = std::make_shared<natpmp>(m_io_service, *this);
 			s.natpmp_mapper->start(s.local_endpoint.address(), s.device);
 		}
-		remap_ports(remap_natpmp, s);
 	}
 
 	namespace {
@@ -6666,7 +6690,10 @@ namespace aux {
 	{
 		INVARIANT_CHECK;
 		for (auto& s : m_listen_sockets)
+		{
 			start_natpmp(*s);
+			remap_ports(remap_natpmp, *s);
+		}
 	}
 
 	upnp* session_impl::start_upnp()

test/test_session.cpp

@@ -548,11 +548,11 @@ TORRENT_TEST(reopen_network_sockets)
 
 	s.reopen_network_sockets(session_handle::reopen_map_ports);
 
-	TEST_CHECK(count_alerts(s, 2, 4));
+	TEST_CHECK(count_alerts(s, 0, 4));
 
-	s.reopen_network_sockets(reopen_network_flags_t{0});
+	s.reopen_network_sockets({});
 
-	TEST_CHECK(count_alerts(s, 2, 0));
+	TEST_CHECK(count_alerts(s, 0, 0));
 
 	p.set_bool(settings_pack::enable_upnp, false);
 	p.set_bool(settings_pack::enable_natpmp, false);
@@ -560,7 +560,7 @@ TORRENT_TEST(reopen_network_sockets)
 
 	s.reopen_network_sockets(session_handle::reopen_map_ports);
 
-	TEST_CHECK(count_alerts(s, 2, 0));
+	TEST_CHECK(count_alerts(s, 0, 0));
 }
 #endif