GUACAMOLE-1508: Automatically delete temporary files on webapp shutdown.

guacamole/src/main/java/org/apache/guacamole/GuacamoleServletContextListener.java

@@ -19,6 +19,7 @@
 
 package org.apache.guacamole;
 
+import com.google.common.collect.Lists;
 import org.apache.guacamole.tunnel.TunnelModule;
 import com.google.inject.Guice;
 import com.google.inject.Injector;
@@ -117,6 +118,14 @@ public class GuacamoleServletContextListener extends GuiceServletContextListener
     @Inject
     private List<AuthenticationProvider> authProviders;
 
+    /**
+     * All temporary files that should be deleted upon application shutdown, in
+     * reverse order of desired deletion. This will typically simply be the
+     * order that each file was created.
+     */
+    @Inject
+    private List<File> temporaryFiles;
+    
     /**
      * Internal reference to the Guice injector that was lazily created when
      * getInjector() was first invoked.
@@ -194,20 +203,49 @@ public class GuacamoleServletContextListener extends GuiceServletContextListener
         });
     }
 
+    /**
+     * Deletes the given temporary file/directory, if possible. If the deletion
+     * operation fails, a warning is logged noting the failure. If the given
+     * file is a directory, it will only be deleted if empty.
+     *
+     * @param temp
+     *     The temporary file to delete.
+     */
+    private void deleteTemporaryFile(File temp) {
+        if (!temp.delete()) {
+            logger.warn("Temporary file/directory \"{}\" could not be "
+                    + "deleted. The file may remain until the JVM exits, or "
+                    + "may need to be manually deleted.", temp);
+        }
+        else
+            logger.debug("Deleted temporary file/directory \"{}\".", temp);
+    }
+    
     @Override
     public void contextDestroyed(ServletContextEvent servletContextEvent) {
+        try {
+        
+            // Clean up reference to Guice injector
+            servletContextEvent.getServletContext().removeAttribute(GUICE_INJECTOR);
 
-        // Clean up reference to Guice injector
+            // Shutdown TokenSessionMap
-        servletContextEvent.getServletContext().removeAttribute(GUICE_INJECTOR);
+            if (sessionMap != null)
+                sessionMap.shutdown();
 
-        // Shutdown TokenSessionMap
+            // Unload all extensions
-        if (sessionMap != null)
+            if (authProviders != null) {
-            sessionMap.shutdown();
+                for (AuthenticationProvider authProvider : authProviders)
+                    authProvider.shutdown();
+            }
+
+        }
+        finally {
+
+            // Regardless of what may succeed/fail here, always attempt to
+            // clean up ALL temporary files
+            if (temporaryFiles != null)
+                Lists.reverse(temporaryFiles).stream().forEachOrdered(this::deleteTemporaryFile);
 
-        // Unload all extensions
-        if (authProviders != null) {
-            for (AuthenticationProvider authProvider : authProviders)
-                authProvider.shutdown();
         }
 
         // Continue any Guice-specific cleanup

guacamole/src/main/java/org/apache/guacamole/extension/Extension.java

@@ -27,6 +27,7 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.LinkedHashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipException;
@@ -355,12 +356,18 @@ public class Extension {
      * @param file
      *     The file to load as an extension.
      *
+     * @param temporaryFiles
+     *     A modifiable List that should be populated with all temporary files
+     *     created for this extension. These files should be deleted on
+     *     application shutdown in reverse order.
+     *
      * @throws GuacamoleException
      *     If the provided file is not a .jar file, does not contain the
      *     guac-manifest.json, or if guac-manifest.json is invalid and cannot
      *     be parsed.
      */
-    public Extension(final ClassLoader parent, final File file) throws GuacamoleException {
+    public Extension(final ClassLoader parent, final File file,
+            final List<File> temporaryFiles) throws GuacamoleException {
 
         // Associate extension abstraction with original file
         this.file = file;
@@ -390,7 +397,7 @@ public class Extension {
             }
 
             // Create isolated classloader for this extension
-            classLoader = ExtensionClassLoader.getInstance(file, parent);
+            classLoader = ExtensionClassLoader.getInstance(file, temporaryFiles, parent);
 
         }
 

guacamole/src/main/java/org/apache/guacamole/extension/ExtensionClassLoader.java

@@ -84,6 +84,11 @@ public class ExtensionClassLoader extends URLClassLoader {
      * @param extension
      *     The extension .jar file from which classes should be loaded.
      *
+     * @param temporaryFiles
+     *     A modifiable List that should be populated with all temporary files
+     *     created for the given extension. These files should be deleted on
+     *     application shutdown in reverse order.
+     *
      * @param parent
      *     The ClassLoader to use if class resolution through the extension
      *     .jar fails.
@@ -97,7 +102,8 @@ public class ExtensionClassLoader extends URLClassLoader {
      *     file cannot be read.
      */
     public static ExtensionClassLoader getInstance(final File extension,
-            final ClassLoader parent) throws GuacamoleException {
+            final List<File> temporaryFiles, final ClassLoader parent)
+            throws GuacamoleException {
 
         try {
             // Attempt to create classloader which loads classes from the given
@@ -106,7 +112,7 @@ public class ExtensionClassLoader extends URLClassLoader {
 
                 @Override
                 public ExtensionClassLoader run() throws GuacamoleException {
-                    return new ExtensionClassLoader(extension, parent);
+                    return new ExtensionClassLoader(extension, temporaryFiles, parent);
                 }
 
             });
@@ -194,6 +200,11 @@ public class ExtensionClassLoader extends URLClassLoader {
      * @param extension
      *     The extension .jar file to generate URLs for.
      *
+     * @param temporaryFiles
+     *     A modifiable List that should be populated with all temporary files
+     *     created for the given extension. These files should be deleted on
+     *     application shutdown in reverse order.
+     *
      * @return
      *     An array of all URLs relevant to the given extension .jar.
      *
@@ -202,8 +213,8 @@ public class ExtensionClassLoader extends URLClassLoader {
      *     cannot be read, or any necessary temporary files/directories cannot
      *     be created.
      */
-    private static URL[] getExtensionURLs(File extension)
+    private static URL[] getExtensionURLs(File extension,
-            throws GuacamoleException {
+            List<File> temporaryFiles) throws GuacamoleException {
 
         JarFile extensionJar;
         try {
@@ -238,6 +249,7 @@ public class ExtensionClassLoader extends URLClassLoader {
             try {
                 if (extensionTempLibDir == null) {
                     extensionTempLibDir = Files.createTempDirectory(EXTENSION_TEMP_DIR_PREFIX);
+                    temporaryFiles.add(extensionTempLibDir.toFile());
                     extensionTempLibDir.toFile().deleteOnExit();
                 }
             }
@@ -252,6 +264,7 @@ public class ExtensionClassLoader extends URLClassLoader {
             File tempLibrary;
             try {
                 tempLibrary = Files.createTempFile(extensionTempLibDir, EXTENSION_TEMP_LIB_PREFIX, ".jar").toFile();
+                temporaryFiles.add(tempLibrary);
                 tempLibrary.deleteOnExit();
             }
             catch (IOException e) {
@@ -295,6 +308,11 @@ public class ExtensionClassLoader extends URLClassLoader {
      * @param extension
      *     The extension .jar file from which classes should be loaded.
      *
+     * @param temporaryFiles
+     *     A modifiable List that should be populated with all temporary files
+     *     created for the given extension. These files should be deleted on
+     *     application shutdown in reverse order.
+     *
      * @param parent
      *     The ClassLoader to use if class resolution through the extension
      *     .jar fails.
@@ -303,9 +321,9 @@ public class ExtensionClassLoader extends URLClassLoader {
      *     If the given file is not actually a file, or the contents of the
      *     file cannot be read.
      */
-    private ExtensionClassLoader(File extension, ClassLoader parent)
+    private ExtensionClassLoader(File extension, List<File> temporaryFiles,
-            throws GuacamoleException {
+            ClassLoader parent) throws GuacamoleException {
-        super(getExtensionURLs(extension), null);
+        super(getExtensionURLs(extension, temporaryFiles), null);
         this.parent = parent;
     }
 

guacamole/src/main/java/org/apache/guacamole/extension/ExtensionModule.java

@@ -140,6 +140,13 @@ public class ExtensionModule extends ServletModule {
     private final List<Listener> boundListeners =
             new ArrayList<Listener>();
 
+    /**
+     * All temporary files that should be deleted upon application shutdown, in
+     * reverse order of desired deletion. This will typically simply be the
+     * order that each file was created.
+     */
+    private final List<File> temporaryFiles = new ArrayList<>();
+
     /**
      * Service for adding and retrieving language resources.
      */
@@ -261,6 +268,20 @@ public class ExtensionModule extends ServletModule {
         return Collections.unmodifiableList(boundAuthenticationProviders);
     }
 
+    /**
+     * Returns a list of all temporary files that should be deleted upon
+     * application shutdown, in reverse order of desired deletion. This will
+     * typically simply be the order that each file was created.
+     *
+     * @return
+     *     A List of all temporary files that should be deleted upon
+     *     application shutdown. The List is not modifiable.
+     */
+    @Provides
+    public List<File> getTemporaryFiles() {
+        return Collections.unmodifiableList(temporaryFiles);
+    }
+    
     /**
      * Binds the given provider class such that a listener is bound for each
      * listener interface implemented by the provider and such that all bound
@@ -479,7 +500,7 @@ public class ExtensionModule extends ServletModule {
             try {
 
                 // Load extension from file
-                Extension extension = new Extension(getParentClassLoader(), extensionFile);
+                Extension extension = new Extension(getParentClassLoader(), extensionFile, temporaryFiles);
 
                 // Validate Guacamole version of extension
                 if (!isCompatible(extension.getGuacamoleVersion())) {