From 69b293a793ea8f27b23e086c020a4cca22cc8c67 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Tue, 24 Oct 2023 15:24:52 -0400 Subject: [PATCH] refactor: minor decoupling in peer-mgr (#6155) * refactor: minor decoupling in peer-mgr Pass a tr_torrents& and TimerMaker& into the tr_peerMgr and HandshakeMediator constructors so they can be used directly instead of via tr_session. No functional changes. * refactor: in HandshakeMediator, make the session reference const --- libtransmission/peer-mgr.cc | 60 ++++++++++++++++++++-------------- libtransmission/peer-socket.cc | 2 +- libtransmission/peer-socket.h | 2 +- libtransmission/torrent.cc | 13 -------- libtransmission/torrent.h | 2 -- libtransmission/torrents.cc | 13 ++++++++ libtransmission/torrents.h | 3 ++ 7 files changed, 53 insertions(+), 42 deletions(-) diff --git a/libtransmission/peer-mgr.cc b/libtransmission/peer-mgr.cc index bf7c8fb1c..70e4da5e4 100644 --- a/libtransmission/peer-mgr.cc +++ b/libtransmission/peer-mgr.cc @@ -79,19 +79,25 @@ private: } public: - explicit HandshakeMediator(tr_session& session) noexcept + explicit HandshakeMediator( + tr_session const& session, + libtransmission::TimerMaker& timer_maker, + tr_torrents& torrents) noexcept : session_{ session } + , timer_maker_{ timer_maker } + , torrents_{ torrents } { } [[nodiscard]] std::optional torrent(tr_sha1_digest_t const& info_hash) const override { - return torrent(session_.torrents().get(info_hash)); + return torrent(torrents_.get(info_hash)); } - [[nodiscard]] std::optional torrent_from_obfuscated(tr_sha1_digest_t const& info_hash) const override + [[nodiscard]] std::optional torrent_from_obfuscated( + tr_sha1_digest_t const& obfuscated_info_hash) const override { - return torrent(tr_torrentFindFromObfuscatedHash(&session_, info_hash)); + return torrent(torrents_.find_from_obfuscated_hash(obfuscated_info_hash)); } [[nodiscard]] bool allows_dht() const override @@ -108,7 +114,7 @@ public: [[nodiscard]] libtransmission::TimerMaker& timer_maker() override { - return session_.timerMaker(); + return timer_maker_; } [[nodiscard]] size_t pad(void* setme, size_t maxlen) const override @@ -119,7 +125,9 @@ public: } private: - tr_session& session_; + tr_session const& session_; + libtransmission::TimerMaker& timer_maker_; + tr_torrents& torrents_; }; using Handshakes = std::unordered_map; @@ -926,12 +934,13 @@ public: using OutboundCandidates = small:: max_size_vector, OutboundCandidateListCapacity>; - explicit tr_peerMgr(tr_session* session_in) + explicit tr_peerMgr(tr_session* session_in, libtransmission::TimerMaker& timer_maker, tr_torrents& torrents) : session{ session_in } - , handshake_mediator_{ *session } - , bandwidth_timer_{ session->timerMaker().create([this]() { bandwidthPulse(); }) } - , rechoke_timer_{ session->timerMaker().create([this]() { rechokePulseMarshall(); }) } - , refill_upkeep_timer_{ session->timerMaker().create([this]() { refillUpkeep(); }) } + , torrents_{ torrents } + , handshake_mediator_{ *session, timer_maker, torrents } + , bandwidth_timer_{ timer_maker.create([this]() { bandwidthPulse(); }) } + , rechoke_timer_{ timer_maker.create([this]() { rechokePulseMarshall(); }) } + , refill_upkeep_timer_{ timer_maker.create([this]() { refillUpkeep(); }) } , blocklist_tag_{ session->blocklist_changed_.observe([this]() { on_blocklist_changed(); }) } { bandwidth_timer_->start_repeating(BandwidthTimerPeriod); @@ -968,11 +977,12 @@ public: [[nodiscard]] tr_swarm* get_existing_swarm(tr_sha1_digest_t const& hash) const { - auto* const tor = session->torrents().get(hash); + auto* const tor = torrents_.get(hash); return tor == nullptr ? nullptr : tor->swarm; } tr_session* const session; + tr_torrents& torrents_; Handshakes incoming_handshakes; HandshakeMediator handshake_mediator_; @@ -988,7 +998,7 @@ private: { /* we cache whether or not a peer is blocklisted... since the blocklist has changed, erase that cached value */ - for (auto* const tor : session->torrents()) + for (auto* const tor : torrents_) { for (auto& pool : { std::ref(tor->swarm->connectable_pool), std::ref(tor->swarm->incoming_pool) }) { @@ -1030,7 +1040,7 @@ tr_peer::~tr_peer() tr_peerMgr* tr_peerMgrNew(tr_session* session) { - return new tr_peerMgr{ session }; + return new tr_peerMgr{ session, session->timerMaker(), session->torrents() }; } void tr_peerMgrFree(tr_peerMgr* manager) @@ -1159,7 +1169,7 @@ void tr_peerMgr::refillUpkeep() const { auto const lock = unique_lock(); - for (auto* const tor : session->torrents()) + for (auto* const tor : torrents_) { tor->swarm->cancelOldRequests(); tor->swarm->remove_inactive_peer_info(); @@ -1289,7 +1299,7 @@ void tr_peerMgrAddIncoming(tr_peerMgr* manager, tr_peer_socket&& socket) TR_ASSERT(manager->session != nullptr); auto const lock = manager->unique_lock(); - tr_session* session = manager->session; + auto* const session = manager->session; if (session->addressIsBlocked(socket.address())) { @@ -1986,7 +1996,7 @@ void tr_peerMgr::rechokePulse() const auto const lock = unique_lock(); auto const now = tr_time_msec(); - for (auto* const tor : session->torrents()) + for (auto* const tor : torrents_) { if (tor->is_running()) { @@ -2172,12 +2182,12 @@ void tr_peerMgr::reconnectPulse() { using namespace disconnect_helpers; - auto const lock = session->unique_lock(); + auto const lock = unique_lock(); auto const now_sec = tr_time(); // remove crappy peers auto bad_peers_buf = bad_peers_t{}; - auto& torrents = session->torrents(); + auto& torrents = torrents_; for (auto* const tor : torrents) { auto* const swarm = tor->swarm; @@ -2216,7 +2226,7 @@ namespace bandwidth_helpers { void pumpAllPeers(tr_peerMgr* mgr) { - for (auto* const tor : mgr->session->torrents()) + for (auto* const tor : mgr->torrents_) { for (auto* const peer : tor->swarm->peers) { @@ -2240,7 +2250,7 @@ void tr_peerMgr::bandwidthPulse() session->top_bandwidth_.allocate(Msec); // torrent upkeep - for (auto* const tor : session->torrents()) + for (auto* const tor : torrents_) { tor->do_idle_work(); tr_torrentMagnetDoIdleWork(tor); @@ -2506,13 +2516,13 @@ void tr_peerMgr::make_new_peer_connections() { using namespace connect_helpers; - auto const lock = session->unique_lock(); + auto const lock = unique_lock(); // get the candidates if we need to auto& candidates = outbound_candidates_; if (std::empty(candidates)) { - get_peer_candidates(session->peerLimit(), session->torrents(), candidates); + get_peer_candidates(session->peerLimit(), torrents_, candidates); } // initiate connections to the last N candidates @@ -2522,7 +2532,7 @@ void tr_peerMgr::make_new_peer_connections() { auto const& [tor_id, sock_addr] = *it; - if (auto* const tor = session->torrents().get(tor_id); tor != nullptr) + if (auto* const tor = torrents_.get(tor_id); tor != nullptr) { if (auto* const peer_info = tor->swarm->get_existing_peer_info(sock_addr); peer_info != nullptr) { @@ -2537,7 +2547,7 @@ void tr_peerMgr::make_new_peer_connections() void HandshakeMediator::set_utp_failed(tr_sha1_digest_t const& info_hash, tr_socket_address const& socket_address) { - if (auto* const tor = session_.torrents().get(info_hash); tor != nullptr) + if (auto* const tor = torrents_.get(info_hash); tor != nullptr) { if (auto* const peer_info = tor->swarm->get_existing_peer_info(socket_address); peer_info != nullptr) { diff --git a/libtransmission/peer-socket.cc b/libtransmission/peer-socket.cc index 5d39403b5..8c807c5e2 100644 --- a/libtransmission/peer-socket.cc +++ b/libtransmission/peer-socket.cc @@ -130,7 +130,7 @@ size_t tr_peer_socket::try_read(InBuf& buf, size_t max, [[maybe_unused]] bool bu return {}; } -bool tr_peer_socket::limit_reached(tr_session* const session) noexcept +bool tr_peer_socket::limit_reached(tr_session const* const session) noexcept { return n_open_sockets_.load() >= session->peerLimit(); } diff --git a/libtransmission/peer-socket.h b/libtransmission/peer-socket.h index 0a729580b..cbc610750 100644 --- a/libtransmission/peer-socket.h +++ b/libtransmission/peer-socket.h @@ -131,7 +131,7 @@ public: struct UTPSocket* utp; } handle = {}; - [[nodiscard]] static bool limit_reached(tr_session* session) noexcept; + [[nodiscard]] static bool limit_reached(tr_session const* session) noexcept; private: enum class Type diff --git a/libtransmission/torrent.cc b/libtransmission/torrent.cc index e83579e57..df30271d4 100644 --- a/libtransmission/torrent.cc +++ b/libtransmission/torrent.cc @@ -130,19 +130,6 @@ tr_torrent* tr_torrentFindFromMagnetLink(tr_session* session, char const* magnet return magnet_link == nullptr ? nullptr : session->torrents().get(magnet_link); } -tr_torrent* tr_torrentFindFromObfuscatedHash(tr_session* session, tr_sha1_digest_t const& obfuscated_hash) -{ - for (auto* const tor : session->torrents()) - { - if (tor->obfuscated_hash == obfuscated_hash) - { - return tor; - } - } - - return nullptr; -} - bool tr_torrentSetMetainfoFromFile(tr_torrent* tor, tr_torrent_metainfo const* metainfo, char const* filename) { if (tr_torrentHasMetadata(tor)) diff --git a/libtransmission/torrent.h b/libtransmission/torrent.h index 6a9fe2135..7e7b08b2e 100644 --- a/libtransmission/torrent.h +++ b/libtransmission/torrent.h @@ -64,8 +64,6 @@ bool tr_ctorGetIncompleteDir(tr_ctor const* ctor, char const** setme_incomplete_ void tr_torrentChangeMyPort(tr_torrent* tor); -[[nodiscard]] tr_torrent* tr_torrentFindFromObfuscatedHash(tr_session* session, tr_sha1_digest_t const& hash); - bool tr_torrentReqIsValid(tr_torrent const* tor, tr_piece_index_t index, uint32_t offset, uint32_t length); [[nodiscard]] tr_block_span_t tr_torGetFileBlockSpan(tr_torrent const* tor, tr_file_index_t file); diff --git a/libtransmission/torrents.cc b/libtransmission/torrents.cc index 10bc171be..1d1d30e3e 100644 --- a/libtransmission/torrents.cc +++ b/libtransmission/torrents.cc @@ -61,6 +61,19 @@ tr_torrent const* tr_torrents::get(tr_sha1_digest_t const& hash) const return begin == end ? nullptr : *begin; } +tr_torrent* tr_torrents::find_from_obfuscated_hash(tr_sha1_digest_t const& obfuscated_hash) +{ + for (auto* const tor : *this) + { + if (tor->obfuscated_hash == obfuscated_hash) + { + return tor; + } + } + + return nullptr; +} + tr_torrent_id_t tr_torrents::add(tr_torrent* tor) { auto const id = static_cast(std::size(by_id_)); diff --git a/libtransmission/torrents.h b/libtransmission/torrents.h index d591c5ce1..8ff149af5 100644 --- a/libtransmission/torrents.h +++ b/libtransmission/torrents.h @@ -53,6 +53,9 @@ public: return get(metainfo.info_hash()); } + // O(n)} + [[nodiscard]] tr_torrent* find_from_obfuscated_hash(tr_sha1_digest_t const& obfuscated_hash); + // These convenience functions use get(tr_sha1_digest_t const&) // after parsing the magnet link to get the info hash. If you have // the info hash already, use get() instead to avoid excess parsing.