From dcb8b816a3009f009ecf3c198d2fe6d42caa16b8 Mon Sep 17 00:00:00 2001 From: Arvid Norberg Date: Sun, 22 Jul 2018 00:01:45 +0200 Subject: [PATCH] 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 + include/libtorrent/aux_/session_impl.hpp | 4 ++ src/session_impl.cpp | 55 ++++++++++++++++++------ test/test_session.cpp | 8 ++-- 4 files changed, 50 insertions(+), 18 deletions(-) diff --git a/ChangeLog b/ChangeLog index f738d31e7..eccf3f7e0 100644 --- a/ChangeLog +++ b/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) diff --git a/include/libtorrent/aux_/session_impl.hpp b/include/libtorrent/aux_/session_impl.hpp index 95059ed31..09ad35dee 100644 --- a/include/libtorrent/aux_/session_impl.hpp +++ b/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 diff --git a/src/session_impl.cpp b/src/session_impl.cpp index acd07dad2..9d022f3e4 100644 --- a/src/session_impl.cpp +++ b/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 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>( + 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()) { - 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 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 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(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() diff --git a/test/test_session.cpp b/test/test_session.cpp index 63bcc2eb3..0aba7ad43 100644 --- a/test/test_session.cpp +++ b/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