Enable Happy Eyeballs by default

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

@@ -34,6 +34,7 @@
 import io.grpc.ExperimentalApi;
 import io.grpc.LoadBalancer;
 import io.grpc.Status;
+import io.grpc.SynchronizationContext;
 import io.grpc.SynchronizationContext.ScheduledHandle;
 import java.net.SocketAddress;
 import java.util.ArrayList;
@@ -72,7 +73,7 @@
   private ConnectivityState rawConnectivityState = IDLE;
   private ConnectivityState concludedState = IDLE;
   private final boolean enableHappyEyeballs =
-      GrpcUtil.getFlag(GRPC_EXPERIMENTAL_XDS_DUALSTACK_ENDPOINTS, false);
+      GrpcUtil.getFlag(GRPC_EXPERIMENTAL_XDS_DUALSTACK_ENDPOINTS, true);
 
   PickFirstLeafLoadBalancer(Helper helper) {
     this.helper = checkNotNull(helper, "helper");
@@ -406,7 +407,16 @@
       }
     }
 
-    scheduleConnectionTask = helper.getSynchronizationContext().schedule(
+    SynchronizationContext synchronizationContext = null;
+    try {
+      synchronizationContext = helper.getSynchronizationContext();
+    } catch (NullPointerException e) {
+      // All helpers should have a sync context, but if one doesn't (ex. user had a custom test)
+      // we don't want to break previously working functionality.
+      return;
+    }
+
+    scheduleConnectionTask = synchronizationContext.schedule(
         new StartNextConnection(),
         CONNECTION_DELAY_INTERVAL_MS,
         TimeUnit.MILLISECONDS,

core/src/test/java/io/grpc/internal/AutoConfiguredLoadBalancerFactoryTest.java

@@ -48,6 +48,7 @@
 import io.grpc.LoadBalancerRegistry;
 import io.grpc.NameResolver.ConfigOrError;
 import io.grpc.Status;
+import io.grpc.SynchronizationContext;
 import io.grpc.internal.AutoConfiguredLoadBalancerFactory.AutoConfiguredLoadBalancer;
 import io.grpc.internal.PickFirstLeafLoadBalancer.PickFirstLeafLoadBalancerConfig;
 import io.grpc.internal.PickFirstLoadBalancer.PickFirstLoadBalancerConfig;
@@ -58,6 +59,7 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
@@ -691,6 +693,16 @@ private static class TestLoadBalancer extends ForwardingLoadBalancer {
   }
 
   private class TestHelper extends ForwardingLoadBalancerHelper {
+    final SynchronizationContext syncContext = new SynchronizationContext(
+        new Thread.UncaughtExceptionHandler() {
+          @Override
+          public void uncaughtException(Thread t, Throwable e) {
+            throw new AssertionError(e);
+          }
+        });
+
+    final FakeClock fakeClock = new FakeClock();
+
     @Override
     protected Helper delegate() {
       return null;
@@ -705,6 +717,16 @@ public ChannelLogger getChannelLogger() {
     public void updateBalancingState(ConnectivityState newState, SubchannelPicker newPicker) {
       // noop
     }
+
+    @Override
+    public SynchronizationContext getSynchronizationContext() {
+      return syncContext;
+    }
+
+    @Override
+    public ScheduledExecutorService getScheduledExecutorService() {
+      return fakeClock.getScheduledExecutorService();
+    }
   }
 
   private static class TestSubchannel extends Subchannel {

interop-testing/src/test/java/io/grpc/testing/integration/Http2Test.java

@@ -18,6 +18,7 @@
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertTrue;
 
 import io.grpc.ChannelCredentials;
 import io.grpc.ManagedChannelBuilder;
@@ -38,7 +39,6 @@
 import io.grpc.stub.MetadataUtils;
 import io.grpc.testing.TlsTesting;
 import java.io.IOException;
-import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.util.Arrays;
 import org.junit.Test;
@@ -139,15 +139,15 @@ protected ManagedChannelBuilder<?> createChannelBuilder() {
   @Test
   public void remoteAddr() {
     InetSocketAddress isa = (InetSocketAddress) obtainRemoteClientAddr();
-    assertEquals(InetAddress.getLoopbackAddress(), isa.getAddress());
+    assertTrue(isa.getAddress().isLoopbackAddress());
     // It should not be the same as the server
     assertNotEquals(((InetSocketAddress) getListenAddress()).getPort(), isa.getPort());
   }
 
   @Test
   public void localAddr() throws Exception {
     InetSocketAddress isa = (InetSocketAddress) obtainLocalServerAddr();
-    assertEquals(InetAddress.getLoopbackAddress(), isa.getAddress());
+    assertTrue(isa.getAddress().isLoopbackAddress());
     assertEquals(((InetSocketAddress) getListenAddress()).getPort(), isa.getPort());
   }
 

rls/src/test/java/io/grpc/rls/RlsLoadBalancerTest.java

@@ -217,6 +217,8 @@ public void lb_working_withDefaultTarget_rlsResponding() throws Exception {
     inOrder.verify(helper).createSubchannel(any(CreateSubchannelArgs.class));
     inOrder.verify(helper, atLeast(0))
         .updateBalancingState(eq(ConnectivityState.CONNECTING), any(SubchannelPicker.class));
+    inOrder.verify(helper, atLeast(0)).getSynchronizationContext();
+    inOrder.verify(helper, atLeast(0)).getScheduledExecutorService();
     inOrder.verifyNoMoreInteractions();
     assertThat(res.getStatus().isOk()).isTrue();
     assertThat(subchannels).hasSize(1);
@@ -325,6 +327,8 @@ public void lb_working_withoutDefaultTarget() throws Exception {
     inOrder.verify(helper).createSubchannel(any(CreateSubchannelArgs.class));
     inOrder.verify(helper, atLeast(0))
         .updateBalancingState(eq(ConnectivityState.CONNECTING), any(SubchannelPicker.class));
+    inOrder.verify(helper, atLeast(0)).getSynchronizationContext();
+    inOrder.verify(helper, atLeast(0)).getScheduledExecutorService();
     inOrder.verifyNoMoreInteractions();
     assertThat(res.getStatus().isOk()).isTrue();
 

util/src/test/java/io/grpc/util/MultiChildLoadBalancerTest.java

@@ -32,7 +32,6 @@
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.verifyNoMoreInteractions;
 
 import com.google.common.collect.Lists;
 import io.grpc.Attributes;
@@ -128,7 +127,7 @@ public void pickAfterResolved() {
         (TestLb.TestSubchannelPicker) pickerCaptor.getValue();
     assertThat(subchannelPicker.getReadySubchannels()).containsExactly(readySubchannel);
 
-    verifyNoMoreInteractions(mockHelper);
+    AbstractTestHelper.verifyNoMoreMeaningfulInteractions(mockHelper);
   }
 
   @Test
@@ -192,7 +191,7 @@ public void pickAfterResolvedUpdatedHosts() {
     verify(mockHelper, times(3)).createSubchannel(any(LoadBalancer.CreateSubchannelArgs.class));
     inOrder.verify(mockHelper, times(2)).updateBalancingState(eq(READY), pickerCaptor.capture());
 
-    verifyNoMoreInteractions(mockHelper);
+    AbstractTestHelper.verifyNoMoreMeaningfulInteractions(mockHelper);
   }
 
   @Test

util/src/test/java/io/grpc/util/RoundRobinLoadBalancerTest.java

@@ -153,7 +153,7 @@ public void pickAfterResolved() throws Exception {
     assertEquals(READY, stateCaptor.getAllValues().get(1));
     assertThat(getList(pickerCaptor.getValue())).containsExactly(readySubchannel);
 
-    verifyNoMoreInteractions(mockHelper);
+    AbstractTestHelper.verifyNoMoreMeaningfulInteractions(mockHelper);
   }
 
   @Test
@@ -234,7 +234,7 @@ public void pickAfterResolvedUpdatedHosts() throws Exception {
     picker = pickerCaptor.getValue();
     assertThat(getList(picker)).containsExactly(oldSubchannel, newSubchannel);
 
-    verifyNoMoreInteractions(mockHelper);
+    AbstractTestHelper.verifyNoMoreMeaningfulInteractions(mockHelper);
   }
 
   @Test
@@ -269,7 +269,7 @@ public void pickAfterStateChange() throws Exception {
 
     verify(subchannel, atLeastOnce()).requestConnection();
     verify(mockHelper, times(3)).createSubchannel(any(CreateSubchannelArgs.class));
-    verifyNoMoreInteractions(mockHelper);
+    AbstractTestHelper.verifyNoMoreMeaningfulInteractions(mockHelper);
   }
 
   @Test
@@ -355,7 +355,7 @@ public void refreshNameResolutionWhenSubchannelConnectionBroken() {
       inOrder.verify(mockHelper).updateBalancingState(eq(CONNECTING), isA(EmptyPicker.class));
     }
 
-    verifyNoMoreInteractions(mockHelper);
+    AbstractTestHelper.verifyNoMoreMeaningfulInteractions(mockHelper);
   }
 
   @Test
@@ -432,7 +432,7 @@ public void nameResolutionErrorWithActiveChannels() throws Exception {
 
     LoadBalancer.PickResult pickResult2 = pickerCaptor.getValue().pickSubchannel(mockArgs);
     assertEquals(readySubchannel, pickResult2.getSubchannel());
-    verifyNoMoreInteractions(mockHelper);
+    AbstractTestHelper.verifyNoMoreMeaningfulInteractions(mockHelper);
   }
 
   @Test

util/src/testFixtures/java/io/grpc/util/AbstractTestHelper.java

@@ -18,7 +18,10 @@
 
 import static org.mockito.AdditionalAnswers.delegatesTo;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.atLeast;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
 
 import com.google.common.collect.Maps;
 import io.grpc.Attributes;
@@ -32,12 +35,15 @@
 import io.grpc.LoadBalancer.Subchannel;
 import io.grpc.LoadBalancer.SubchannelPicker;
 import io.grpc.LoadBalancer.SubchannelStateListener;
+import io.grpc.SynchronizationContext;
+import io.grpc.internal.FakeClock;
 import io.grpc.internal.PickFirstLoadBalancerProvider;
 import java.net.SocketAddress;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.ScheduledExecutorService;
 import org.mockito.ArgumentCaptor;
 import org.mockito.InOrder;
 
@@ -60,9 +66,26 @@ public abstract class AbstractTestHelper extends ForwardingLoadBalancerHelper {
   protected final Map<Subchannel, Subchannel> realToMockSubChannelMap = new HashMap<>();
   private final Map<Subchannel, SubchannelStateListener> subchannelStateListeners =
       Maps.newLinkedHashMap();
+  private final FakeClock fakeClock;
+  private final SynchronizationContext syncContext;
 
   public abstract Map<List<EquivalentAddressGroup>, Subchannel> getSubchannelMap();
 
+  public AbstractTestHelper() {
+    this(new FakeClock(), new SynchronizationContext(new Thread.UncaughtExceptionHandler() {
+      @Override
+      public void uncaughtException(Thread t, Throwable e) {
+        throw new RuntimeException(e);
+      }
+    }));
+  }
+
+  public AbstractTestHelper(FakeClock fakeClock, SynchronizationContext syncContext) {
+    super();
+    this.fakeClock = fakeClock;
+    this.syncContext = syncContext;
+  }
+
   public Map<Subchannel, Subchannel> getMockToRealSubChannelMap() {
     return mockToRealSubChannelMap;
   }
@@ -79,6 +102,18 @@ public Map<Subchannel, SubchannelStateListener> getSubchannelStateListeners() {
     return subchannelStateListeners;
   }
 
+  public static final FakeClock.TaskFilter NOT_START_NEXT_CONNECTION =
+      new FakeClock.TaskFilter() {
+        @Override
+        public boolean shouldAccept(Runnable command) {
+          return !command.toString().contains("StartNextConnection");
+        }
+      };
+
+  public static int getNumFilteredPendingTasks(FakeClock fakeClock) {
+    return fakeClock.getPendingTasks(NOT_START_NEXT_CONNECTION).size();
+  }
+
   public void deliverSubchannelState(Subchannel subchannel, ConnectivityStateInfo newState) {
     Subchannel realSc = getMockToRealSubChannelMap().get(subchannel);
     if (realSc == null) {
@@ -128,6 +163,16 @@ public void setChannel(Subchannel subchannel, Channel channel) {
     ((TestSubchannel)subchannel).channel = channel;
   }
 
+  @Override
+  public SynchronizationContext getSynchronizationContext() {
+    return syncContext;
+  }
+
+  @Override
+  public ScheduledExecutorService getScheduledExecutorService() {
+    return fakeClock.getScheduledExecutorService();
+  }
+
   @Override
   public String toString() {
     return "Test Helper";
@@ -148,6 +193,17 @@ public static void refreshInvokedAndUpdateBS(InOrder inOrder, ConnectivityState
     }
   }
 
+  public static void verifyNoMoreMeaningfulInteractions(Helper helper) {
+    verify(helper, atLeast(0)).getSynchronizationContext();
+    verify(helper, atLeast(0)).getScheduledExecutorService();
+    verifyNoMoreInteractions(helper);
+  }
+
+  public static void verifyNoMoreMeaningfulInteractions(Helper helper, InOrder inOrder) {
+    inOrder.verify(helper, atLeast(0)).getSynchronizationContext();
+    inOrder.verify(helper, atLeast(0)).getScheduledExecutorService();
+    inOrder.verifyNoMoreInteractions();
+  }
 
   protected class TestSubchannel extends ForwardingSubchannel {
     CreateSubchannelArgs args;

xds/src/main/java/io/grpc/xds/XdsEndpointResource.java

@@ -101,7 +101,7 @@ protected EdsUpdate doParse(Args args, Message unpackedMessage) throws ResourceI
   }
 
   private static boolean isEnabledXdsDualStack() {
-    return GrpcUtil.getFlag(GRPC_EXPERIMENTAL_XDS_DUALSTACK_ENDPOINTS, false);
+    return GrpcUtil.getFlag(GRPC_EXPERIMENTAL_XDS_DUALSTACK_ENDPOINTS, true);
   }
 
   private static EdsUpdate processClusterLoadAssignment(ClusterLoadAssignment assignment)

xds/src/test/java/io/grpc/xds/LeastRequestLoadBalancerTest.java

@@ -159,7 +159,7 @@ public void pickAfterResolved() throws Exception {
     assertEquals(READY, stateCaptor.getAllValues().get(1));
     assertThat(getList(pickerCaptor.getValue())).containsExactly(readySubchannel);
 
-    verifyNoMoreInteractions(helper);
+    AbstractTestHelper.verifyNoMoreMeaningfulInteractions(helper);
   }
 
   @Test
@@ -226,7 +226,7 @@ public void pickAfterResolvedUpdatedHosts() throws Exception {
 
     assertThat(getList(pickerCaptor.getValue())).containsExactly(oldSubchannel, newSubchannel);
 
-    verifyNoMoreInteractions(helper);
+    AbstractTestHelper.verifyNoMoreMeaningfulInteractions(helper);
   }
 
   private Subchannel getSubchannel(EquivalentAddressGroup removedEag) {
@@ -288,7 +288,7 @@ public void pickAfterStateChange() throws Exception {
     int expectedCount = PickFirstLoadBalancerProvider.isEnabledNewPickFirst() ? 1 : 2;
     verify(subchannel, times(expectedCount)).requestConnection();
     verify(helper, times(3)).createSubchannel(any(CreateSubchannelArgs.class));
-    verifyNoMoreInteractions(helper);
+    AbstractTestHelper.verifyNoMoreMeaningfulInteractions(helper);
   }
 
   @Test
@@ -319,7 +319,7 @@ public void pickAfterConfigChange() {
     // At this point it should use a ReadyPicker with newConfig
     pickerCaptor.getValue().pickSubchannel(mockArgs);
     verify(mockRandom, times(oldConfig.choiceCount + newConfig.choiceCount)).nextInt(1);
-    verifyNoMoreInteractions(helper);
+    AbstractTestHelper.verifyNoMoreMeaningfulInteractions(helper);
   }
 
   @Test
@@ -375,7 +375,7 @@ public void stayTransientFailureUntilReady() {
     inOrder.verify(helper).updateBalancingState(eq(READY), isA(ReadyPicker.class));
 
     verify(helper, times(3)).createSubchannel(any(CreateSubchannelArgs.class));
-    verifyNoMoreInteractions(helper);
+    AbstractTestHelper.verifyNoMoreMeaningfulInteractions(helper);
   }
 
   private String getStatusString(SubchannelPicker picker) {
@@ -426,7 +426,7 @@ public void refreshNameResolutionWhenSubchannelConnectionBroken() {
       inOrder.verify(helper).updateBalancingState(eq(CONNECTING), isA(EmptyPicker.class));
     }
 
-    verifyNoMoreInteractions(helper);
+    AbstractTestHelper.verifyNoMoreMeaningfulInteractions(helper);
   }
 
   @Test
@@ -548,7 +548,7 @@ public void nameResolutionErrorWithActiveChannels() throws Exception {
     LoadBalancer.PickResult pickResult2 = pickerCaptor.getValue().pickSubchannel(mockArgs);
     verify(mockRandom, times(choiceCount * 2)).nextInt(1);
     assertEquals(readySubchannel, pickResult2.getSubchannel());
-    verifyNoMoreInteractions(helper);
+    AbstractTestHelper.verifyNoMoreMeaningfulInteractions(helper);
   }
 
   @Test