From 9ba78db8658e907a2ca2ddd9452dd6536366ee07 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Wed, 6 May 2020 12:15:05 +0100 Subject: [PATCH] Allow Undertow to stop when a request is being handled Previously, unlike embedded Jetty, Netty, and Tomcat, Undertow would not stop when one of its worker threads was in use. This meant that a a long-running or stalled request could prevent the application from shutting down in response to SIGTERM or SIGINT, and SIGTERM would be required to get the process to exit. This commit updates the factories for the reactive and servlet Undertow web server factories to configure Undertow to use a 0ms shutdown timeout. This aligns it with the behaviour of Jetty, Netty, and Tomcat. Tests have been introduced to verify the behaviour across the reactive and servlet variants of all four supported embedded web servers. Fixes gh-21319 --- .../UndertowReactiveWebServerFactory.java | 3 +- .../UndertowServletWebServerFactory.java | 3 +- ...AbstractReactiveWebServerFactoryTests.java | 28 +++++++++++++ .../AbstractServletWebServerFactoryTests.java | 40 +++++++++++++++++++ 4 files changed, 72 insertions(+), 2 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/undertow/UndertowReactiveWebServerFactory.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/undertow/UndertowReactiveWebServerFactory.java index 08a33832f7e..ab26aa5b769 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/undertow/UndertowReactiveWebServerFactory.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/undertow/UndertowReactiveWebServerFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -117,6 +117,7 @@ public class UndertowReactiveWebServerFactory extends AbstractReactiveWebServerF else { builder.addHttpListener(port, getListenAddress()); } + builder.setServerOption(UndertowOptions.SHUTDOWN_TIMEOUT, 0); for (UndertowBuilderCustomizer customizer : this.builderCustomizers) { customizer.customize(builder); } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/undertow/UndertowServletWebServerFactory.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/undertow/UndertowServletWebServerFactory.java index 8dc3b442e46..89b5bc2c8a7 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/undertow/UndertowServletWebServerFactory.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/undertow/UndertowServletWebServerFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -241,6 +241,7 @@ public class UndertowServletWebServerFactory extends AbstractServletWebServerFac else { builder.addHttpListener(port, getListenAddress()); } + builder.setServerOption(UndertowOptions.SHUTDOWN_TIMEOUT, 0); for (UndertowBuilderCustomizer customizer : this.builderCustomizers) { customizer.customize(builder); } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/server/AbstractReactiveWebServerFactoryTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/server/AbstractReactiveWebServerFactoryTests.java index 8dfbe18a5e0..e2cc94f6af8 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/server/AbstractReactiveWebServerFactoryTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/server/AbstractReactiveWebServerFactoryTests.java @@ -23,7 +23,10 @@ import java.nio.charset.StandardCharsets; import java.security.KeyStore; import java.time.Duration; import java.util.Arrays; +import java.util.concurrent.BrokenBarrierException; import java.util.concurrent.Callable; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.CyclicBarrier; import javax.net.ssl.KeyManagerFactory; import javax.net.ssl.SSLException; @@ -333,6 +336,31 @@ public abstract class AbstractReactiveWebServerFactoryTests { .hasMessageContaining("Could not load key store 'null'"); } + @Test + void whenARequestIsActiveThenStopWillComplete() throws InterruptedException, BrokenBarrierException { + AbstractReactiveWebServerFactory factory = getFactory(); + CyclicBarrier barrier = new CyclicBarrier(2); + CountDownLatch latch = new CountDownLatch(1); + this.webServer = factory.getWebServer((request, response) -> { + try { + barrier.await(); + latch.await(); + } + catch (InterruptedException ex) { + Thread.currentThread().interrupt(); + } + catch (BrokenBarrierException ex) { + throw new IllegalStateException(ex); + } + return response.setComplete(); + }); + this.webServer.start(); + new Thread(() -> getWebClient().build().get().uri("/").exchange().block()).start(); + barrier.await(); + this.webServer.stop(); + latch.countDown(); + } + protected WebClient prepareCompressionTest() { Compression compression = new Compression(); compression.setEnabled(true); diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactoryTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactoryTests.java index c7098f14eb0..56acfe94cc7 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactoryTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactoryTests.java @@ -45,7 +45,10 @@ import java.util.EnumSet; import java.util.HashMap; import java.util.Locale; import java.util.Map; +import java.util.concurrent.BrokenBarrierException; import java.util.concurrent.Callable; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.CyclicBarrier; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import java.util.zip.GZIPInputStream; @@ -986,6 +989,43 @@ public abstract class AbstractServletWebServerFactoryTests { .satisfies(this::wrapsFailingServletException); } + @Test + void whenARequestIsActiveThenStopWillComplete() throws InterruptedException, BrokenBarrierException { + AbstractServletWebServerFactory factory = getFactory(); + CyclicBarrier barrier = new CyclicBarrier(2); + CountDownLatch latch = new CountDownLatch(1); + this.webServer = factory.getWebServer((context) -> context.addServlet("blocking", new HttpServlet() { + + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) + throws ServletException, IOException { + try { + barrier.await(); + latch.await(); + } + catch (InterruptedException ex) { + Thread.currentThread().interrupt(); + } + catch (BrokenBarrierException ex) { + throw new ServletException(ex); + } + } + + }).addMapping("/")); + this.webServer.start(); + new Thread(() -> { + try { + getResponse(getLocalUrl("/")); + } + catch (Exception ex) { + // Continue + } + }).start(); + barrier.await(); + this.webServer.stop(); + latch.countDown(); + } + private void wrapsFailingServletException(WebServerException ex) { Throwable cause = ex.getCause(); while (cause != null) {