From 642f160236817ff1392ac96085cd5a3f3a96bc1f Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Fri, 8 Sep 2023 22:57:05 +0200 Subject: Use ShutdownHook to gracefully handle JVM shutdown in all classes which already registered their own shutdown hook - CloneCommand - GC#PidLock - FS#FileStoreAttributes - LocalDiskRepositoryTestCase#Cleanup Change-Id: I3efc1f83f3cbbf43eeeaaedcd2bee1ef31971a72 --- .../src/org/eclipse/jgit/api/CloneCommand.java | 16 +++++----------- .../org/eclipse/jgit/internal/storage/file/GC.java | 18 ++++-------------- org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java | 21 ++++++++++----------- 3 files changed, 19 insertions(+), 36 deletions(-) (limited to 'org.eclipse.jgit/src') 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 107b00e274..3e034f1a6a 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.dircache.DirCacheCheckout; 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 class CloneCommand extends TransportCommand { @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 class CloneCommand extends TransportCommand { 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 fd9e550526..6933bfb3d4 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.CommitGraphWriter; 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 @@ public class GC { private FileChannel channel; - private Thread cleanupHook; + private ShutdownHook.Listener shutdownListener = this::close; PidLock() { pidFile = repo.getDirectory().toPath().resolve(GC_PID); @@ -1829,12 +1830,7 @@ public class GC { } 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 @@ public class GC { 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 b8fc4fc1e2..3e95de1459 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.ConfigInvalidException; 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 abstract class FS { 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 } } -- cgit v1.2.3