From 6094212217c1a6e70c1b2182f1ef57d26be7eb20 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Thu, 7 Sep 2023 18:17:10 +0100 Subject: [PATCH] Defer accessing loop resources until web server start Closes gh-37209 --- .../netty/NettyReactiveWebServerFactory.java | 13 ++----- .../web/embedded/netty/NettyWebServer.java | 36 +++++++++++++++++++ .../NettyReactiveWebServerFactoryTests.java | 21 ++++++++++- 3 files changed, 58 insertions(+), 12 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/netty/NettyReactiveWebServerFactory.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/netty/NettyReactiveWebServerFactory.java index f351da622f..306077ffbd 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/netty/NettyReactiveWebServerFactory.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/netty/NettyReactiveWebServerFactory.java @@ -27,7 +27,6 @@ import java.util.Set; import reactor.netty.http.HttpProtocol; import reactor.netty.http.server.HttpServer; -import reactor.netty.resources.LoopResources; import org.springframework.boot.web.reactive.server.AbstractReactiveWebServerFactory; import org.springframework.boot.web.reactive.server.ReactiveWebServerFactory; @@ -78,7 +77,7 @@ public class NettyReactiveWebServerFactory extends AbstractReactiveWebServerFact NettyWebServer createNettyWebServer(HttpServer httpServer, ReactorHttpHandlerAdapter handlerAdapter, Duration lifecycleTimeout, Shutdown shutdown) { - return new NettyWebServer(httpServer, handlerAdapter, lifecycleTimeout, shutdown); + return new NettyWebServer(httpServer, handlerAdapter, lifecycleTimeout, shutdown, this.resourceFactory); } /** @@ -158,15 +157,7 @@ public class NettyReactiveWebServerFactory extends AbstractReactiveWebServerFact } private HttpServer createHttpServer() { - HttpServer server = HttpServer.create(); - if (this.resourceFactory != null) { - LoopResources resources = this.resourceFactory.getLoopResources(); - Assert.notNull(resources, "No LoopResources: is ReactorResourceFactory not initialized yet?"); - server = server.runOn(resources).bindAddress(this::getListenAddress); - } - else { - server = server.bindAddress(this::getListenAddress); - } + HttpServer server = HttpServer.create().bindAddress(this::getListenAddress); if (Ssl.isEnabled(getSsl())) { server = customizeSslConfiguration(server); } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/netty/NettyWebServer.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/netty/NettyWebServer.java index f21f47a700..208ce2a2e7 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/netty/NettyWebServer.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/netty/NettyWebServer.java @@ -35,6 +35,7 @@ import reactor.netty.http.server.HttpServer; import reactor.netty.http.server.HttpServerRequest; import reactor.netty.http.server.HttpServerResponse; import reactor.netty.http.server.HttpServerRoutes; +import reactor.netty.resources.LoopResources; import org.springframework.boot.web.server.GracefulShutdownCallback; import org.springframework.boot.web.server.GracefulShutdownResult; @@ -42,6 +43,7 @@ import org.springframework.boot.web.server.PortInUseException; import org.springframework.boot.web.server.Shutdown; import org.springframework.boot.web.server.WebServer; import org.springframework.boot.web.server.WebServerException; +import org.springframework.http.client.reactive.ReactorResourceFactory; import org.springframework.http.server.reactive.ReactorHttpHandlerAdapter; import org.springframework.util.Assert; @@ -74,12 +76,40 @@ public class NettyWebServer implements WebServer { private final GracefulShutdown gracefulShutdown; + private final ReactorResourceFactory resourceFactory; + private List routeProviders = Collections.emptyList(); private volatile DisposableServer disposableServer; + /** + * Creates a new {@code NettyWebServer} instance. + * @param httpServer the HTTP server + * @param handlerAdapter the handler adapter + * @param lifecycleTimeout the lifecycle timeout, may be {@code null} + * @param shutdown the shutdown, may be {@code null} + * @deprecated since 3.2.0 for removal in 3.4.0 in favor of + * {@link #NettyWebServer(HttpServer, ReactorHttpHandlerAdapter, Duration, Shutdown, ReactorResourceFactory)} + */ + @Deprecated(since = "3.2.0", forRemoval = true) public NettyWebServer(HttpServer httpServer, ReactorHttpHandlerAdapter handlerAdapter, Duration lifecycleTimeout, Shutdown shutdown) { + this(httpServer, handlerAdapter, lifecycleTimeout, shutdown, null); + } + + /** + * Creates a new {@code NettyWebServer} instance. + * @param httpServer the HTTP server + * @param handlerAdapter the handler adapter + * @param lifecycleTimeout the lifecycle timeout, may be {@code null} + * @param shutdown the shutdown, may be {@code null} + * @param resourceFactory the factory for the server's {@link LoopResources loop + * resources}, may be {@code null} + * @since 3.2.0 + * {@link #NettyWebServer(HttpServer, ReactorHttpHandlerAdapter, Duration, Shutdown, ReactorResourceFactory)} + */ + public NettyWebServer(HttpServer httpServer, ReactorHttpHandlerAdapter handlerAdapter, Duration lifecycleTimeout, + Shutdown shutdown, ReactorResourceFactory resourceFactory) { Assert.notNull(httpServer, "HttpServer must not be null"); Assert.notNull(handlerAdapter, "HandlerAdapter must not be null"); this.lifecycleTimeout = lifecycleTimeout; @@ -87,6 +117,7 @@ public class NettyWebServer implements WebServer { this.httpServer = httpServer.channelGroup(new DefaultChannelGroup(new DefaultEventExecutor())); this.gracefulShutdown = (shutdown == Shutdown.GRACEFUL) ? new GracefulShutdown(() -> this.disposableServer) : null; + this.resourceFactory = resourceFactory; } public void setRouteProviders(List routeProviders) { @@ -143,6 +174,11 @@ public class NettyWebServer implements WebServer { else { server = server.route(this::applyRouteProviders); } + if (this.resourceFactory != null) { + LoopResources resources = this.resourceFactory.getLoopResources(); + Assert.notNull(resources, "No LoopResources: is ReactorResourceFactory not initialized yet?"); + server = server.runOn(resources); + } if (this.lifecycleTimeout != null) { return server.bindNow(this.lifecycleTimeout); } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/netty/NettyReactiveWebServerFactoryTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/netty/NettyReactiveWebServerFactoryTests.java index b1e8a318aa..652042688c 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/netty/NettyReactiveWebServerFactoryTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/netty/NettyReactiveWebServerFactoryTests.java @@ -41,12 +41,14 @@ import org.springframework.boot.web.server.Shutdown; import org.springframework.boot.web.server.Ssl; import org.springframework.http.MediaType; import org.springframework.http.client.reactive.ReactorClientHttpConnector; +import org.springframework.http.client.reactive.ReactorResourceFactory; import org.springframework.http.server.reactive.ReactorHttpHandlerAdapter; import org.springframework.web.reactive.function.BodyInserters; import org.springframework.web.reactive.function.client.WebClient; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.assertj.core.api.Assertions.assertThatNoException; import static org.mockito.ArgumentMatchers.any; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.inOrder; @@ -80,6 +82,23 @@ class NettyReactiveWebServerFactoryTests extends AbstractReactiveWebServerFactor assertThat(this.webServer.getPort()).isEqualTo(-1); } + @Test + void resourceFactoryAndWebServerLifecycle() { + NettyReactiveWebServerFactory factory = getFactory(); + factory.setPort(0); + ReactorResourceFactory resourceFactory = new ReactorResourceFactory(); + factory.setResourceFactory(resourceFactory); + this.webServer = factory.getWebServer(new EchoHandler()); + assertThatNoException().isThrownBy(() -> { + resourceFactory.start(); + this.webServer.start(); + this.webServer.stop(); + resourceFactory.stop(); + resourceFactory.start(); + this.webServer.start(); + }); + } + private void portMatchesRequirement(PortInUseException exception) { assertThat(exception.getPort()).isEqualTo(this.webServer.getPort()); } @@ -199,7 +218,7 @@ class NettyReactiveWebServerFactoryTests extends AbstractReactiveWebServerFactor NoPortNettyWebServer(HttpServer httpServer, ReactorHttpHandlerAdapter handlerAdapter, Duration lifecycleTimeout, Shutdown shutdown) { - super(httpServer, handlerAdapter, lifecycleTimeout, shutdown); + super(httpServer, handlerAdapter, lifecycleTimeout, shutdown, null); } @Override