diff --git a/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java b/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java index dffdf2c7476..ebab2eb4cc5 100644 --- a/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java +++ b/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java @@ -118,6 +118,10 @@ public void uncaughtException(Thread t, Throwable e) { private final CountDownLatch internalTerminationLatch = new CountDownLatch(1); private final SettableFuture initialStartFuture = SettableFuture.create(); private boolean initialStarted; + // Must be accessed in syncContext. + // Guards the forceful-shutdown work in shutdownNow(), independently of the shutdown AtomicBoolean + // above, so it isn't skipped when shutdown() + private boolean shutdownNowed; private ScheduledHandle restartTimer; private ObjectPool xdsClientPool; private XdsClient xdsClient; @@ -408,16 +412,15 @@ public void run() { @Override public Server shutdownNow() { - if (!shutdown.compareAndSet(false, true)) { - return this; - } + shutdown(); syncContext.execute(new Runnable() { @Override public void run() { - if (!delegate.isShutdown()) { - delegate.shutdownNow(); + if (shutdownNowed) { + return; } - internalShutdown(); + shutdownNowed = true; + delegate.shutdownNow(); initialStartFuture.set(new IOException("server is forcefully shut down")); } }); diff --git a/xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java b/xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java index 47ac32cdc8a..e309df4f453 100644 --- a/xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java @@ -487,6 +487,62 @@ public void run() { } } + @Test + public void shutdownNow_afterShutdown_stillUnblocksStartThread() throws Exception { + final SettableFuture start = SettableFuture.create(); + Executors.newSingleThreadExecutor() + .execute( + new Runnable() { + @Override + public void run() { + try { + start.set(xdsServerWrapper.start()); + } catch (Exception ex) { + start.setException(ex); + } + } + }); + assertThat(xdsClient.ldsResource.get(5, TimeUnit.SECONDS)) + .isEqualTo("grpc/server?udpa.resource.listening_address=0.0.0.0:1"); + xdsServerWrapper.shutdown(); + xdsServerWrapper.shutdownNow(); + try { + start.get(5, TimeUnit.SECONDS); + fail("should have thrown but not"); + } catch (ExecutionException ex) { + assertThat(ex).hasCauseThat().isInstanceOf(IOException.class); + assertThat(ex).hasCauseThat().hasMessageThat().isEqualTo("server is forcefully shut down"); + } + } + + @Test + public void shutdownNow_calledTwice_forcefullyShutsDownDelegateOnce() throws Exception { + final SettableFuture start = SettableFuture.create(); + Executors.newSingleThreadExecutor() + .execute( + new Runnable() { + @Override + public void run() { + try { + start.set(xdsServerWrapper.start()); + } catch (Exception ex) { + start.setException(ex); + } + } + }); + assertThat(xdsClient.ldsResource.get(5, TimeUnit.SECONDS)) + .isEqualTo("grpc/server?udpa.resource.listening_address=0.0.0.0:1"); + xdsServerWrapper.shutdownNow(); + xdsServerWrapper.shutdownNow(); + try { + start.get(5, TimeUnit.SECONDS); + fail("should have thrown but not"); + } catch (ExecutionException ex) { + assertThat(ex).hasCauseThat().isInstanceOf(IOException.class); + } + verify(mockServer, times(1)).shutdownNow(); + } + @Test public void initialStartIoException() throws Exception { final SettableFuture start = SettableFuture.create();