diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java index 510e6a5cc9..f95e938a69 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java @@ -791,6 +791,11 @@ final class ManagedChannelImpl extends ManagedChannel implements channelStateManager.gotoState(TRANSIENT_FAILURE); } + @VisibleForTesting + boolean isInPanicMode() { + return panicMode; + } + // Called from syncContext private void updateSubchannelPicker(SubchannelPicker newPicker) { subchannelPicker = newPicker; diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java index 6bd3f89146..f133fba939 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java @@ -136,7 +136,6 @@ import java.util.logging.Logger; import javax.annotation.Nullable; import org.junit.After; import org.junit.Assert; -import org.junit.Assume; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -269,6 +268,7 @@ public class ManagedChannelImplTest { private ChannelBuilder channelBuilder; private boolean requestConnection = true; private BlockingQueue transports; + private boolean panicExpected; private ArgumentCaptor streamListenerCaptor = ArgumentCaptor.forClass(ClientStreamListener.class); @@ -338,6 +338,9 @@ public class ManagedChannelImplTest { assertTrue(timer.getDueTasks() + " should be empty", timer.getDueTasks().isEmpty()); assertEquals(executor.getPendingTasks() + " should be empty", 0, executor.numPendingTasks()); if (channel != null) { + if (!panicExpected) { + assertFalse(channel.isInPanicMode()); + } channel.shutdownNow(); channel = null; } @@ -405,15 +408,8 @@ public class ManagedChannelImplTest { @SuppressWarnings("deprecation") public void createSubchannel_old_propagateSubchannelStatesToOldApi() { createChannel(); - final AtomicReference subchannelCapture = new AtomicReference<>(); - helper.getSynchronizationContext().execute(new Runnable() { - @Override - public void run() { - subchannelCapture.set(helper.createSubchannel(addressGroup, Attributes.EMPTY)); - } - }); - Subchannel subchannel = subchannelCapture.get(); + Subchannel subchannel = helper.createSubchannel(addressGroup, Attributes.EMPTY); subchannel.requestConnection(); verify(mockTransportFactory) @@ -427,6 +423,15 @@ public class ManagedChannelImplTest { verify(mockLoadBalancer).handleSubchannelState( same(subchannel), eq(ConnectivityStateInfo.forNonError(READY))); + + channel.shutdown(); + verify(mockLoadBalancer).shutdown(); + subchannel.shutdown(); + + verify(mockLoadBalancer, atLeast(0)).canHandleEmptyAddressListFromNameResolution(); + verify(mockLoadBalancer, atLeast(0)).handleNameResolutionError(any(Status.class)); + // handleSubchannelState() should not be called after shutdown() + verifyNoMoreInteractions(mockLoadBalancer); } @Test @@ -773,9 +778,17 @@ public class ManagedChannelImplTest { channel.shutdown(); verify(mockLoadBalancer).shutdown(); + verifyNoMoreInteractions(stateListener1, stateListener2); - // No more callback should be delivered to LoadBalancer after it's shut down + // LoadBalancer will normally shutdown all subchannels + subchannel1.shutdown(); + subchannel2.shutdown(); + + // No more callback are delivered to LoadBalancer or the state listeners after it's shut down + transportInfo1.listener.transportShutdown(Status.UNAVAILABLE); transportInfo2.listener.transportReady(); + verifyNoMoreInteractions(stateListener1, stateListener2); + resolver.listener.onError(resolutionError); resolver.resolved(); verifyNoMoreInteractions(mockLoadBalancer); @@ -2213,8 +2226,8 @@ public class ManagedChannelImplTest { assertEquals(TRANSIENT_FAILURE, channel.getState(true)); verifyPanicMode(panicReason); - // No new resolver or balancer are created - verifyNoMoreInteractions(mockLoadBalancerProvider); + // Besides the resolver created initially, no new resolver or balancer are created. + verify(mockLoadBalancerProvider).newLoadBalancer(any(Helper.class)); assertThat(nameResolverFactory.resolvers).isEmpty(); // A misbehaving balancer that calls updateBalancingState() after it's shut down will not be @@ -2223,7 +2236,12 @@ public class ManagedChannelImplTest { verifyPanicMode(panicReason); // Cannot be revived by exitIdleMode() - channel.exitIdleMode(); + channel.syncContext.execute(new Runnable() { + @Override + public void run() { + channel.exitIdleMode(); + } + }); verifyPanicMode(panicReason); // Can still shutdown normally @@ -2270,11 +2288,11 @@ public class ManagedChannelImplTest { executor.runDueTasks(); verifyCallListenerClosed(mockCallListener, Status.Code.INTERNAL, panicReason); verifyCallListenerClosed(mockCallListener2, Status.Code.INTERNAL, panicReason); + panicExpected = true; } private void verifyPanicMode(Throwable cause) { - Assume.assumeTrue("Panic mode disabled to resolve issues with some tests. See #3293", false); - + panicExpected = true; @SuppressWarnings("unchecked") ClientCall.Listener mockListener = (ClientCall.Listener) mock(ClientCall.Listener.class);