core: fix thread safety in NameResolver.Listener

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

@@ -234,8 +234,10 @@ final class ManagedChannelImpl extends ManagedChannel implements
   private final ChannelTracer channelTracer;
   private final ChannelLogger channelLogger;
   private final InternalChannelz channelz;
+  // Must be mutated and read from syncContext
   @CheckForNull
   private Boolean haveBackends; // a flag for doing channel tracing when flipped
+  // Must be mutated and read from syncContext
   @Nullable
   private Map<String, ?> lastServiceConfig; // used for channel tracing when value changed
 
@@ -1277,6 +1279,9 @@ final class ManagedChannelImpl extends ManagedChannel implements
 
     @Override
     public void onAddresses(final List<EquivalentAddressGroup> servers, final Attributes config) {
+      final class NamesResolved implements Runnable {
+        @Override
+        public void run() {
           channelLogger.log(
               ChannelLogLevel.DEBUG, "Resolved address: {0}, config={1}", servers, config);
 
@@ -1284,15 +1289,12 @@ final class ManagedChannelImpl extends ManagedChannel implements
             channelLogger.log(ChannelLogLevel.INFO, "Address resolved: {0}", servers);
             haveBackends = true;
           }
-      final Map<String, ?> serviceConfig = config.get(GrpcAttributes.NAME_RESOLVER_SERVICE_CONFIG);
+          final Map<String, ?> serviceConfig =
+              config.get(GrpcAttributes.NAME_RESOLVER_SERVICE_CONFIG);
           if (serviceConfig != null && !serviceConfig.equals(lastServiceConfig)) {
             channelLogger.log(ChannelLogLevel.INFO, "Service config changed");
             lastServiceConfig = serviceConfig;
           }
-
-      final class NamesResolved implements Runnable {
-        @Override
-        public void run() {
           // Call LB only if it's not shutdown.  If LB is shutdown, lbHelper won't match.
           if (NameResolverListenerImpl.this.helper != ManagedChannelImpl.this.lbHelper) {
             return;
@@ -1315,7 +1317,7 @@ final class ManagedChannelImpl extends ManagedChannel implements
           }
 
           if (servers.isEmpty() && !helper.lb.canHandleEmptyAddressListFromNameResolution()) {
-            onError(Status.UNAVAILABLE.withDescription(
+            handleErrorInSyncContext(Status.UNAVAILABLE.withDescription(
                     "Name resolver " + resolver + " returned an empty list"));
           } else {
             helper.lb.handleResolvedAddressGroups(servers, config);
@@ -1329,15 +1331,23 @@ final class ManagedChannelImpl extends ManagedChannel implements
     @Override
     public void onError(final Status error) {
       checkArgument(!error.isOk(), "the error status must not be OK");
+      final class NameResolverErrorHandler implements Runnable {
+        @Override
+        public void run() {
+          handleErrorInSyncContext(error);
+        }
+      }
+
+      syncContext.execute(new NameResolverErrorHandler());
+    }
+
+    private void handleErrorInSyncContext(Status error) {
       logger.log(Level.WARNING, "[{0}] Failed to resolve name. status={1}",
           new Object[] {getLogId(), error});
       if (haveBackends == null || haveBackends) {
         channelLogger.log(ChannelLogLevel.WARNING, "Failed to resolve name: {0}", error);
         haveBackends = false;
       }
-      final class NameResolverErrorHandler implements Runnable {
-        @Override
-        public void run() {
       // Call LB only if it's not shutdown.  If LB is shutdown, lbHelper won't match.
       if (NameResolverListenerImpl.this.helper != ManagedChannelImpl.this.lbHelper) {
         return;
@@ -1364,10 +1374,6 @@ final class ManagedChannelImpl extends ManagedChannel implements
     }
   }
 
-      syncContext.execute(new NameResolverErrorHandler());
-    }
-  }
-
   private final class SubchannelImpl extends AbstractSubchannel {
     // Set right after SubchannelImpl is created.
     InternalSubchannel subchannel;