Merge "CleanupService: preload JgitText if not running in OSGi"

2 files changed, 49 insertions, 18 deletions

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/util/CleanupService.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/util/CleanupService.java
index 76e09307ab..29ed7564d3 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/util/CleanupService.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/util/CleanupService.java
@@ -9,6 +9,10 @@
  */
 package org.eclipse.jgit.internal.util;
 
+import org.eclipse.jgit.internal.JGitText;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 /**
  * A class that is registered as an OSGi service via the manifest. If JGit runs
  * in OSGi, OSGi will instantiate a singleton as soon as the bundle is activated
@@ -23,12 +27,17 @@ package org.eclipse.jgit.internal.util;
  */
 public final class CleanupService {
 
+	private static final Logger LOG = LoggerFactory
+			.getLogger(CleanupService.class);
+
 	private static final Object LOCK = new Object();
 
 	private static CleanupService INSTANCE;
 
 	private final boolean isOsgi;
 
+	private JGitText jgitText;
+
 	private Runnable cleanup;
 
 	/**
@@ -74,8 +83,24 @@ public final class CleanupService {
 		if (isOsgi) {
 			cleanup = cleanUp;
 		} else {
+			// Ensure the JGitText class is loaded. Depending on the framework
+			// JGit runs in, it may not be possible anymore to load classes when
+			// the hook runs. For instance when run in a maven plug-in: the
+			// Plexus class world that loaded JGit may already have been
+			// disposed by the time the JVM shutdown hook runs when the whole
+			// maven build terminates.
+			jgitText = JGitText.get();
+			assert jgitText != null;
 			try {
-				Runtime.getRuntime().addShutdownHook(new Thread(cleanUp));
+				Runtime.getRuntime().addShutdownHook(new Thread(() -> {
+					try {
+						cleanUp.run();
+						// Don't catch exceptions; let the JVM do the problem
+						// reporting.
+					} finally {
+						jgitText = null;
+					}
+				}));
 			} catch (IllegalStateException e) {
 				// Ignore -- the JVM is already shutting down.
 			}
@@ -86,7 +111,11 @@ public final class CleanupService {
 		if (isOsgi && cleanup != null) {
 			Runnable r = cleanup;
 			cleanup = null;
-			r.run();
+			try {
+				r.run();
+			} catch (RuntimeException e) {
+				LOG.error(JGitText.get().shutdownCleanupFailed, e);
+			}
 		}
 	}
 }
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
index 5ba33dbbff..f6b4723489 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/util/ShutdownHook.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/util/ShutdownHook.java
@@ -15,9 +15,9 @@ 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 java.util.concurrent.atomic.AtomicBoolean;
 
 import org.eclipse.jgit.internal.JGitText;
 import org.slf4j.Logger;
@@ -66,25 +66,27 @@ public enum ShutdownHook {
 
 	private final Set<Listener> listeners = ConcurrentHashMap.newKeySet();
 
-	private volatile boolean shutdownInProgress;
+	private final AtomicBoolean shutdownInProgress = new AtomicBoolean();
 
 	private ShutdownHook() {
 		CleanupService.getInstance().register(this::cleanup);
 	}
 
 	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);
+		if (!shutdownInProgress.getAndSet(true)) {
+			ExecutorService runner = Executors.newWorkStealingPool();
+			try {
+				runner.submit(() -> {
+					this.doCleanup();
+					return null;
+				}).get(30L, TimeUnit.SECONDS);
+			} catch (InterruptedException | ExecutionException
+					| TimeoutException e) {
+				throw new RuntimeException(e.getMessage(), e);
+			} finally {
+				runner.shutdownNow();
+			}
 		}
-		runner.shutdownNow();
 	}
 
 	private void doCleanup() {
@@ -112,7 +114,7 @@ public enum ShutdownHook {
 	 * @return {@code true} if this object has been registered
 	 */
 	public boolean register(Listener l) {
-		if (shutdownInProgress) {
+		if (shutdownInProgress.get()) {
 			return listeners.contains(l);
 		}
 		LOG.debug("register {} with shutdown hook", l); //$NON-NLS-1$
@@ -131,7 +133,7 @@ public enum ShutdownHook {
 	 * @return {@code true} if this object is no longer registered
 	 */
 	public boolean unregister(Listener l) {
-		if (shutdownInProgress) {
+		if (shutdownInProgress.get()) {
 			return !listeners.contains(l);
 		}
 		LOG.debug("unregister {} from shutdown hook", l); //$NON-NLS-1$
@@ -145,6 +147,6 @@ public enum ShutdownHook {
 	 * @return {@code true} if a JGit shutdown is in progress
 	 */
 	public boolean isShutdownInProgress() {
-		return shutdownInProgress;
+		return shutdownInProgress.get();
 	}
 }