[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.

(cherry picked from commit 49297d937c)
This commit is contained in:
Fabio Alessandrelli 2022-01-29 01:33:29 +01:00 committed by Rémi Verschelde
parent c25365d5ff
commit aa3ef20fcb
No known key found for this signature in database
GPG key ID: C3336907360768E1
2 changed files with 13 additions and 31 deletions

View file

@ -96,6 +96,11 @@ struct _IP_ResolverPrivate {
if (queue[i].status.get() != IP::RESOLVER_STATUS_WAITING) { if (queue[i].status.get() != IP::RESOLVER_STATUS_WAITING) {
continue; 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].response = response;
queue[i].status.set(response.empty() ? IP::RESOLVER_STATUS_ERROR : IP::RESOLVER_STATUS_DONE); queue[i].status.set(response.empty() ? IP::RESOLVER_STATUS_ERROR : IP::RESOLVER_STATUS_DONE);
} }
@ -118,30 +123,8 @@ struct _IP_ResolverPrivate {
}; };
IP_Address IP::resolve_hostname(const String &p_hostname, IP::Type p_type) { IP_Address IP::resolve_hostname(const String &p_hostname, IP::Type p_type) {
List<IP_Address> res; const Array addresses = resolve_hostname_addresses(p_hostname, p_type);
String key = _IP_ResolverPrivate::get_cache_key(p_hostname, p_type); return addresses.size() ? addresses[0].operator IP_Address() : IP_Address();
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 IP_Address();
} }
Array IP::resolve_hostname_addresses(const String &p_hostname, Type p_type) { Array IP::resolve_hostname_addresses(const String &p_hostname, Type p_type) {
@ -157,17 +140,16 @@ Array IP::resolve_hostname_addresses(const String &p_hostname, Type p_type) {
resolver->mutex.unlock(); resolver->mutex.unlock();
_resolve_hostname(res, p_hostname, p_type); _resolve_hostname(res, p_hostname, p_type);
resolver->mutex.lock(); resolver->mutex.lock();
// We might be overriding another result, but we don't care (they are the // We might be overriding another result, but we don't care as long as the result is valid.
// same hostname). if (res.size()) {
resolver->cache[key] = res; resolver->cache[key] = res;
}
} }
resolver->mutex.unlock(); resolver->mutex.unlock();
Array result; Array result;
for (int i = 0; i < res.size(); ++i) { 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; return result;
} }

View file

@ -115,7 +115,7 @@ void IP_Unix::_resolve_hostname(List<IP_Address> &r_addresses, const String &p_h
continue; continue;
} }
IP_Address ip = _sockaddr2ip(next->ai_addr); IP_Address ip = _sockaddr2ip(next->ai_addr);
if (!r_addresses.find(ip)) { if (ip.is_valid() && !r_addresses.find(ip)) {
r_addresses.push_back(ip); r_addresses.push_back(ip);
} }
next = next->ai_next; next = next->ai_next;