From 84bde2a836d804658330fd8388c78832080b4a71 Mon Sep 17 00:00:00 2001 From: Dmytro Nosan Date: Wed, 5 Feb 2025 12:48:53 +0200 Subject: [PATCH 1/2] Catch WebServer stop or destroy exception when context refresh fails This commit adds the stop or destroy failure as a suppressed exception if either ServletWebServerApplicationContext or ReactiveWebServerApplicationContext refresh fails. See gh-44310 Signed-off-by: Dmytro Nosan --- .../ReactiveWebServerApplicationContext.java | 13 ++++-- .../ServletWebServerApplicationContext.java | 17 ++++--- ...ctiveWebServerApplicationContextTests.java | 35 ++++++++++++++ ...rvletWebServerApplicationContextTests.java | 46 ++++++++++++++++++- 4 files changed, 100 insertions(+), 11 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/context/ReactiveWebServerApplicationContext.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/context/ReactiveWebServerApplicationContext.java index 9ff12bd8238..8c787dd995a 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/context/ReactiveWebServerApplicationContext.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/context/ReactiveWebServerApplicationContext.java @@ -65,13 +65,18 @@ public class ReactiveWebServerApplicationContext extends GenericReactiveWebAppli try { super.refresh(); } - catch (RuntimeException ex) { + catch (RuntimeException refreshEx) { WebServer webServer = getWebServer(); if (webServer != null) { - webServer.stop(); - webServer.destroy(); + try { + webServer.stop(); + webServer.destroy(); + } + catch (RuntimeException stopOrDestroyEx) { + refreshEx.addSuppressed(stopOrDestroyEx); + } } - throw ex; + throw refreshEx; } } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/context/ServletWebServerApplicationContext.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/context/ServletWebServerApplicationContext.java index 6562e68d1e8..2555919b957 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/context/ServletWebServerApplicationContext.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/context/ServletWebServerApplicationContext.java @@ -145,13 +145,18 @@ public class ServletWebServerApplicationContext extends GenericWebApplicationCon try { super.refresh(); } - catch (RuntimeException ex) { - WebServer webServer = this.webServer; - if (webServer != null) { - webServer.stop(); - webServer.destroy(); + catch (RuntimeException refreshEx) { + try { + WebServer webServer = this.webServer; + if (webServer != null) { + webServer.stop(); + webServer.destroy(); + } } - throw ex; + catch (RuntimeException stopOrDestroyEx) { + refreshEx.addSuppressed(stopOrDestroyEx); + } + throw refreshEx; } } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/context/ReactiveWebServerApplicationContextTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/context/ReactiveWebServerApplicationContextTests.java index e87deafa73c..1c91af3fe14 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/context/ReactiveWebServerApplicationContextTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/context/ReactiveWebServerApplicationContextTests.java @@ -43,6 +43,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; import static org.mockito.BDDMockito.then; +import static org.mockito.BDDMockito.willThrow; import static org.mockito.Mockito.times; /** @@ -133,6 +134,40 @@ class ReactiveWebServerApplicationContextTests { then(webServer).should().destroy(); } + @Test + void whenContextRefreshFailedThenWebServerStopFailedCatchStopException() { + addWebServerFactoryBean(); + addHttpHandlerBean(); + this.context.registerBeanDefinition("refreshFailure", new RootBeanDefinition(RefreshFailure.class, () -> { + willThrow(new RuntimeException("WebServer has failed to stop")).willCallRealMethod() + .given(this.context.getWebServer()) + .stop(); + return new RefreshFailure(); + })); + assertThatExceptionOfType(BeanCreationException.class).isThrownBy(this.context::refresh) + .withStackTraceContaining("WebServer has failed to stop"); + WebServer webServer = this.context.getWebServer(); + then(webServer).should().stop(); + then(webServer).should(times(0)).destroy(); + } + + @Test + void whenContextRefreshFailedThenWebServerIsStoppedAndDestroyFailedCatchDestroyException() { + addWebServerFactoryBean(); + addHttpHandlerBean(); + this.context.registerBeanDefinition("refreshFailure", new RootBeanDefinition(RefreshFailure.class, () -> { + willThrow(new RuntimeException("WebServer has failed to destroy")).willCallRealMethod() + .given(this.context.getWebServer()) + .destroy(); + return new RefreshFailure(); + })); + assertThatExceptionOfType(BeanCreationException.class).isThrownBy(this.context::refresh) + .withStackTraceContaining("WebServer has failed to destroy"); + WebServer webServer = this.context.getWebServer(); + then(webServer).should().stop(); + then(webServer).should().destroy(); + } + @Test void whenContextIsClosedThenWebServerIsStoppedAndDestroyed() { addWebServerFactoryBean(); diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/context/ServletWebServerApplicationContextTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/context/ServletWebServerApplicationContextTests.java index 694fb3828e4..d474256bf60 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/context/ServletWebServerApplicationContextTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/context/ServletWebServerApplicationContextTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2024 the original author or authors. + * Copyright 2012-2025 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. @@ -51,6 +51,7 @@ import org.springframework.boot.availability.AvailabilityChangeEvent; import org.springframework.boot.testsupport.system.CapturedOutput; import org.springframework.boot.testsupport.system.OutputCaptureExtension; import org.springframework.boot.web.context.ServerPortInfoApplicationContextInitializer; +import org.springframework.boot.web.server.WebServer; import org.springframework.boot.web.servlet.DelegatingFilterProxyRegistrationBean; import org.springframework.boot.web.servlet.FilterRegistrationBean; import org.springframework.boot.web.servlet.ServletContextInitializer; @@ -81,6 +82,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.BDDMockito.given; import static org.mockito.BDDMockito.then; +import static org.mockito.BDDMockito.willThrow; import static org.mockito.Mockito.atMost; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; @@ -211,6 +213,48 @@ class ServletWebServerApplicationContextTests { assertThat(listener.receivedEvents()).isEmpty(); } + @Test + void whenContextRefreshFailedThenWebServerIsStoppedAndDestroyed() { + addWebServerFactoryBean(); + this.context.registerBeanDefinition("refreshFailure", new RootBeanDefinition(RefreshFailure.class)); + assertThatExceptionOfType(BeanCreationException.class).isThrownBy(this.context::refresh); + WebServer webServer = this.context.getWebServer(); + then(webServer).should(times(2)).stop(); + then(webServer).should().destroy(); + } + + @Test + void whenContextRefreshFailedThenWebServerStopFailedCatchStopException() { + addWebServerFactoryBean(); + this.context.registerBeanDefinition("refreshFailure", new RootBeanDefinition(RefreshFailure.class, () -> { + willThrow(new RuntimeException("WebServer has failed to stop")).willCallRealMethod() + .given(this.context.getWebServer()) + .stop(); + return new RefreshFailure(); + })); + assertThatExceptionOfType(BeanCreationException.class).isThrownBy(this.context::refresh) + .withStackTraceContaining("WebServer has failed to stop"); + WebServer webServer = this.context.getWebServer(); + then(webServer).should().stop(); + then(webServer).should(times(0)).destroy(); + } + + @Test + void whenContextRefreshFailedThenWebServerIsStoppedAndDestroyFailedCatchDestroyException() { + addWebServerFactoryBean(); + this.context.registerBeanDefinition("refreshFailure", new RootBeanDefinition(RefreshFailure.class, () -> { + willThrow(new RuntimeException("WebServer has failed to destroy")).willCallRealMethod() + .given(this.context.getWebServer()) + .destroy(); + return new RefreshFailure(); + })); + assertThatExceptionOfType(BeanCreationException.class).isThrownBy(this.context::refresh) + .withStackTraceContaining("WebServer has failed to destroy"); + WebServer webServer = this.context.getWebServer(); + then(webServer).should().stop(); + then(webServer).should().destroy(); + } + @Test void cannotSecondRefresh() { addWebServerFactoryBean(); From 062b73f4a43b2f36e3f0030a06a7b73cec8dde75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Nicoll?= Date: Tue, 18 Feb 2025 10:59:52 +0100 Subject: [PATCH 2/2] Polish contribution See gh-44310 --- .../context/ReactiveWebServerApplicationContext.java | 6 +++--- .../servlet/context/ServletWebServerApplicationContext.java | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/context/ReactiveWebServerApplicationContext.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/context/ReactiveWebServerApplicationContext.java index 8c787dd995a..9e6bef52194 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/context/ReactiveWebServerApplicationContext.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/context/ReactiveWebServerApplicationContext.java @@ -65,7 +65,7 @@ public class ReactiveWebServerApplicationContext extends GenericReactiveWebAppli try { super.refresh(); } - catch (RuntimeException refreshEx) { + catch (RuntimeException ex) { WebServer webServer = getWebServer(); if (webServer != null) { try { @@ -73,10 +73,10 @@ public class ReactiveWebServerApplicationContext extends GenericReactiveWebAppli webServer.destroy(); } catch (RuntimeException stopOrDestroyEx) { - refreshEx.addSuppressed(stopOrDestroyEx); + ex.addSuppressed(stopOrDestroyEx); } } - throw refreshEx; + throw ex; } } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/context/ServletWebServerApplicationContext.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/context/ServletWebServerApplicationContext.java index 2555919b957..cc8e6a9650b 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/context/ServletWebServerApplicationContext.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/context/ServletWebServerApplicationContext.java @@ -145,7 +145,7 @@ public class ServletWebServerApplicationContext extends GenericWebApplicationCon try { super.refresh(); } - catch (RuntimeException refreshEx) { + catch (RuntimeException ex) { try { WebServer webServer = this.webServer; if (webServer != null) { @@ -154,9 +154,9 @@ public class ServletWebServerApplicationContext extends GenericWebApplicationCon } } catch (RuntimeException stopOrDestroyEx) { - refreshEx.addSuppressed(stopOrDestroyEx); + ex.addSuppressed(stopOrDestroyEx); } - throw refreshEx; + throw ex; } }