core: Simplify DnsNameResolver by using ObjectPool

ObjectPool is our standard solution for dealing with the
sometimes-shutdown resources. This was implemented by a contributor not
familiar with regular tools.

There are wider changes that can be made here, but I chose to just do a
smaller change because this class is used by GrpclbNameResolver.
This commit is contained in:
Eric Anderson 2024-12-10 12:53:24 -08:00 committed by GitHub
parent 210f9c083e
commit 6055adca5a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 12 additions and 14 deletions

View File

@ -134,10 +134,10 @@ public class DnsNameResolver extends NameResolver {
private final String host;
private final int port;
/** Executor that will be used if an Executor is not provide via {@link NameResolver.Args}. */
private final Resource<Executor> executorResource;
private final ObjectPool<Executor> executorPool;
private final long cacheTtlNanos;
private final SynchronizationContext syncContext;
private final ServiceConfigParser serviceConfigParser;
// Following fields must be accessed from syncContext
private final Stopwatch stopwatch;
@ -145,10 +145,6 @@ public class DnsNameResolver extends NameResolver {
private boolean shutdown;
private Executor executor;
/** True if using an executor resource that should be released after use. */
private final boolean usingExecutorResource;
private final ServiceConfigParser serviceConfigParser;
private boolean resolving;
// The field must be accessed from syncContext, although the methods on an Listener2 can be called
@ -165,7 +161,7 @@ public class DnsNameResolver extends NameResolver {
checkNotNull(args, "args");
// TODO: if a DNS server is provided as nsAuthority, use it.
// https://www.captechconsulting.com/blogs/accessing-the-dusty-corners-of-dns-with-java
this.executorResource = executorResource;
// Must prepend a "//" to the name when constructing a URI, otherwise it will be treated as an
// opaque URI, thus the authority and host of the resulted URI would be null.
URI nameUri = URI.create("//" + checkNotNull(name, "name"));
@ -179,11 +175,15 @@ public class DnsNameResolver extends NameResolver {
port = nameUri.getPort();
}
this.proxyDetector = checkNotNull(args.getProxyDetector(), "proxyDetector");
Executor offloadExecutor = args.getOffloadExecutor();
if (offloadExecutor != null) {
this.executorPool = new FixedObjectPool<>(offloadExecutor);
} else {
this.executorPool = SharedResourcePool.forResource(executorResource);
}
this.cacheTtlNanos = getNetworkAddressCacheTtlNanos(isAndroid);
this.stopwatch = checkNotNull(stopwatch, "stopwatch");
this.syncContext = checkNotNull(args.getSynchronizationContext(), "syncContext");
this.executor = args.getOffloadExecutor();
this.usingExecutorResource = executor == null;
this.serviceConfigParser = checkNotNull(args.getServiceConfigParser(), "serviceConfigParser");
}
@ -200,9 +200,7 @@ public class DnsNameResolver extends NameResolver {
@Override
public void start(Listener2 listener) {
Preconditions.checkState(this.listener == null, "already started");
if (usingExecutorResource) {
executor = SharedResourceHolder.get(executorResource);
}
executor = executorPool.getObject();
this.listener = checkNotNull(listener, "listener");
resolve();
}
@ -413,8 +411,8 @@ public class DnsNameResolver extends NameResolver {
return;
}
shutdown = true;
if (executor != null && usingExecutorResource) {
executor = SharedResourceHolder.release(executorResource, executor);
if (executor != null) {
executor = executorPool.returnObject(executor);
}
}