From 79068c512a1a652d779988acf3977796daaf2d3e Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Sat, 15 Oct 2022 08:22:43 -0500 Subject: [PATCH] refactor: decouple tr-dht from peerMsgs, peerMgr (#3966) * refactor: decouple peer-mgr from tr-dht * refactor: remove tr_dhtPort() * refactor: decouple peer-msgs from tr-dht * refactor: make tr_udp_core.udp_port_ const * refactor: rename tr_udp_core::dhtUninit() as startShutdown() --- libtransmission/handshake.cc | 2 +- libtransmission/handshake.h | 2 +- libtransmission/peer-mgr.cc | 5 ++-- libtransmission/peer-msgs.cc | 9 +++---- libtransmission/session.cc | 6 ++--- libtransmission/session.h | 35 +++++++++++++------------ libtransmission/tr-dht.cc | 10 ------- libtransmission/tr-dht.h | 3 --- libtransmission/tr-udp.cc | 15 +++++++---- tests/libtransmission/handshake-test.cc | 2 +- 10 files changed, 40 insertions(+), 49 deletions(-) diff --git a/libtransmission/handshake.cc b/libtransmission/handshake.cc index 4a48a4786..32740c280 100644 --- a/libtransmission/handshake.cc +++ b/libtransmission/handshake.cc @@ -237,7 +237,7 @@ static bool buildHandshakeMessage(tr_handshake const* const handshake, uint8_t* /* Note that this doesn't depend on whether the torrent is private. * We don't accept DHT peers for a private torrent, * but we participate in the DHT regardless. */ - if (handshake->mediator->isDHTEnabled()) + if (handshake->mediator->allowsDHT()) { HANDSHAKE_SET_DHT(walk); } diff --git a/libtransmission/handshake.h b/libtransmission/handshake.h index 97bec01fc..986393802 100644 --- a/libtransmission/handshake.h +++ b/libtransmission/handshake.h @@ -61,7 +61,7 @@ public: [[nodiscard]] virtual libtransmission::TimerMaker& timerMaker() = 0; - [[nodiscard]] virtual bool isDHTEnabled() const = 0; + [[nodiscard]] virtual bool allowsDHT() const = 0; [[nodiscard]] virtual bool allowsTCP() const = 0; diff --git a/libtransmission/peer-mgr.cc b/libtransmission/peer-mgr.cc index 8a8d40c8f..6497ba927 100644 --- a/libtransmission/peer-mgr.cc +++ b/libtransmission/peer-mgr.cc @@ -44,7 +44,6 @@ #include "timer.h" #include "torrent.h" #include "tr-assert.h" -#include "tr-dht.h" #include "tr-utp.h" #include "utils.h" #include "webseed.h" @@ -102,9 +101,9 @@ public: return torrentInfo(tr_torrentFindFromObfuscatedHash(&session_, obfuscated_info_hash)); } - [[nodiscard]] bool isDHTEnabled() const override + [[nodiscard]] bool allowsDHT() const override { - return tr_dhtEnabled(); + return session_.allowsDHT(); } [[nodiscard]] bool allowsTCP() const override diff --git a/libtransmission/peer-msgs.cc b/libtransmission/peer-msgs.cc index e3443ab06..cda6b310b 100644 --- a/libtransmission/peer-msgs.cc +++ b/libtransmission/peer-msgs.cc @@ -37,7 +37,6 @@ #include "torrent-magnet.h" #include "torrent.h" #include "tr-assert.h" -#include "tr-dht.h" #include "utils.h" #include "variant.h" #include "version.h" @@ -265,7 +264,7 @@ public: { if (torrent->allowsPex()) { - pex_timer_ = torrent->session->timerMaker().create([this]() { sendPex(); }); + pex_timer_ = session->timerMaker().create([this]() { sendPex(); }); pex_timer_->startRepeating(SendPexInterval); } @@ -282,12 +281,12 @@ public: tellPeerWhatWeHave(this); - if (auto const port = tr_dhtPort(); io->supportsDHT() && port.has_value()) + if (session->allowsDHT() && io->supportsDHT()) { // only send PORT over IPv6 iff IPv6 DHT is running (BEP-32). if (io->address().isIPv4() || tr_globalIPv6(nullptr).has_value()) { - protocolSendPort(this, *port); + protocolSendPort(this, session->udpPort()); } } @@ -1693,7 +1692,7 @@ static ReadState readBtMessage(tr_peerMsgsImpl* msgs, size_t inlen) if (auto const dht_port = tr_port::fromNetwork(nport); !std::empty(dht_port)) { msgs->dht_port = dht_port; - tr_dhtAddNode(msgs->io->address(), msgs->dht_port, false); + msgs->session->addDhtNode(msgs->io->address(), msgs->dht_port); } } break; diff --git a/libtransmission/session.cc b/libtransmission/session.cc index 31459e141..5b76900cd 100644 --- a/libtransmission/session.cc +++ b/libtransmission/session.cc @@ -724,7 +724,7 @@ void tr_session::initImpl(init_data& data) tr_sessionSet(this, &settings); - this->udp_core_ = std::make_unique(*this); + this->udp_core_ = std::make_unique(*this, udpPort()); this->web_ = tr_web::create(this->web_mediator_); @@ -1785,7 +1785,7 @@ void tr_session::closeImplStart() lpd_.reset(); - udp_core_->dhtUninit(); + udp_core_->startShutdown(); save_timer_.reset(); now_timer_.reset(); @@ -2057,7 +2057,7 @@ void tr_sessionSetDHTEnabled(tr_session* session, bool enabled) { session->udp_core_.reset(); session->is_dht_enabled_ = enabled; - session->udp_core_ = std::make_unique(*session); + session->udp_core_ = std::make_unique(*session, session->udpPort()); }); } diff --git a/libtransmission/session.h b/libtransmission/session.h index 2f49f7c63..3a0f63f3b 100644 --- a/libtransmission/session.h +++ b/libtransmission/session.h @@ -230,11 +230,11 @@ private: class tr_udp_core { public: - tr_udp_core(tr_session& session); + tr_udp_core(tr_session& session, tr_port udp_port); ~tr_udp_core(); - static void dhtUninit(); + static void startShutdown(); static void dhtUpkeep(); void set_socket_buffers(); @@ -247,23 +247,10 @@ private: void sendto(void const* buf, size_t buflen, struct sockaddr const* to, socklen_t const tolen) const; - [[nodiscard]] constexpr auto port() const noexcept - { - return udp_port_; - } - - [[nodiscard]] constexpr auto udp_socket() const noexcept - { - return udp_socket_; - } - - [[nodiscard]] constexpr auto udp6_socket() const noexcept - { - return udp6_socket_; - } + void addDhtNode(tr_address const& addr, tr_port port); private: - tr_port udp_port_ = {}; + tr_port const udp_port_; tr_session& session_; struct event* udp_event_ = nullptr; struct event* udp6_event_ = nullptr; @@ -613,6 +600,12 @@ public: return public_peer_port_; } + [[nodiscard]] constexpr tr_port udpPort() const noexcept + { + // uses the same port number that's used for incoming TCP connections + return public_peer_port_; + } + [[nodiscard]] constexpr auto queueEnabled(tr_direction dir) const noexcept { return queue_enabled_[dir]; @@ -823,6 +816,14 @@ public: void addTorrent(tr_torrent* tor); + void addDhtNode(tr_address const& addr, tr_port port) + { + if (udp_core_) + { + udp_core_->addDhtNode(addr, port); + } + } + private: [[nodiscard]] tr_port randomPort() const; diff --git a/libtransmission/tr-dht.cc b/libtransmission/tr-dht.cc index 436fe02a7..f2a48053b 100644 --- a/libtransmission/tr-dht.cc +++ b/libtransmission/tr-dht.cc @@ -593,16 +593,6 @@ void tr_dhtUninit() impl = {}; } -std::optional tr_dhtPort() -{ - if (impl.session == nullptr) - { - return {}; - } - - return impl.session->udp_core_->port(); -} - bool tr_dhtAddNode(tr_address addr, tr_port port, bool bootstrap) { if (!tr_dhtEnabled()) diff --git a/libtransmission/tr-dht.h b/libtransmission/tr-dht.h index 913f5c487..afcab6288 100644 --- a/libtransmission/tr-dht.h +++ b/libtransmission/tr-dht.h @@ -8,7 +8,6 @@ #error only libtransmission should #include this header. #endif -#include #include #include "transmission.h" @@ -20,8 +19,6 @@ void tr_dhtUninit(); bool tr_dhtEnabled(); -std::optional tr_dhtPort(); - bool tr_dhtAddNode(tr_address, tr_port, bool bootstrap); void tr_dhtUpkeep(); void tr_dhtCallback(unsigned char* buf, int buflen, struct sockaddr* from, socklen_t fromlen); diff --git a/libtransmission/tr-udp.cc b/libtransmission/tr-udp.cc index 8c6963289..14f472bf9 100644 --- a/libtransmission/tr-udp.cc +++ b/libtransmission/tr-udp.cc @@ -90,6 +90,11 @@ static void set_socket_buffers(tr_socket_t fd, bool large) } } +void tr_session::tr_udp_core::addDhtNode(tr_address const& addr, tr_port port) +{ + tr_dhtAddNode(addr, port, false); +} + void tr_session::tr_udp_core::set_socket_buffers() { bool const utp = session_.allowsUTP(); @@ -228,10 +233,10 @@ static void event_callback(evutil_socket_t s, [[maybe_unused]] short type, void* } } -tr_session::tr_udp_core::tr_udp_core(tr_session& session) - : session_{ session } +tr_session::tr_udp_core::tr_udp_core(tr_session& session, tr_port udp_port) + : udp_port_{ udp_port } + , session_{ session } { - udp_port_ = session_.peerPort(); if (std::empty(udp_port_)) { return; @@ -322,7 +327,7 @@ void tr_session::tr_udp_core::dhtUpkeep() } } -void tr_session::tr_udp_core::dhtUninit() +void tr_session::tr_udp_core::startShutdown() { if (tr_dhtEnabled()) { @@ -332,7 +337,7 @@ void tr_session::tr_udp_core::dhtUninit() tr_session::tr_udp_core::~tr_udp_core() { - dhtUninit(); + startShutdown(); if (udp_socket_ != TR_BAD_SOCKET) { diff --git a/tests/libtransmission/handshake-test.cc b/tests/libtransmission/handshake-test.cc index 6faf948ed..4f7d4828c 100644 --- a/tests/libtransmission/handshake-test.cc +++ b/tests/libtransmission/handshake-test.cc @@ -73,7 +73,7 @@ public: return session_->timerMaker(); } - [[nodiscard]] bool isDHTEnabled() const override + [[nodiscard]] bool allowsDHT() const override { return false; }