WebServer is not destroyed when ReactiveWebServerApplicationContext refresh fails #44134
Conversation
spring-projects-issues
added
the
status: waiting-for-triage
| WebServer webServer = getWebServer(); | ||
| if (webServer != null) { | ||
| webServer.stop(); | ||
| webServer.destroy(); |
There was a problem hiding this comment.
Doesn't it make sense to destroy webServer even if stop() throws an exception?
Probably out of the scope for this fix, but it looks like Juergen was thinking about calling destroy() method in context of a bean lifecycle: #34955 (comment)
Thank you for the quick action in regard #44101! 🙏
There was a problem hiding this comment.
I don’t think so. If stop() throws an exception, it likely means that some resources, such as files, networks, etc. are still in use, which could cause destroy() to fail as well.
|
An alternative approach is 3.3.x...nosan:spring-boot:44101-alternative, which always tries to stop and destroy |
| } | ||
| } | ||
| catch (Exception shutdownEx) { | ||
| ex.addSuppressed(shutdownEx); |
There was a problem hiding this comment.
Let's consider this separately please. If we do it, it should be applied on the servlet side as well.
There was a problem hiding this comment.
Sure, I've reverted these changes.
There was a problem hiding this comment.
We think that adding the stop or destroy failure as a suppressed exception is a good idea. Would you like to open a PR for it or would you prefer that we take care of it?
wilkinsona
changed the title
wilkinsona
added
type: bug
wilkinsona
added
the
status: waiting-for-feedback
Prior to this commit, if ReactiveWebServerApplicationContext failed to refresh, only WebServer.stop() was called. This commit additionally invokes WebServer.destroy(), aligning the behavior with ServletWebServerApplicationContext when a refresh failure occurs. Signed-off-by: Dmytro Nosan <dimanosan@gmail.com>
snicoll
removed
the
status: waiting-for-feedback
Prior to this commit, if ReactiveWebServerApplicationContext failed to refresh, only WebServer.stop() was called. This commit additionally invokes WebServer.destroy(), aligning the behavior with ServletWebServerApplicationContext when a refresh failure occurs. See gh-44134 Signed-off-by: Dmytro Nosan <dimanosan@gmail.com>
Prior to this commit, if
ReactiveWebServerApplicationContextfailed to refresh, onlyWebServer.stop()was called.This commit additionally invokes
WebServer.destroy(), aligning the behaviour withServletWebServerApplicationContextwhen a refresh failure occurs.gh-44101
I am also thinking about these changes as well:
3.3.x...nosan:spring-boot:44100-1