From 5c8de555ae4cbbb06e819f8f922c114a09cbb511 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20G=C3=B3mez-S=C3=A1nchez?=
 <magicDGS@users.noreply.github.com>
Date: Mon, 5 Feb 2018 18:54:46 +0100
Subject: [PATCH] Add DeleteOnExitPathHook to register java.nio.Path for
 deletion (#1037)

* Add DeleteOnExitPathHook to register java.nio.Path for deletion, this closely mirrors the equivalent java built in `DeleteOnExitHook` but uses paths instead of files.
---
 .../java/htsjdk/samtools/util/IOUtil.java     | 31 ++-------
 .../util/nio/DeleteOnExitPathHook.java        | 63 +++++++++++++++++++
 .../java/htsjdk/samtools/util/IoUtilTest.java | 15 -----
 3 files changed, 68 insertions(+), 41 deletions(-)
 create mode 100644 src/main/java/htsjdk/samtools/util/nio/DeleteOnExitPathHook.java

diff --git a/src/main/java/htsjdk/samtools/util/IOUtil.java b/src/main/java/htsjdk/samtools/util/IOUtil.java
index 26aaa1f4aa..7f6bfac420 100644
--- a/src/main/java/htsjdk/samtools/util/IOUtil.java
+++ b/src/main/java/htsjdk/samtools/util/IOUtil.java
@@ -31,6 +31,7 @@
 import htsjdk.samtools.seekablestream.SeekableHTTPStream;
 import htsjdk.samtools.seekablestream.SeekableStream;
 import htsjdk.tribble.Tribble;
+import htsjdk.samtools.util.nio.DeleteOnExitPathHook;
 
 import java.io.BufferedInputStream;
 import java.io.BufferedOutputStream;
@@ -353,34 +354,12 @@ public static Path getDefaultTmpDirPath() {
     }
 
     /**
-     * Deletes on exit a path by creating a shutdown hook.
-     */
-    public static void deleteOnExit(final Path path) {
-        // add a shutdown hook to remove the path on exit
-        Runtime.getRuntime().addShutdownHook(new DeletePathThread(path));
-    }
-
-    /**
-     * WARNING: visible for testing. Do not use.
-     *
-     * Class for delete a path, used in a shutdown hook for delete on exit.
+     * Register a {@link Path} for deletion on JVM exit.
      *
-     * @see #deleteOnExit(Path)
+     * @see DeleteOnExitPathHook
      */
-    static final class DeletePathThread extends Thread {
-
-        private final Path path;
-
-        DeletePathThread(Path path) {this.path = path;}
-
-        @Override
-        public void run() {
-            try {
-                Files.deleteIfExists(path);
-            } catch (IOException e) {
-                throw new RuntimeIOException(e);
-            }
-        }
+    public static void deleteOnExit(final Path path) {
+        DeleteOnExitPathHook.add(path);
     }
 
     /** Returns the name of the file minus the extension (i.e. text after the last "." in the filename). */
diff --git a/src/main/java/htsjdk/samtools/util/nio/DeleteOnExitPathHook.java b/src/main/java/htsjdk/samtools/util/nio/DeleteOnExitPathHook.java
new file mode 100644
index 0000000000..ae2a0dd46f
--- /dev/null
+++ b/src/main/java/htsjdk/samtools/util/nio/DeleteOnExitPathHook.java
@@ -0,0 +1,63 @@
+package htsjdk.samtools.util.nio;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.LinkedHashSet;
+
+/**
+ * Class to hold a set of {@link Path} to be delete on the JVM exit through a shutdown hook.
+ *
+ * <p>This class is a modification of {@link java.io.DeleteOnExitHook} to handle {@link Path}
+ * instead of {@link java.io.File}.
+ *
+ * @author Daniel Gomez-Sanchez (magicDGS)
+ */
+public class DeleteOnExitPathHook {
+    private static LinkedHashSet<Path> paths = new LinkedHashSet<>();
+    static {
+        Runtime.getRuntime().addShutdownHook(new Thread(DeleteOnExitPathHook::runHooks));
+    }
+
+    private DeleteOnExitPathHook() {}
+
+    /**
+     * Adds a {@link Path} for deletion on JVM exit.
+     *
+     * @param path path to be deleted.
+     *
+     * @throws IllegalStateException if the shutdown hook is in progress.
+     */
+    public static synchronized void add(Path path) {
+        if(paths == null) {
+            // DeleteOnExitHook is running. Too late to add a file
+            throw new IllegalStateException("Shutdown in progress");
+        }
+
+        paths.add(path);
+    }
+
+    static void runHooks() {
+        LinkedHashSet<Path> thePaths;
+
+        synchronized (DeleteOnExitPathHook.class) {
+            thePaths = paths;
+            paths = null;
+        }
+
+        ArrayList<Path> toBeDeleted = new ArrayList<>(thePaths);
+
+        // reverse the list to maintain previous jdk deletion order.
+        // Last in first deleted.
+        Collections.reverse(toBeDeleted);
+        for (Path path : toBeDeleted) {
+            try {
+                Files.delete(path);
+            } catch (IOException | SecurityException e) {
+                // do nothing if cannot be deleted, because it is a shutdown hook
+            }
+        }
+    }
+}
diff --git a/src/test/java/htsjdk/samtools/util/IoUtilTest.java b/src/test/java/htsjdk/samtools/util/IoUtilTest.java
index f4e31024f2..51fd76986a 100644
--- a/src/test/java/htsjdk/samtools/util/IoUtilTest.java
+++ b/src/test/java/htsjdk/samtools/util/IoUtilTest.java
@@ -352,21 +352,6 @@ private List<Path> createJimsFiles(final String folderName, final List<String> f
         return paths;
     }
 
-    @DataProvider
-    public Object[][] pathsForDeletePathThread() throws Exception {
-        return new Object[][] {
-                {File.createTempFile("testDeletePathThread", "file").toPath()},
-                {Files.createFile(inMemoryfileSystem.getPath("testDeletePathThread"))}
-        };
-    }
-
-    @Test(dataProvider = "pathsForDeletePathThread")
-    public void testDeletePathThread(final Path path) throws Exception {
-        Assert.assertTrue(Files.exists(path));
-        new IOUtil.DeletePathThread(path).run();
-        Assert.assertFalse(Files.exists(path));
-    }
-
     @DataProvider
     public Object[][] pathsForWritableDirectory() throws Exception {
         return new Object[][] {