phaneendra-injarapu commented on code in PR #112:
URL: https://github.com/apache/maven-shared-io/pull/112#discussion_r3720485763
##########
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) {
Review Comment:
The synchronized block isn't guarding the deletion - it only publishes the
**downloadRoot** field, which is written under the same class lock in
**downloadRoot()**, so the hook doesn't read a stale value. I've reworded the
comment to say that.
One overlap is possible, and it's harmless: **cleanup()** running on another
thread while the hook deletes the root. The hook can't race itself, since it's
registered once and the JVM runs it on a single thread. Both paths now use
**FileUtils.deleteQuietly**, which ignores failures, so the loser of that race
either finds the entry already gone or leaves onedirectory to the OS temp sweep
- the fallback this class already relies on. Nothing throws.
--
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]
