From f9dde8240fcf8cfd8b1a89e5ec2e6d2c0113686f Mon Sep 17 00:00:00 2001 From: Maxime THIEBAUT <46688461+0xThiebaut@users.noreply.github.com> Date: Fri, 25 Oct 2024 23:06:18 +0200 Subject: [PATCH 1/3] Add announce_port support --- ChangeLog | 1 + include/libtorrent/session_handle.hpp | 1 + include/libtorrent/settings_pack.hpp | 14 +++++ simulation/test_tracker.cpp | 84 +++++++++++++++++++++++++++ src/session_impl.cpp | 8 ++- src/settings_pack.cpp | 3 +- src/torrent.cpp | 10 ++-- 7 files changed, 113 insertions(+), 8 deletions(-) diff --git a/ChangeLog b/ChangeLog index 6b4e8d1cfe6..0a68b98147a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -15,6 +15,7 @@ * fix integer overflow in piece picker * torrent_status::num_pieces counts pieces passed hash check, as documented * check settings_pack::max_out_request_queue before performance alert + * add announce_port setting to override the port announced to trackers 2.0.10 released diff --git a/include/libtorrent/session_handle.hpp b/include/libtorrent/session_handle.hpp index bb81f7faa91..e90d4c8b462 100644 --- a/include/libtorrent/session_handle.hpp +++ b/include/libtorrent/session_handle.hpp @@ -507,6 +507,7 @@ namespace libtorrent { // specified info-hash, advertising the specified port. If the port is // left at its default, 0, the port will be implied by the DHT message's // source port (which may improve connectivity through a NAT). + // ``dht_announce()`` is not affected by the ``announce_port`` override setting. // // Both these functions are exposed for advanced custom use of the DHT. // All torrents eligible to be announce to the DHT will be automatically, diff --git a/include/libtorrent/settings_pack.hpp b/include/libtorrent/settings_pack.hpp index 0446f2b60d6..8fd5e236038 100644 --- a/include/libtorrent/settings_pack.hpp +++ b/include/libtorrent/settings_pack.hpp @@ -2068,6 +2068,20 @@ namespace aux { i2p_inbound_length, i2p_outbound_length, + // ``announce_port`` is the port passed along as the ``port`` parameter + // to remote trackers such as HTTP or DHT. This setting does not affect + // the effective listening port nor local service discovery announcements. + // If left as zero (default), the listening port value is used. + // + // .. note:: + // This setting is only meant for very special cases where a + // seed's listening port differs from the external port. As an + // example, if a local proxy is used and that the proxy supports + // reverse tunnels through NAT-PMP, the tracker must connect to + // the external NAT-PMP port (configured using ``announce_port``) + // instead of the actual local listening port. + announce_port, + max_int_setting_internal }; diff --git a/simulation/test_tracker.cpp b/simulation/test_tracker.cpp index 6dabfbc326a..ac776c9aed7 100644 --- a/simulation/test_tracker.cpp +++ b/simulation/test_tracker.cpp @@ -361,6 +361,90 @@ void on_alert_notify(lt::session* ses) }); } +void test_announce() +{ + using sim::asio::ip::address_v4; + sim::default_config network_cfg; + sim::simulation sim{network_cfg}; + + sim::asio::io_context web_server(sim, make_address_v4("2.2.2.2")); + + // listen on port 8080 + sim::http_server http(web_server, 8080); + + int announces = 0; + + // expect announced IP & port + std::string const expect_port = "&port=1234"; + std::string const expect_ip = "&ip=1.2.3.4"; + + http.register_handler("/announce" + , [&announces, expect_port, expect_ip](std::string method, std::string req + , std::map&) + { + ++announces; + TEST_EQUAL(method, "GET"); + TEST_CHECK(req.find(expect_port) != std::string::npos); + TEST_CHECK(req.find(expect_ip) != std::string::npos); + char response[500]; + int const size = std::snprintf(response, sizeof(response), "d8:intervali1800e5:peers0:e"); + return sim::send_response(200, "OK", size) + response; + }); + + { + lt::session_proxy zombie; + + std::vector ips; + ips.push_back(make_address("123.0.0.3")); + + asio::io_context ios(sim, ips); + lt::settings_pack sett = settings(); + sett.set_str(settings_pack::listen_interfaces, "0.0.0.0:6881"); + sett.set_str(settings_pack::announce_ip, "1.2.3.4"); + sett.set_int(settings_pack::announce_port, 1234); + + auto ses = std::make_unique(sett, ios); + + ses->set_alert_notify(std::bind(&on_alert_notify, ses.get())); + + lt::add_torrent_params p; + p.name = "test-torrent"; + p.save_path = "."; + p.info_hashes.v1.assign("abababababababababab"); + + p.trackers.push_back("http://2.2.2.2:8080/announce"); + ses->async_add_torrent(p); + + // stop the torrent 5 seconds in + sim::timer t1(sim, lt::seconds(5) + , [&ses](boost::system::error_code const&) + { + std::vector torrents = ses->get_torrents(); + for (auto const& t : torrents) + { + t.pause(); + } + }); + + // then shut down 10 seconds in + sim::timer t2(sim, lt::seconds(10) + , [&ses,&zombie](boost::system::error_code const&) + { + zombie = ses->abort(); + ses.reset(); + }); + + sim.run(); + } + + TEST_EQUAL(announces, 2); +} + +// this test makes sure that a seed can overwrite its announced IP & port +TORRENT_TEST(announce_ip_port) { + test_announce(); +} + static const int num_interfaces = 3; void test_ipv6_support(char const* listen_interfaces diff --git a/src/session_impl.cpp b/src/session_impl.cpp index b1efa6d1e3e..fe8598524ff 100644 --- a/src/session_impl.cpp +++ b/src/session_impl.cpp @@ -1301,9 +1301,11 @@ namespace { #endif req.ssl_ctx = &m_ssl_ctx; #endif - - auto ls = req.outgoing_socket.get(); - if (ls) + if (const auto announce_port = std::uint16_t(m_settings.get_int(settings_pack::announce_port))) + { + req.listen_port = announce_port; + } + else if (auto ls = req.outgoing_socket.get()) { req.listen_port = #ifdef TORRENT_SSL_PEERS diff --git a/src/settings_pack.cpp b/src/settings_pack.cpp index 5a2c20f7405..72d0a886fd9 100644 --- a/src/settings_pack.cpp +++ b/src/settings_pack.cpp @@ -401,7 +401,8 @@ constexpr int DISK_WRITE_MODE = settings_pack::enable_os_cache; SET(i2p_inbound_quantity, 3, nullptr), SET(i2p_outbound_quantity, 3, nullptr), SET(i2p_inbound_length, 3, nullptr), - SET(i2p_outbound_length, 3, nullptr) + SET(i2p_outbound_length, 3, nullptr), + SET(announce_port, 0, nullptr) }}); #undef SET diff --git a/src/torrent.cpp b/src/torrent.cpp index d47a5ab94b9..7a21969091d 100644 --- a/src/torrent.cpp +++ b/src/torrent.cpp @@ -2807,15 +2807,17 @@ bool is_downloading_state(int const st) // If this is an SSL torrent the announce needs to specify an SSL // listen port. DHT nodes only operate on non-SSL ports so SSL // torrents cannot use implied_port. - // if we allow incoming uTP connections, set the implied_port - // argument in the announce, this will make the DHT node use + // if we allow incoming uTP connections and don't overwrite + // the announced port, set the implied_port argument + // in the announce, this will make the DHT node use // our source port in the packet as our listen port, which is // likely more accurate when behind a NAT + const auto announce_port = std::uint16_t(settings().get_int(settings_pack::announce_port)); if (is_ssl_torrent()) { flags |= dht::announce::ssl_torrent; } - else if (settings().get_bool(settings_pack::enable_incoming_utp)) + else if (!announce_port && settings().get_bool(settings_pack::enable_incoming_utp)) { flags |= dht::announce::implied_port; } @@ -2823,7 +2825,7 @@ bool is_downloading_state(int const st) std::weak_ptr self(shared_from_this()); m_torrent_file->info_hashes().for_each([&](sha1_hash const& ih, protocol_version v) { - m_ses.dht()->announce(ih, 0, flags + m_ses.dht()->announce(ih, announce_port, flags , std::bind(&torrent::on_dht_announce_response_disp, self, v, _1)); }); } From bb08bdbd8c8ae2355ec79d55de090f894eb6e451 Mon Sep 17 00:00:00 2001 From: Arvid Norberg Date: Sat, 9 Nov 2024 17:15:42 +0100 Subject: [PATCH 2/3] fix race condition when cancelling requests after becoming a seed --- ChangeLog | 1 + src/peer_connection.cpp | 7 +++++-- src/torrent.cpp | 6 ++++++ 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index 0a68b98147a..5a5b4c350a0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,6 +1,7 @@ 2.0.11 not released + * fix race condition when cancelling requests after becoming a seed * fix performance bug in the file pool, evicting MRU instead of LRU (HanabishiRecca) * fix bug where file_progress could sometimes be reported as >100% * don't hint FADV_RANDOM on posix systems. May improve seeding performance diff --git a/src/peer_connection.cpp b/src/peer_connection.cpp index d3a1278eafc..0888a32eb73 100644 --- a/src/peer_connection.cpp +++ b/src/peer_connection.cpp @@ -3200,9 +3200,11 @@ namespace libtorrent { { // if any other peer has a busy request to this block, we need // to cancel it too - t->cancel_block(block_finished); if (t->has_picker()) + { + t->cancel_block(block_finished); t->picker().write_failed(block_finished); + } if (t->has_storage()) { @@ -3751,6 +3753,7 @@ namespace libtorrent { TORRENT_ASSERT(block.piece_index != piece_block::invalid.piece_index); TORRENT_ASSERT(block.piece_index < t->torrent_file().end_piece()); TORRENT_ASSERT(block.block_index < t->torrent_file().piece_size(block.piece_index)); + TORRENT_ASSERT(t->has_picker()); // if all the peers that requested this block has been // cancelled, then just ignore the cancel. @@ -4034,7 +4037,7 @@ namespace libtorrent { INVARIANT_CHECK; std::shared_ptr t = m_torrent.lock(); - TORRENT_ASSERT(t); + if (!t) return; if (m_disconnecting) return; diff --git a/src/torrent.cpp b/src/torrent.cpp index 7a21969091d..90a5408b3b3 100644 --- a/src/torrent.cpp +++ b/src/torrent.cpp @@ -5094,6 +5094,10 @@ namespace { #ifndef TORRENT_DISABLE_STREAMING void torrent::cancel_non_critical() { + // if we don't have a piece picker, there's nothing to cancel. + // e.g. We may have become a seed already. + if (!has_picker()) return; + std::set time_critical; for (auto const& p : m_time_critical_pieces) time_critical.insert(p.piece); @@ -5989,6 +5993,8 @@ namespace { { INVARIANT_CHECK; + TORRENT_ASSERT(has_picker()); + for (auto p : m_connections) { TORRENT_INCREMENT(m_iterating_connections); From 4b4003d0fdc09a257a0841ad965b22533ed87a0d Mon Sep 17 00:00:00 2001 From: Arvid Norberg Date: Mon, 11 Nov 2024 19:43:15 +0100 Subject: [PATCH 3/3] abort_download() is OK in any state --- src/piece_picker.cpp | 2 -- test/test_piece_picker.cpp | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/piece_picker.cpp b/src/piece_picker.cpp index e7e10040f3a..6e81a0fc178 100644 --- a/src/piece_picker.cpp +++ b/src/piece_picker.cpp @@ -3829,8 +3829,6 @@ namespace { TORRENT_ASSERT(info.peer == nullptr || info.peer->in_use); TORRENT_ASSERT(info.piece_index == block.piece_index); - TORRENT_ASSERT(info.state != block_info::state_none); - if (info.state != block_info::state_requested) return; piece_pos const& p = m_piece_map[block.piece_index]; diff --git a/test/test_piece_picker.cpp b/test/test_piece_picker.cpp index ddee1db7464..d643c9f3e44 100644 --- a/test/test_piece_picker.cpp +++ b/test/test_piece_picker.cpp @@ -324,6 +324,38 @@ TORRENT_TEST(piece_block) TEST_CHECK(!(piece_block(zero, 1) < piece_block(zero, 1))); } +TORRENT_TEST(abort_download_states) +{ + auto p = setup_picker("1111111", " ", "7110000", ""); + + //aborting a block that isn't being downloaded is a no-op + TEST_CHECK(p->is_requested({3_piece, 0}) == false); + p->abort_download({3_piece, 0}, tmp_peer); + p->abort_download({3_piece, 1}, tmp_peer); + TEST_CHECK(p->is_requested({3_piece, 0}) == false); + + // aborting a block that's downloading + p->mark_as_downloading({3_piece, 0}, tmp_peer); + TEST_CHECK(p->is_downloaded({3_piece, 0}) == false); + p->abort_download({3_piece, 0}, tmp_peer); + p->abort_download({3_piece, 1}, tmp_peer); + TEST_CHECK(p->is_downloaded({3_piece, 0}) == false); + + // aborting a block that's finished is a no-op + p->mark_as_writing({3_piece, 0}, tmp_peer); + TEST_CHECK(p->is_downloaded({3_piece, 0}) == true); + p->abort_download({3_piece, 0}, tmp_peer); + p->abort_download({3_piece, 1}, tmp_peer); + TEST_CHECK(p->is_downloaded({3_piece, 0}) == true); + + // aborting a block that's written is a no-op + p->mark_as_finished({3_piece, 0}, tmp_peer); + TEST_CHECK(p->is_finished({3_piece, 0}) == true); + p->abort_download({3_piece, 0}, tmp_peer); + p->abort_download({3_piece, 1}, tmp_peer); + TEST_CHECK(p->is_finished({3_piece, 0}) == true); +} + TORRENT_TEST(abort_download) { // test abort_download