diff --git a/core/src/main/java/io/grpc/NameResolver.java b/core/src/main/java/io/grpc/NameResolver.java
index 0aa73ca82e8..a0cf2c113c7 100644
--- a/core/src/main/java/io/grpc/NameResolver.java
+++ b/core/src/main/java/io/grpc/NameResolver.java
@@ -44,9 +44,12 @@
*
*
The addresses and attributes of a target may be changed over time, thus the caller registers a
* {@link Listener} to receive continuous updates.
+ *
+ *
Thread-safety: {@link #getServiceAuthority} may be called concurrently. Other methods on
+ * {@link NameResolver} don't need to be thread-safe. {@link Factory} and {@link Listener} are
+ * thread-safe.
*/
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/1770")
-@ThreadSafe
public abstract class NameResolver {
/**
* Returns the authority used to authenticate connections to servers. It must be
@@ -83,6 +86,7 @@ public abstract class NameResolver {
*/
public void refresh() {}
+ @ThreadSafe
public abstract static class Factory {
/**
* The port number used in case the target or the underlying naming system doesn't provide a
diff --git a/core/src/main/java/io/grpc/internal/DnsNameResolver.java b/core/src/main/java/io/grpc/internal/DnsNameResolver.java
index 23cea0333bc..ccbdee7e2d0 100644
--- a/core/src/main/java/io/grpc/internal/DnsNameResolver.java
+++ b/core/src/main/java/io/grpc/internal/DnsNameResolver.java
@@ -49,7 +49,6 @@
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;
import javax.annotation.Nullable;
-import javax.annotation.concurrent.GuardedBy;
/**
* A DNS-based {@link NameResolver}.
@@ -65,17 +64,11 @@ class DnsNameResolver extends NameResolver {
private final int port;
private final Resource timerServiceResource;
private final Resource executorResource;
- @GuardedBy("this")
private boolean shutdown;
- @GuardedBy("this")
private ScheduledExecutorService timerService;
- @GuardedBy("this")
private ExecutorService executor;
- @GuardedBy("this")
private ScheduledFuture> resolutionTask;
- @GuardedBy("this")
private boolean resolving;
- @GuardedBy("this")
private Listener listener;
DnsNameResolver(@Nullable String nsAuthority, String name, Attributes params,
@@ -111,7 +104,7 @@ public final String getServiceAuthority() {
}
@Override
- public final synchronized void start(Listener listener) {
+ public final void start(Listener listener) {
Preconditions.checkState(this.listener == null, "already started");
timerService = SharedResourceHolder.get(timerServiceResource);
executor = SharedResourceHolder.get(executorResource);
@@ -120,7 +113,7 @@ public final synchronized void start(Listener listener) {
}
@Override
- public final synchronized void refresh() {
+ public final void refresh() {
Preconditions.checkState(listener != null, "not started");
resolve();
}
@@ -130,18 +123,16 @@ public final synchronized void refresh() {
public void run() {
InetAddress[] inetAddrs;
Listener savedListener;
- synchronized (DnsNameResolver.this) {
- // If this task is started by refresh(), there might already be a scheduled task.
- if (resolutionTask != null) {
- resolutionTask.cancel(false);
- resolutionTask = null;
- }
- if (shutdown) {
- return;
- }
- savedListener = listener;
- resolving = true;
+ // If this task is started by refresh(), there might already be a scheduled task.
+ if (resolutionTask != null) {
+ resolutionTask.cancel(false);
+ resolutionTask = null;
}
+ if (shutdown) {
+ return;
+ }
+ savedListener = listener;
+ resolving = true;
try {
if (System.getenv("GRPC_PROXY_EXP") != null) {
EquivalentAddressGroup server =
@@ -153,16 +144,14 @@ public void run() {
try {
inetAddrs = getAllByName(host);
} catch (UnknownHostException e) {
- synchronized (DnsNameResolver.this) {
- if (shutdown) {
- return;
- }
- // Because timerService is the single-threaded GrpcUtil.TIMER_SERVICE in production,
- // we need to delegate the blocking work to the executor
- resolutionTask =
- timerService.schedule(new LogExceptionRunnable(resolutionRunnableOnExecutor),
- 1, TimeUnit.MINUTES);
+ if (shutdown) {
+ return;
}
+ // Because timerService is the single-threaded GrpcUtil.TIMER_SERVICE in production,
+ // we need to delegate the blocking work to the executor
+ resolutionTask =
+ timerService.schedule(new LogExceptionRunnable(resolutionRunnableOnExecutor),
+ 1, TimeUnit.MINUTES);
savedListener.onError(Status.UNAVAILABLE.withCause(e));
return;
}
@@ -174,9 +163,7 @@ public void run() {
}
savedListener.onAddresses(servers, Attributes.EMPTY);
} finally {
- synchronized (DnsNameResolver.this) {
- resolving = false;
- }
+ resolving = false;
}
}
};
@@ -184,10 +171,8 @@ public void run() {
private final Runnable resolutionRunnableOnExecutor = new Runnable() {
@Override
public void run() {
- synchronized (DnsNameResolver.this) {
- if (!shutdown) {
- executor.execute(resolutionRunnable);
- }
+ if (!shutdown) {
+ executor.execute(resolutionRunnable);
}
}
};
@@ -198,7 +183,6 @@ InetAddress[] getAllByName(String host) throws UnknownHostException {
return InetAddress.getAllByName(host);
}
- @GuardedBy("this")
private void resolve() {
if (resolving || shutdown) {
return;
@@ -207,7 +191,7 @@ private void resolve() {
}
@Override
- public final synchronized void shutdown() {
+ public final void shutdown() {
if (shutdown) {
return;
}