codeconsole commented on code in PR #16741:
URL: https://github.com/apache/grails-core/pull/16741#discussion_r4362401058
##########
grails-undertow/spring-boot/src/main/java/org/apache/grails/undertow/core/servlet/UndertowServletWebServerFactory.java:
##########
@@ -219,12 +222,13 @@ public void setPreservePathOnForward(boolean
preservePathOnForward) {
@Override
public WebServer getWebServer(ServletContextInitializer... initializers) {
Builder builder = createBuilder(this, this::getSslBundle,
this::getServerNameSslBundles);
- DeploymentManager manager = createManager(initializers);
+ DeploymentManager manager = createManager(new TempDirs(getPort()),
initializers);
return getUndertowWebServer(builder, manager, getPort());
}
- private DeploymentManager createManager(ServletContextInitializer...
initializers) {
+ private DeploymentManager createManager(TempDirs tempDirs,
ServletContextInitializer... initializers) {
DeploymentInfo deployment = Servlets.deployment();
+ deployment.addDeploymentCompleteListener(new
CleanTempDirsListener(tempDirs));
Review Comment:
Switched in b4d3b81eba. `CleanTempDirsListener` is now an ordinary listener,
added first to the `DeploymentInfo` as you suggested. The servlet context
destroys its listeners in reverse order, so the cleanup runs after the
application's own `contextDestroyed`, as on Tomcat and Jetty.
Your probe is now
`theApplicationsListenersStillFindTheTemporaryDirectoryWhenTheContextIsDestroyed`.
It registers a listener through a `ServletContextInitializer` and records
`((File) servletContext.getAttribute(ServletContext.TEMPDIR)).isDirectory()` in
`contextDestroyed`. It records `true`, and the directory is gone once `stop()`
returns. With the deployment-complete listener put back, the test fails and the
rest of the class passes.
##########
grails-undertow/spring-boot/src/main/java/org/apache/grails/undertow/core/servlet/UndertowServletWebServer.java:
##########
@@ -92,4 +92,17 @@ public DeploymentManager getDeploymentManager() {
return this.manager;
}
+ /**
+ * Stops the server, which undeploys the deployment of a server that
started, and
+ * undeploys the deployment of a server that never started, so that the
listeners of
+ * its servlet context are destroyed and its temporary directories deleted.
+ */
+ @Override
+ public void destroy() {
+ super.destroy();
+ if (this.manager != null && this.manager.getState() ==
DeploymentManager.State.DEPLOYED) {
Review Comment:
Done in 0e44ba1546 with your guard. `destroy()` now does three things:
- stops a `STARTED` deployment, wrapping a `ServletException` in
`WebServerException`;
- undeploys any deployment that isn't `UNDEPLOYED`;
- returns early when the server has no deployment manager.
The new test,
`aServerWhoseHandlersFailedToBuildIsUndeployedWhenItIsDestroyed`, sits next to
the never-started one. It enables the access log with its directory under a
regular file, so `start()` throws `WebServerException`. It then calls `stop()`
and `destroy()`, as Spring Boot does with the failed server. After that, a
listener the application registered has had its `contextDestroyed` run, and the
`undertow.<port>.*` directory is gone. With the old `DEPLOYED`-only guard put
back, this test fails and the rest of the class passes. All 71 module tests
pass, and `codeStyle` is clean.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
