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

@@ -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);
     }

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;
 

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

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;

libtransmission/session.cc

@@ -724,7 +724,7 @@ void tr_session::initImpl(init_data& data)
 
     tr_sessionSet(this, &settings);
 
-    this->udp_core_ = std::make_unique<tr_session::tr_udp_core>(*this);
+    this->udp_core_ = std::make_unique<tr_session::tr_udp_core>(*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<tr_session::tr_udp_core>(*session);
+            session->udp_core_ = std::make_unique<tr_session::tr_udp_core>(*session, session->udpPort());
         });
 }
 

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;
 

libtransmission/tr-dht.cc

@@ -593,16 +593,6 @@ void tr_dhtUninit()
     impl = {};
 }
 
-std::optional<tr_port> 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())

libtransmission/tr-dht.h

@@ -8,7 +8,6 @@
 #error only libtransmission should #include this header.
 #endif
 
-#include <optional>
 #include <string_view>
 
 #include "transmission.h"
@@ -20,8 +19,6 @@ void tr_dhtUninit();
 
 bool tr_dhtEnabled();
 
-std::optional<tr_port> 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);

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)
     {

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;
         }