From 49297d937ce6128b2c9f1fb09199dd7d4f2404b7 Mon Sep 17 00:00:00 2001 From: Fabio Alessandrelli Date: Sat, 29 Jan 2022 01:33:29 +0100 Subject: [Net] Simplify IP resolution code, fix caching. First, we should not insert into cache if the hostname resolution has failed (as it might be a temporary internet issue), second, the async resolver should also properly insert into cache. Took the chance to remove some duplicate code with critical section in it at the cost of little performance when calling the blocking resolve_hostname function. --- core/io/ip.cpp | 42 ++++++++++++------------------------------ 1 file changed, 12 insertions(+), 30 deletions(-) (limited to 'core') diff --git a/core/io/ip.cpp b/core/io/ip.cpp index 4970afc1d3..8e0d47e762 100644 --- a/core/io/ip.cpp +++ b/core/io/ip.cpp @@ -98,6 +98,11 @@ struct _IP_ResolverPrivate { if (queue[i].status.get() != IP::RESOLVER_STATUS_WAITING) { continue; } + // We might be overriding another result, but we don't care as long as the result is valid. + if (response.size()) { + String key = get_cache_key(hostname, type); + cache[key] = response; + } queue[i].response = response; queue[i].status.set(response.is_empty() ? IP::RESOLVER_STATUS_ERROR : IP::RESOLVER_STATUS_DONE); } @@ -120,30 +125,8 @@ struct _IP_ResolverPrivate { }; IPAddress IP::resolve_hostname(const String &p_hostname, IP::Type p_type) { - List res; - String key = _IP_ResolverPrivate::get_cache_key(p_hostname, p_type); - - resolver->mutex.lock(); - if (resolver->cache.has(key)) { - res = resolver->cache[key]; - } else { - // This should be run unlocked so the resolver thread can keep - // resolving other requests. - resolver->mutex.unlock(); - _resolve_hostname(res, p_hostname, p_type); - resolver->mutex.lock(); - // We might be overriding another result, but we don't care (they are the - // same hostname). - resolver->cache[key] = res; - } - resolver->mutex.unlock(); - - for (int i = 0; i < res.size(); ++i) { - if (res[i].is_valid()) { - return res[i]; - } - } - return IPAddress(); + const Array addresses = resolve_hostname_addresses(p_hostname, p_type); + return addresses.size() ? addresses[0].operator IPAddress() : IPAddress(); } Array IP::resolve_hostname_addresses(const String &p_hostname, Type p_type) { @@ -159,17 +142,16 @@ Array IP::resolve_hostname_addresses(const String &p_hostname, Type p_type) { resolver->mutex.unlock(); _resolve_hostname(res, p_hostname, p_type); resolver->mutex.lock(); - // We might be overriding another result, but we don't care (they are the - // same hostname). - resolver->cache[key] = res; + // We might be overriding another result, but we don't care as long as the result is valid. + if (res.size()) { + resolver->cache[key] = res; + } } resolver->mutex.unlock(); Array result; for (int i = 0; i < res.size(); ++i) { - if (res[i].is_valid()) { - result.push_back(String(res[i])); - } + result.push_back(String(res[i])); } return result; } -- cgit v1.2.3