elharo commented on code in PR #112:
URL: https://github.com/apache/maven-shared-io/pull/112#discussion_r3720086948


##########
src/main/java/org/apache/maven/shared/io/download/DefaultDownloadManager.java:
##########
@@ -68,6 +114,142 @@ public DefaultDownloadManager(WagonManager wagonManager) {
         this.wagonManager = wagonManager;
     }
 
+    /**
+     * Deletes the temporary files downloaded through this manager and empties 
its cache, so that
+     * subsequent requests download again. Calling this is optional: the files 
are removed when the
+     * JVM exits anyway. It is worth calling in a long-lived JVM, such as a 
Maven daemon or an
+     * embedded build, once the downloaded files are no longer needed. Do not 
call it while a
+     * download is in progress on another thread, as that download writes into 
the directory being
+     * removed.
+     */
+    public void cleanup() {
+        cache.clear();
+
+        File directory;
+        synchronized (this) {
+            directory = downloadDirectory;
+            downloadDirectory = null;
+        }
+
+        if (directory != null) {
+            deleteRecursively(directory.toPath());
+        }
+    }
+
+    /**
+     * @return the directory of this manager, creating it, the shared root and 
the shutdown hook that
+     *         removes the root on first use.
+     * @throws IOException if the directory cannot be created.
+     */
+    private synchronized File downloadDirectory() throws IOException {
+        if (downloadDirectory == null || !downloadDirectory.isDirectory()) {
+            downloadDirectory = 
Files.createTempDirectory(downloadRoot().toPath(), "manager-")
+                    .toFile();
+        }
+
+        return downloadDirectory;
+    }
+
+    private static synchronized File downloadRoot() throws IOException {
+        // Recreate the root if it was removed behind our back, for instance 
by a temp dir sweeper.
+        if (downloadRoot == null || !downloadRoot.isDirectory()) {
+            downloadRoot =
+                    
Files.createTempDirectory("maven-shared-io-downloads-").toFile();
+            registerShutdownHook();
+        }
+
+        return downloadRoot;
+    }
+
+    /**
+     * Registers, at most once, the hook that removes {@link #downloadRoot} at 
JVM exit. Registering
+     * one hook for the lifetime of the class, instead of one per root, is 
what keeps the JVM's hook
+     * set from growing: a root that a temp dir sweeper removes is replaced 
without a second hook.
+     */
+    private static void registerShutdownHook() {
+        if (shutdownHookAttempted) {
+            return;
+        }
+
+        // Set before the attempt, so a failure is not retried on every 
recreation of the root.
+        shutdownHookAttempted = true;
+
+        Thread hook = new Thread(DefaultDownloadManager::deleteDownloadRoot, 
"maven-shared-io-download-cleanup");
+
+        // The hook lives until the JVM exits, so give it no more references 
than it needs. Clearing
+        // the inherited context class loader matters in Maven, where it is 
the class realm of the
+        // plugin that happened to download first, and the hook would keep 
that realm alive for the
+        // rest of a long lived JVM such as a daemon. The hook still holds the 
class loader that
+        // defined this class, which no shutdown hook running library code can 
avoid, but that is one
+        // reference for the whole class loader rather than one per downloaded 
file.
+        hook.setContextClassLoader(null);
+
+        try {
+            Runtime.getRuntime().addShutdownHook(hook);
+            registeredShutdownHooks++;
+        } catch (IllegalStateException e) {
+            // The JVM is already shutting down; no hook can be registered any 
more and the
+            // files are left to the operating system's temp directory cleanup.
+        } catch (SecurityException e) {
+            // Not allowed to register a hook. Downloading must still work, so 
fall back to the
+            // operating system's temp directory cleanup, as above.
+        }
+    }
+
+    /** Removes the current download root. Called only by the shutdown hook. */
+    private static void deleteDownloadRoot() {
+        File root;
+        synchronized (DefaultDownloadManager.class) {
+            root = downloadRoot;
+        }
+
+        if (root != null) {
+            deleteRecursively(root.toPath());
+        }
+    }
+
+    /**
+     * Deletes a file, or a directory and everything below it, ignoring 
failures. Symbolic links are
+     * removed without being followed, so nothing outside the tree is touched.
+     * <p>
+     * This uses only JDK types on purpose. It runs from a shutdown hook, at a 
point where the class
+     * loader that defined this class may have been closed, which happens to a 
Maven plugin realm at
+     * the end of a build. Any class not already loaded, {@code FileUtils} for 
one, could not be
+     * resolved then and the hook would fail without deleting anything.
+     *
+     * @param path the file or directory to delete.
+     */
+    private static void deleteRecursively(Path path) {

Review Comment:
   Let's not reinvent the wheel. There are existing methods to do this in 
various places. Maybe maven-shared-utils and definitely Apache commons. 
Probably other places too. This functionality is really hard to get right. 



-- 
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]

Reply via email to