[GitHub] clebertsuconic commented on a change in pull request #2473: ARTEMIS-1058 Jars in web tmp dir locked on Windows

[GitBox <gi...@apache.org>]

clebertsuconic commented on a change in pull request #2473: ARTEMIS-1058 Jars in web tmp dir locked on Windows
URL: https://github.com/apache/activemq-artemis/pull/2473#discussion_r248722829
 
 

 ##########
 File path: artemis-web/src/main/java/org/apache/activemq/artemis/component/WebServerComponent.java
 ##########
 @@ -237,28 +236,39 @@ public void start() throws Exception {
    public void internalStop() throws Exception {
       server.stop();
       if (webContexts != null) {
+
          File tmpdir = null;
+         StringBuilder strBuilder = new StringBuilder();
+         boolean found = false;
          for (WebAppContext context : webContexts) {
             tmpdir = context.getTempDirectory();
 
-            if (tmpdir != null && !context.isPersistTempDirectory()) {
+            if (tmpdir != null && tmpdir.exists() && !context.isPersistTempDirectory()) {
                //tmpdir will be removed by deleteOnExit()
-               //somehow when broker is stopped and restarted quickly
-               //this tmpdir won't get deleted sometimes
-               boolean fileDeleted = TimeUtils.waitOnBoolean(false, 5000, tmpdir::exists);
-
-               if (!fileDeleted) {
-                  //because the execution order of shutdown hooks are
-                  //not determined, so it's possible that the deleteOnExit
-                  //is executed after this hook, in that case we force a delete.
-                  FileUtil.deleteDirectory(tmpdir);
-                  logger.debug("Force to delete temporary file on shutdown: " + tmpdir.getAbsolutePath());
-                  if (tmpdir.exists()) {
-                     ActiveMQWebLogger.LOGGER.tmpFileNotDeleted(tmpdir);
-                  }
+               //However because the URLClassLoader never release/close its opened
+               //jars the jar file won't be able to get deleted on Windows platform
+               //until after the process fully terminated. To fix this here arranges
+               //a separate process to try clean up the temp dir
+               FileUtil.deleteDirectory(tmpdir);
+               if (tmpdir.exists()) {
+                  ActiveMQWebLogger.LOGGER.tmpFileNotDeleted(tmpdir);
+                  strBuilder.append(tmpdir);
+                  strBuilder.append(",");
+                  found = true;
                }
             }
          }
+
+         if (found) {
+            String artemisHome = System.getProperty("artemis.home");
 
 Review comment:
   you're wrong actually,
   
   WebServerComponent is getting the artemisInstance and artemisHome as parameters.
   
   It is important you don't use the variable here as you may break embedded cases.
   
   We use things like this on our testsuite as well. so, you can use a property to store what's passed from the configure method, and use the variable directly here instead.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services