From 6536a1c698648fc94a253e926317a0b366efe28b Mon Sep 17 00:00:00 2001
From: Terry Wilson <terrymwilson@gmail.com>
Date: Thu, 19 Aug 2021 11:15:16 -0700
Subject: [PATCH] core: Exit idle mode in enterIdle() if there's delayed
 transport.

This change assures that if there are only calls in real transport the
channel will remain in idle mode. Idle mode will be exited if there
are calls in delayed transport to allow them to be processed.
---
 .../io/grpc/internal/ManagedChannelImpl.java  |  3 +-
 .../ManagedChannelImplIdlenessTest.java       | 62 +++++++++++++++++++
 2 files changed, 64 insertions(+), 1 deletion(-)

diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java
index 6cd5598e2a6b..33dbeb61351c 100644
--- a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java
+++ b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java
@@ -423,7 +423,8 @@ private void enterIdleMode() {
     delayedTransport.reprocess(null);
     channelLogger.log(ChannelLogLevel.INFO, "Entering IDLE state");
     channelStateManager.gotoState(IDLE);
-    if (inUseStateAggregator.isInUse()) {
+    // If there are still delayed streams we have to exit idle mode in order to handle them.
+    if (delayedTransport.hasPendingStreams()) {
       exitIdleMode();
     }
   }
diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplIdlenessTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplIdlenessTest.java
index 6a76f75c8b72..a1bc78610b65 100644
--- a/core/src/test/java/io/grpc/internal/ManagedChannelImplIdlenessTest.java
+++ b/core/src/test/java/io/grpc/internal/ManagedChannelImplIdlenessTest.java
@@ -26,7 +26,9 @@
 import static org.mockito.AdditionalAnswers.delegatesTo;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.ArgumentMatchers.isA;
 import static org.mockito.ArgumentMatchers.same;
+import static org.mockito.Mockito.atMostOnce;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
@@ -284,6 +286,22 @@ public void delayedTransportHoldsOffIdleness() throws Exception {
     verify(mockLoadBalancer).shutdown();
   }
 
+  @Test
+  public void delayedTransportExitsIdleAfterEnter() throws Exception {
+    // Start a new call that will go to the delayed transport
+    ClientCall<String, Integer> call = channel.newCall(method, CallOptions.DEFAULT);
+    call.start(mockCallListener, new Metadata());
+    deliverResolutionResult();
+
+    channel.enterIdle();
+
+    // Since we have a call in delayed transport, the call to enterIdle() should have resulted in
+    // the channel going to idle mode and then immediately exiting. We confirm this by verifying
+    // that the name resolver was started up twice - once when the call was first created and a
+    // second time after exiting idle mode.
+    verify(mockNameResolver, times(2)).start(any(NameResolver.Listener2.class));
+  }
+
   @Test
   public void realTransportsHoldsOffIdleness() throws Exception {
     final EquivalentAddressGroup addressGroup = servers.get(1);
@@ -332,6 +350,50 @@ public void realTransportsHoldsOffIdleness() throws Exception {
     verify(mockLoadBalancer).shutdown();
   }
 
+  @Test
+  public void enterIdleWhileRealTransportInProgress() {
+    final EquivalentAddressGroup addressGroup = servers.get(1);
+
+    // Start a call, which goes to delayed transport
+    ClientCall<String, Integer> call = channel.newCall(method, CallOptions.DEFAULT);
+    call.start(mockCallListener, new Metadata());
+
+    // Verify that we have exited the idle mode
+    ArgumentCaptor<Helper> helperCaptor = ArgumentCaptor.forClass(null);
+    verify(mockLoadBalancerProvider).newLoadBalancer(helperCaptor.capture());
+    deliverResolutionResult();
+    Helper helper = helperCaptor.getValue();
+
+    // Create a subchannel for the real transport to happen on.
+    Subchannel subchannel = createSubchannelSafely(helper, addressGroup, Attributes.EMPTY);
+    requestConnectionSafely(helper, subchannel);
+    MockClientTransportInfo t0 = newTransports.poll();
+    t0.listener.transportReady();
+
+    SubchannelPicker mockPicker = mock(SubchannelPicker.class);
+    when(mockPicker.pickSubchannel(any(PickSubchannelArgs.class)))
+            .thenReturn(PickResult.withSubchannel(subchannel));
+    updateBalancingStateSafely(helper, READY, mockPicker);
+
+    // Delayed transport creates real streams in the app executor
+    executor.runDueTasks();
+
+    // Move transport to the in-use state
+    t0.listener.transportInUse(true);
+
+    // Now we enter Idle mode while real transport is happening
+    channel.enterIdle();
+
+    // Verify that the name resolver and the load balance were shut down.
+    verify(mockNameResolver).shutdown();
+    verify(mockLoadBalancer).shutdown();
+
+    // When there are no pending streams, the call to enterIdle() should stick and
+    // we remain in idle mode. We verify this by making sure that the name resolver
+    // was not started up more than once (the initial startup).
+    verify(mockNameResolver, atMostOnce()).start(isA(NameResolver.Listener2.class));
+  }
+
   @Test
   public void updateSubchannelAddresses_newAddressConnects() {
     ClientCall<String, Integer> call = channel.newCall(method, CallOptions.DEFAULT);