fix: use weak pointers for ip cache curl callback (#8157)

This commit is contained in:
Yat Ho
2026-01-17 07:11:03 +08:00
committed by GitHub
parent 6248b9d303
commit 4e03de7630
5 changed files with 40 additions and 37 deletions

View File

@@ -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> tr_ip_cache::create(Mediator& mediator)
{
return std::shared_ptr<tr_ip_cache>{ 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;

View File

@@ -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<tr_ip_cache>
{
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<tr_ip_cache> create(Mediator& mediator);
tr_ip_cache() = delete;
~tr_ip_cache();
@@ -96,6 +96,8 @@ private:
template<typename T>
using array_ip_t = std::array<T, NUM_TR_AF_INET_TYPES>;
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;

View File

@@ -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<void>* 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<void>* 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();

View File

@@ -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<tr_address> 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<tr_address> 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<tr_ip_cache> 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 };

View File

@@ -89,7 +89,7 @@ protected:
}
// To be created within the test body
std::unique_ptr<tr_ip_cache> ip_cache_;
std::shared_ptr<tr_ip_cache> ip_cache_;
};
TEST_F(IPCacheTest, bindAddr)
@@ -123,7 +123,7 @@ TEST_F(IPCacheTest, bindAddr)
};
auto mediator = LocalMockMediator{};
ip_cache_ = std::make_unique<tr_ip_cache>(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<tr_ip_cache>(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<tr_ip_cache>(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<tr_ip_cache>(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<tr_ip_cache>(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)