]> source.dussan.org Git - jgit.git/commitdiff
Add ShutdownHook to cleanup FileLocks on graceful JVM shutdown 13/204213/12
authorMatthias Sohn <matthias.sohn@sap.com>
Fri, 8 Sep 2023 00:14:20 +0000 (02:14 +0200)
committerMatthias Sohn <matthias.sohn@sap.com>
Tue, 12 Sep 2023 20:43:15 +0000 (22:43 +0200)
This should avoid stale lock files if the JVM is terminated gracefully.

Implement a ShutdownHook which can register/unregister listeners which
need to do some cleanup during graceful JVM shutdown. This hook is
registered as a Java shutdown hook and  when the JVM shuts down
calls #onShutdown of registered listeners using a parallel stream
to let them run concurrently.

See https://docs.oracle.com/javase/8/docs/technotes/guides/lang/hook-design.html

Bug: 582379
Change-Id: I1621dc5f7d9a8c832b6d1b74cbc47578b1c2f0b8

org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/LockFile.java
org.eclipse.jgit/src/org/eclipse/jgit/internal/util/ShutdownHook.java [new file with mode: 0644]

index 80a65b22637032c5c4643f893c0967ef9911532e..141ea8e201b631afa2c396a0d00290992326f7f4 100644 (file)
@@ -707,6 +707,9 @@ shortCompressedStreamAt=Short compressed stream at {0}
 shortReadOfBlock=Short read of block.
 shortReadOfOptionalDIRCExtensionExpectedAnotherBytes=Short read of optional DIRC extension {0}; expected another {1} bytes within the section.
 shortSkipOfBlock=Short skip of block.
+shutdownCleanup=Cleanup {} during JVM shutdown
+shutdownCleanupFailed=Cleanup during JVM shutdown failed
+shutdownCleanupListenerFailed=Cleanup of {0} during JVM shutdown failed
 signatureVerificationError=Signature verification failed
 signatureVerificationUnavailable=No signature verifier registered
 signedTagMessageNoLf=A non-empty message of a signed tag must end in LF.
index 1da1de44183c402cd0817772cf3eab5d4de3a413..f0d6657336f5489b7544be8af66492269c7d6692 100644 (file)
@@ -735,6 +735,9 @@ public class JGitText extends TranslationBundle {
        /***/ public String shortReadOfBlock;
        /***/ public String shortReadOfOptionalDIRCExtensionExpectedAnotherBytes;
        /***/ public String shortSkipOfBlock;
+       /***/ public String shutdownCleanup;
+       /***/ public String shutdownCleanupFailed;
+       /***/ public String shutdownCleanupListenerFailed;
        /***/ public String signatureVerificationError;
        /***/ public String signatureVerificationUnavailable;
        /***/ public String signedTagMessageNoLf;
index 8ada0a90e1480d7570380ac34738d66a53238be5..a2d8bd01405aa3d31dc79aaa6deab7ffb04c2afd 100644 (file)
@@ -31,6 +31,7 @@ import java.time.Instant;
 import java.util.concurrent.TimeUnit;
 
 import org.eclipse.jgit.internal.JGitText;
+import org.eclipse.jgit.internal.util.ShutdownHook;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.util.FS;
@@ -113,6 +114,8 @@ public class LockFile {
 
        private LockToken token;
 
+       private ShutdownHook.Listener shutdownListener = this::unlock;
+
        /**
         * Create a new lock for any file.
         *
@@ -147,6 +150,7 @@ public class LockFile {
                }
                boolean obtainedLock = token.isCreated();
                if (obtainedLock) {
+                       ShutdownHook.INSTANCE.register(shutdownListener);
                        haveLck = true;
                        isAppend = false;
                        written = false;
@@ -471,6 +475,7 @@ public class LockFile {
         *             the lock is not held.
         */
        public boolean commit() {
+               ShutdownHook.INSTANCE.unregister(shutdownListener);
                if (os != null) {
                        unlock();
                        throw new IllegalStateException(MessageFormat.format(JGitText.get().lockOnNotClosed, ref));
@@ -551,6 +556,7 @@ public class LockFile {
         * The temporary file (if created) is deleted before returning.
         */
        public void unlock() {
+               ShutdownHook.INSTANCE.unregister(shutdownListener);
                if (os != null) {
                        try {
                                os.close();
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/util/ShutdownHook.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/util/ShutdownHook.java
new file mode 100644 (file)
index 0000000..9a299bc
--- /dev/null
@@ -0,0 +1,149 @@
+/*
+ * Copyright (C) 2023, SAP SE and others
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Distribution License v. 1.0 which is available at
+ * https://www.eclipse.org/org/documents/edl-v10.php.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+package org.eclipse.jgit.internal.util;
+
+import java.text.MessageFormat;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.RejectedExecutionException;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+
+import org.eclipse.jgit.internal.JGitText;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * A hook registered as a JVM shutdown hook managing a set of objects needing
+ * cleanup during JVM shutdown. See {@link Runtime#addShutdownHook}.
+ */
+public enum ShutdownHook {
+       /**
+        * Singleton
+        */
+       INSTANCE;
+
+       /**
+        * Object that needs to cleanup on JVM shutdown.
+        */
+       public interface Listener {
+               /**
+                * Cleanup resources when JVM shuts down, called from JVM shutdown hook.
+                * <p>
+                * Implementations should be coded defensively
+                * <ul>
+                * <li>they should finish their work quickly
+                * <li>they should be written to be thread-safe and to avoid deadlocks
+                * insofar as possible
+                * <li>they should not rely blindly upon services that may have
+                * registered their own shutdown hooks and therefore may themselves be
+                * in the process of shutting down
+                * <li>attempts to use other thread-based services may lead to
+                * deadlocks.
+                * </ul>
+                * See {@link Runtime#addShutdownHook} for more details.
+                */
+               public void onShutdown();
+       }
+
+       private static final Logger LOG = LoggerFactory
+                       .getLogger(ShutdownHook.class);
+
+       private final Set<Listener> listeners = ConcurrentHashMap.newKeySet();
+
+       private volatile boolean shutdownInProgress;
+
+       private ShutdownHook() {
+               try {
+                       Runtime.getRuntime().addShutdownHook(new Thread(this::cleanup));
+               } catch (IllegalStateException e) {
+                       // ignore - the VM is already shutting down
+               }
+       }
+
+       private void cleanup() {
+               shutdownInProgress = true;
+               ExecutorService runner = Executors.newWorkStealingPool();
+               try {
+                       runner.submit(() -> {
+                               this.doCleanup();
+                               return null;
+                       }).get(30L, TimeUnit.SECONDS);
+               } catch (RejectedExecutionException | InterruptedException
+                               | ExecutionException | TimeoutException e) {
+                       LOG.error(JGitText.get().shutdownCleanupFailed, e);
+               }
+               runner.shutdownNow();
+       }
+
+       private void doCleanup() {
+               listeners.parallelStream().forEach(this::notify);
+       }
+
+       private void notify(Listener l) {
+               LOG.warn(JGitText.get().shutdownCleanup, l);
+               try {
+                       l.onShutdown();
+               } catch (RuntimeException e) {
+                       LOG.error(MessageFormat.format(
+                                       JGitText.get().shutdownCleanupListenerFailed, l), e);
+               }
+       }
+
+       /**
+        * Register object that needs cleanup during JVM shutdown if it is not
+        * already registered. Registration is disabled when JVM shutdown is already
+        * in progress.
+        *
+        * @param l
+        *            the object to call {@link Listener#onShutdown} on when JVM
+        *            shuts down
+        * @return {@code true} if this object has been registered
+        */
+       public boolean register(Listener l) {
+               if (shutdownInProgress) {
+                       return listeners.contains(l);
+               }
+               LOG.debug("register {} with shutdown hook", l); //$NON-NLS-1$
+               listeners.add(l);
+               return true;
+       }
+
+       /**
+        * Unregister object that no longer needs cleanup during JVM shutdown if it
+        * is still registered. Unregistration is disabled when JVM shutdown is
+        * already in progress.
+        *
+        * @param l
+        *            the object registered to be notified for cleanup when the JVM
+        *            shuts down
+        * @return {@code true} if this object is no longer registered
+        */
+       public boolean unregister(Listener l) {
+               if (shutdownInProgress) {
+                       return !listeners.contains(l);
+               }
+               LOG.debug("unregister {} from shutdown hook", l); //$NON-NLS-1$
+               listeners.remove(l);
+               return true;
+       }
+
+       /**
+        * Whether a JVM shutdown is in progress
+        *
+        * @return {@code true} if a JVM shutdown is in progress
+        */
+       public boolean isShutdownInProgress() {
+               return shutdownInProgress;
+       }
+}