diff --git a/libtransmission/ip-cache.cc b/libtransmission/ip-cache.cc index fa5e0abf9..3fc649c21 100644 --- a/libtransmission/ip-cache.cc +++ b/libtransmission/ip-cache.cc @@ -42,10 +42,6 @@ auto constexpr IPQueryServices = std::array{ std::array{ "https://ip4.transmissi auto constexpr UpkeepInterval = 30min; auto constexpr RetryUpkeepInterval = 30s; -// Normally there should only ever be 1 cache instance during the entire program execution -// This is a counter only to cater for SessionTest.honorsSettings -std::size_t cache_exists = 0; - } // namespace namespace @@ -156,8 +152,11 @@ tr_ip_cache::tr_ip_cache(Mediator& mediator_in) upkeep_timers_[i]->set_callback(cb); start_timer(type, UpkeepInterval); } +} - ++cache_exists; +std::shared_ptr tr_ip_cache::create(Mediator& mediator) +{ + return std::shared_ptr{ new tr_ip_cache{ mediator } }; } tr_ip_cache::~tr_ip_cache() @@ -175,8 +174,6 @@ tr_ip_cache::~tr_ip_cache() { tr_logAddDebug("Destructed while some global IP queries were pending."); } - - --cache_exists; } bool tr_ip_cache::try_shutdown() noexcept @@ -239,6 +236,7 @@ void tr_ip_cache::update_addr(tr_address_type type) noexcept void tr_ip_cache::update_global_addr(tr_address_type type) noexcept { auto const ix_service = ix_service_[type]; + TR_ASSERT(type < NUM_TR_AF_INET_TYPES); TR_ASSERT(has_ip_protocol_[type]); TR_ASSERT(ix_service < std::size(IPQueryServices[type])); @@ -249,18 +247,21 @@ void tr_ip_cache::update_global_addr(tr_address_type type) noexcept TR_ASSERT(is_updating_[type] == is_updating_t::YES); // Update global address - static auto constexpr IPProtocolMap = std::array{ tr_web::FetchOptions::IPProtocol::V4, - tr_web::FetchOptions::IPProtocol::V6 }; - auto options = tr_web::FetchOptions{ IPQueryServices[type][ix_service], - [this, type](tr_web::FetchResponse const& response) - { - // Check to avoid segfault - if (cache_exists != 0) - { - this->on_response_ip_query(type, response); - } - }, - nullptr }; + static auto constexpr IPProtocolMap = std::array{ + tr_web::FetchOptions::IPProtocol::V4, + tr_web::FetchOptions::IPProtocol::V6, + }; + auto options = tr_web::FetchOptions{ + IPQueryServices[type][ix_service], + [weak = weak_from_this(), type](tr_web::FetchResponse const& response) + { + if (auto const ptr = weak.lock()) + { + ptr->on_response_ip_query(type, response); + } + }, + nullptr, + }; options.ip_proto = IPProtocolMap[type]; options.sndbuf = 4096; options.rcvbuf = 4096; diff --git a/libtransmission/ip-cache.h b/libtransmission/ip-cache.h index ec045b732..bd71d81a2 100644 --- a/libtransmission/ip-cache.h +++ b/libtransmission/ip-cache.h @@ -34,7 +34,7 @@ * your system is capable in that IP protocol. And if the global address is * the same as the source address, then you are not behind a NAT. */ -class tr_ip_cache +class tr_ip_cache : public std::enable_shared_from_this { public: struct Mediator @@ -53,7 +53,7 @@ public: [[nodiscard]] virtual libtransmission::TimerMaker& timer_maker() = 0; }; - explicit tr_ip_cache(Mediator& mediator_in); + static std::shared_ptr create(Mediator& mediator); tr_ip_cache() = delete; ~tr_ip_cache(); @@ -96,6 +96,8 @@ private: template using array_ip_t = std::array; + explicit tr_ip_cache(Mediator& mediator_in); + void unset_global_addr(tr_address_type type) noexcept; void set_source_addr(tr_address const& addr_new) noexcept; void unset_addr(tr_address_type type) noexcept; diff --git a/libtransmission/session.cc b/libtransmission/session.cc index 8a388d7ea..3388a56a9 100644 --- a/libtransmission/session.cc +++ b/libtransmission/session.cc @@ -442,7 +442,7 @@ tr_address tr_session::bind_address(tr_address_type type) const noexcept { // if user provided an address, use it. // otherwise, use any_ipv4 (0.0.0.0). - return ip_cache_.bind_addr(type); + return ip_cache_->bind_addr(type); } if (type == TR_AF_INET6) @@ -790,11 +790,11 @@ void tr_session::setSettings(tr_session::Settings&& settings_in, bool force) if (auto const& val = new_settings.bind_address_ipv4; force || val != old_settings.bind_address_ipv4) { - ip_cache_.update_addr(TR_AF_INET); + ip_cache_->update_addr(TR_AF_INET); } if (auto const& val = new_settings.bind_address_ipv6; force || val != old_settings.bind_address_ipv6) { - ip_cache_.update_addr(TR_AF_INET6); + ip_cache_->update_addr(TR_AF_INET6); } if (auto const& val = new_settings.default_trackers_str; force || val != old_settings.default_trackers_str) @@ -1432,7 +1432,7 @@ void tr_session::closeImplPart1(std::promise* closed_promise, std::chrono: // ...since global_ip_cache_ relies on web_ to update global addresses, // we tell it to stop updating before web_ starts to refuse new requests. // But we keep it intact for now, so that udp_core_ can continue. - this->ip_cache_.try_shutdown(); + this->ip_cache_->try_shutdown(); // ...and now that those are done, tell web_ that we're shutting // down soon. This leaves the `event=stopped` going but refuses any // new tasks. @@ -1453,7 +1453,7 @@ void tr_session::closeImplPart2(std::promise* closed_promise, std::chrono: // all the &event=stopped tracker announces. // also wait for all ip cache updates to finish so that web_ can // safely destruct. - if ((!web_->is_idle() || !announcer_udp_->is_idle() || !ip_cache_.try_shutdown()) && + if ((!web_->is_idle() || !announcer_udp_->is_idle() || !ip_cache_->try_shutdown()) && std::chrono::steady_clock::now() < deadline) { announcer_->upkeep(); diff --git a/libtransmission/session.h b/libtransmission/session.h index a56872abf..469883b3f 100644 --- a/libtransmission/session.h +++ b/libtransmission/session.h @@ -1108,7 +1108,7 @@ public: [[nodiscard]] bool has_ip_protocol(tr_address_type type) const noexcept { TR_ASSERT(tr_address::is_valid(type)); - return ip_cache_.has_ip_protocol(type); + return ip_cache_->has_ip_protocol(type); } [[nodiscard]] tr_address bind_address(tr_address_type type) const noexcept; @@ -1116,18 +1116,18 @@ public: [[nodiscard]] std::optional global_address(tr_address_type type) const noexcept { TR_ASSERT(tr_address::is_valid(type)); - return ip_cache_.global_addr(type); + return ip_cache_->global_addr(type); } bool set_global_address(tr_address const& addr) noexcept { - return ip_cache_.set_global_addr(addr); + return ip_cache_->set_global_addr(addr); } [[nodiscard]] std::optional source_address(tr_address_type type) const noexcept { TR_ASSERT(tr_address::is_valid(type)); - return ip_cache_.source_addr(type); + return ip_cache_->source_addr(type); } [[nodiscard]] auto speed_limit(tr_direction const dir) const noexcept @@ -1449,7 +1449,7 @@ private: // depends-on: settings_, session_thread_, timer_maker_, web_ IPCacheMediator ip_cache_mediator_{ *this }; - tr_ip_cache ip_cache_{ ip_cache_mediator_ }; + std::shared_ptr ip_cache_ = tr_ip_cache::create(ip_cache_mediator_); // depends-on: settings_, session_thread_, torrents_, global_ip_cache (via tr_session::bind_address()) WebMediator web_mediator_{ this }; diff --git a/tests/libtransmission/ip-cache-test.cc b/tests/libtransmission/ip-cache-test.cc index 3fa8917ce..a094bac17 100644 --- a/tests/libtransmission/ip-cache-test.cc +++ b/tests/libtransmission/ip-cache-test.cc @@ -89,7 +89,7 @@ protected: } // To be created within the test body - std::unique_ptr ip_cache_; + std::shared_ptr ip_cache_; }; TEST_F(IPCacheTest, bindAddr) @@ -123,7 +123,7 @@ TEST_F(IPCacheTest, bindAddr) }; auto mediator = LocalMockMediator{}; - ip_cache_ = std::make_unique(mediator); + ip_cache_ = tr_ip_cache::create(mediator); for (std::size_t i = 0; i < NUM_TR_AF_INET_TYPES; ++i) { @@ -148,7 +148,7 @@ TEST_F(IPCacheTest, setGlobalAddr) static_assert(std::size(AddrStr) == std::size(AddrTests)); auto mediator = MockMediator{}; - ip_cache_ = std::make_unique(mediator); + ip_cache_ = tr_ip_cache::create(mediator); for (std::size_t i = 0; i < std::size(AddrStr); ++i) { @@ -172,7 +172,7 @@ TEST_F(IPCacheTest, globalSourceIPv4) } }; auto mediator = LocalMockMediator{}; - ip_cache_ = std::make_unique(mediator); + ip_cache_ = tr_ip_cache::create(mediator); ip_cache_->update_source_addr(TR_AF_INET); auto const addr = ip_cache_->source_addr(TR_AF_INET); @@ -196,7 +196,7 @@ TEST_F(IPCacheTest, globalSourceIPv6) } }; auto mediator = LocalMockMediator{}; - ip_cache_ = std::make_unique(mediator); + ip_cache_ = tr_ip_cache::create(mediator); ip_cache_->update_source_addr(TR_AF_INET6); auto const addr = ip_cache_->source_addr(TR_AF_INET6); @@ -242,7 +242,7 @@ TEST_F(IPCacheTest, onResponseIPQuery) }; auto mediator = LocalMockMediator{}; - ip_cache_ = std::make_unique(mediator); + ip_cache_ = tr_ip_cache::create(mediator); mediator.address_type = 0; for (std::size_t& i = mediator.address_type; i < NUM_TR_AF_INET_TYPES; ++i)