diff --git a/org.eclipse.jgit.junit/META-INF/MANIFEST.MF b/org.eclipse.jgit.junit/META-INF/MANIFEST.MF index 7f85f0c06..addb737f5 100644 --- a/org.eclipse.jgit.junit/META-INF/MANIFEST.MF +++ b/org.eclipse.jgit.junit/META-INF/MANIFEST.MF @@ -15,6 +15,7 @@ Import-Package: org.eclipse.jgit.annotations;version="[6.8.0,6.9.0)", org.eclipse.jgit.errors;version="[6.8.0,6.9.0)", org.eclipse.jgit.internal.storage.file;version="[6.8.0,6.9.0)", org.eclipse.jgit.internal.storage.pack;version="[6.8.0,6.9.0)", + org.eclipse.jgit.internal.util;version="[6.8.0,6.9.0)", org.eclipse.jgit.lib;version="[6.8.0,6.9.0)", org.eclipse.jgit.merge;version="[6.8.0,6.9.0)", org.eclipse.jgit.revwalk;version="[6.8.0,6.9.0)", diff --git a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/LocalDiskRepositoryTestCase.java b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/LocalDiskRepositoryTestCase.java index f816158b1..407290a39 100644 --- a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/LocalDiskRepositoryTestCase.java +++ b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/LocalDiskRepositoryTestCase.java @@ -20,7 +20,6 @@ import java.io.IOException; import java.io.PrintStream; import java.time.Instant; -import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -28,10 +27,12 @@ import java.util.Map; import java.util.Set; import java.util.TreeSet; +import java.util.concurrent.ConcurrentHashMap; import org.eclipse.jgit.dircache.DirCache; import org.eclipse.jgit.dircache.DirCacheEntry; import org.eclipse.jgit.internal.storage.file.FileRepository; +import org.eclipse.jgit.internal.util.ShutdownHook; import org.eclipse.jgit.lib.ConfigConstants; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; @@ -114,7 +115,7 @@ private String getTestName() { @Before public void setUp() throws Exception { tmp = File.createTempFile("jgit_" + getTestName() + '_', "_tmp"); - CleanupThread.deleteOnShutdown(tmp); + Cleanup.deleteOnShutdown(tmp); if (!tmp.delete() || !tmp.mkdir()) { throw new IOException("Cannot create " + tmp); } @@ -222,7 +223,7 @@ public void tearDown() throws Exception { recursiveDelete(tmp, false, true); } if (tmp != null && !tmp.exists()) { - CleanupThread.removed(tmp); + Cleanup.removed(tmp); } SystemReader.setInstance(null); } @@ -623,29 +624,28 @@ private static HashMap cloneEnv() { return new HashMap<>(System.getenv()); } - private static final class CleanupThread extends Thread { - private static final CleanupThread me; + private static final class Cleanup { + private static final Cleanup INSTANCE = new Cleanup(); + static { - me = new CleanupThread(); - Runtime.getRuntime().addShutdownHook(me); + ShutdownHook.INSTANCE.register(() -> INSTANCE.onShutdown()); + } + + private final Set toDelete = ConcurrentHashMap.newKeySet(); + + private Cleanup() { + // empty } static void deleteOnShutdown(File tmp) { - synchronized (me) { - me.toDelete.add(tmp); - } + INSTANCE.toDelete.add(tmp); } static void removed(File tmp) { - synchronized (me) { - me.toDelete.remove(tmp); - } + INSTANCE.toDelete.remove(tmp); } - private final List toDelete = new ArrayList<>(); - - @Override - public void run() { + private void onShutdown() { // On windows accidentally open files or memory // mapped regions may prevent files from being deleted. // Suggesting a GC increases the likelihood that our diff --git a/org.eclipse.jgit/META-INF/MANIFEST.MF b/org.eclipse.jgit/META-INF/MANIFEST.MF index 96eb18870..a62890de4 100644 --- a/org.eclipse.jgit/META-INF/MANIFEST.MF +++ b/org.eclipse.jgit/META-INF/MANIFEST.MF @@ -124,7 +124,8 @@ Export-Package: org.eclipse.jgit.annotations;version="6.8.0", x-friends:="org.eclipse.jgit.ssh.apache, org.eclipse.jgit.ssh.jsch, org.eclipse.jgit.test", - org.eclipse.jgit.internal.util;version="6.8.0";x-internal:=true, + org.eclipse.jgit.internal.util;version="6.8.0"; + x-friends:=" org.eclipse.jgit.junit", org.eclipse.jgit.lib;version="6.8.0"; uses:="org.eclipse.jgit.transport, org.eclipse.jgit.util.sha1, diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/CloneCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/CloneCommand.java index 107b00e27..3e034f1a6 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/CloneCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/CloneCommand.java @@ -29,6 +29,7 @@ import org.eclipse.jgit.errors.IncorrectObjectTypeException; import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.internal.JGitText; +import org.eclipse.jgit.internal.util.ShutdownHook; import org.eclipse.jgit.lib.AnyObjectId; import org.eclipse.jgit.lib.BranchConfig.BranchRebaseMode; import org.eclipse.jgit.lib.ConfigConstants; @@ -100,6 +101,8 @@ public class CloneCommand extends TransportCommand { private List shallowExcludes = new ArrayList<>(); + private ShutdownHook.Listener shutdownListener = this::cleanup; + private enum FETCH_TYPE { MULTIPLE_BRANCHES, ALL_BRANCHES, MIRROR } @@ -181,12 +184,7 @@ public Git call() throws GitAPIException, InvalidRemoteException, @SuppressWarnings("resource") // Closed by caller Repository repository = init(); FetchResult fetchResult = null; - Thread cleanupHook = new Thread(() -> cleanup()); - try { - Runtime.getRuntime().addShutdownHook(cleanupHook); - } catch (IllegalStateException e) { - // ignore - the VM is already shutting down - } + ShutdownHook.INSTANCE.register(shutdownListener); try { fetchResult = fetch(repository, u); } catch (IOException ioe) { @@ -210,11 +208,7 @@ public Git call() throws GitAPIException, InvalidRemoteException, cleanup(); throw e; } finally { - try { - Runtime.getRuntime().removeShutdownHook(cleanupHook); - } catch (IllegalStateException e) { - // ignore - the VM is already shutting down - } + ShutdownHook.INSTANCE.unregister(shutdownListener); } try { checkout(repository, fetchResult); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java index fd9e55052..6933bfb3d 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java @@ -75,6 +75,7 @@ import org.eclipse.jgit.internal.storage.commitgraph.GraphCommits; import org.eclipse.jgit.internal.storage.pack.PackExt; import org.eclipse.jgit.internal.storage.pack.PackWriter; +import org.eclipse.jgit.internal.util.ShutdownHook; import org.eclipse.jgit.lib.ConfigConstants; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.CoreConfig; @@ -1799,7 +1800,7 @@ private class PidLock implements AutoCloseable { private FileChannel channel; - private Thread cleanupHook; + private ShutdownHook.Listener shutdownListener = this::close; PidLock() { pidFile = repo.getDirectory().toPath().resolve(GC_PID); @@ -1829,12 +1830,7 @@ boolean lock() throws IOException { } channel.write(ByteBuffer .wrap(getProcDesc().getBytes(StandardCharsets.UTF_8))); - try { - Runtime.getRuntime().addShutdownHook( - cleanupHook = new Thread(() -> close())); - } catch (IllegalStateException e) { - // ignore - the VM is already shutting down - } + ShutdownHook.INSTANCE.register(shutdownListener); } catch (IOException | OverlappingFileLockException e) { try { failedToLock(); @@ -1903,13 +1899,7 @@ private String getHostName() { public void close() { boolean wasLocked = false; try { - if (cleanupHook != null) { - try { - Runtime.getRuntime().removeShutdownHook(cleanupHook); - } catch (IllegalStateException e) { - // ignore - the VM is already shutting down - } - } + ShutdownHook.INSTANCE.unregister(shutdownListener); if (lock != null && lock.isValid()) { lock.release(); wasLocked = true; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java index b8fc4fc1e..3e95de145 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java @@ -67,6 +67,7 @@ import org.eclipse.jgit.errors.LockFailedException; import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.internal.storage.file.FileSnapshot; +import org.eclipse.jgit.internal.util.ShutdownHook; import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.ConfigConstants; import org.eclipse.jgit.lib.Constants; @@ -301,18 +302,16 @@ public static final class FileStoreAttributes { static { // Shut down the SAVE_RUNNER on System.exit() + ShutdownHook.INSTANCE + .register(FileStoreAttributes::shutdownSafeRunner); + } + + private static void shutdownSafeRunner() { try { - Runtime.getRuntime().addShutdownHook(new Thread(() -> { - try { - SAVE_RUNNER.shutdownNow(); - SAVE_RUNNER.awaitTermination(100, - TimeUnit.MILLISECONDS); - } catch (Exception e) { - // Ignore; we're shutting down - } - })); - } catch (IllegalStateException e) { - // ignore - may fail if shutdown is already in progress + SAVE_RUNNER.shutdownNow(); + SAVE_RUNNER.awaitTermination(100, TimeUnit.MILLISECONDS); + } catch (Exception e) { + // Ignore; we're shutting down } }