Skip to content

Commit 6055adc

Browse files
authored
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.
1 parent 210f9c0 commit 6055adc

File tree

1 file changed

+12
-14
lines changed

1 file changed

+12
-14
lines changed

core/src/main/java/io/grpc/internal/DnsNameResolver.java

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -134,21 +134,17 @@ public class DnsNameResolver extends NameResolver {
134134
private final String host;
135135
private final int port;
136136

137-
/** Executor that will be used if an Executor is not provide via {@link NameResolver.Args}. */
138-
private final Resource<Executor> executorResource;
137+
private final ObjectPool<Executor> executorPool;
139138
private final long cacheTtlNanos;
140139
private final SynchronizationContext syncContext;
140+
private final ServiceConfigParser serviceConfigParser;
141141

142142
// Following fields must be accessed from syncContext
143143
private final Stopwatch stopwatch;
144144
protected boolean resolved;
145145
private boolean shutdown;
146146
private Executor executor;
147147

148-
/** True if using an executor resource that should be released after use. */
149-
private final boolean usingExecutorResource;
150-
private final ServiceConfigParser serviceConfigParser;
151-
152148
private boolean resolving;
153149

154150
// The field must be accessed from syncContext, although the methods on an Listener2 can be called
@@ -165,7 +161,7 @@ protected DnsNameResolver(
165161
checkNotNull(args, "args");
166162
// TODO: if a DNS server is provided as nsAuthority, use it.
167163
// https://www.captechconsulting.com/blogs/accessing-the-dusty-corners-of-dns-with-java
168-
this.executorResource = executorResource;
164+
169165
// Must prepend a "//" to the name when constructing a URI, otherwise it will be treated as an
170166
// opaque URI, thus the authority and host of the resulted URI would be null.
171167
URI nameUri = URI.create("//" + checkNotNull(name, "name"));
@@ -179,11 +175,15 @@ protected DnsNameResolver(
179175
port = nameUri.getPort();
180176
}
181177
this.proxyDetector = checkNotNull(args.getProxyDetector(), "proxyDetector");
178+
Executor offloadExecutor = args.getOffloadExecutor();
179+
if (offloadExecutor != null) {
180+
this.executorPool = new FixedObjectPool<>(offloadExecutor);
181+
} else {
182+
this.executorPool = SharedResourcePool.forResource(executorResource);
183+
}
182184
this.cacheTtlNanos = getNetworkAddressCacheTtlNanos(isAndroid);
183185
this.stopwatch = checkNotNull(stopwatch, "stopwatch");
184186
this.syncContext = checkNotNull(args.getSynchronizationContext(), "syncContext");
185-
this.executor = args.getOffloadExecutor();
186-
this.usingExecutorResource = executor == null;
187187
this.serviceConfigParser = checkNotNull(args.getServiceConfigParser(), "serviceConfigParser");
188188
}
189189

@@ -200,9 +200,7 @@ protected String getHost() {
200200
@Override
201201
public void start(Listener2 listener) {
202202
Preconditions.checkState(this.listener == null, "already started");
203-
if (usingExecutorResource) {
204-
executor = SharedResourceHolder.get(executorResource);
205-
}
203+
executor = executorPool.getObject();
206204
this.listener = checkNotNull(listener, "listener");
207205
resolve();
208206
}
@@ -413,8 +411,8 @@ public void shutdown() {
413411
return;
414412
}
415413
shutdown = true;
416-
if (executor != null && usingExecutorResource) {
417-
executor = SharedResourceHolder.release(executorResource, executor);
414+
if (executor != null) {
415+
executor = executorPool.returnObject(executor);
418416
}
419417
}
420418

0 commit comments

Comments
 (0)