Merge branch 'stable-4.8' into stable-4.9

* stable-4.8:
  Fix GC run in foreground to not use executor

Change-Id: Id9d864a8e727fefa35ca87eccb4e3801eb689c3c
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

2 files changed, 22 insertions, 55 deletions

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 574f3b3367..736067bffc 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
@@ -78,15 +78,12 @@ import java.util.Objects;
 import java.util.Set;
 import java.util.TreeMap;
 import java.util.concurrent.Callable;
-import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Future;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 import org.eclipse.jgit.annotations.NonNull;
-import org.eclipse.jgit.api.errors.JGitInternalException;
 import org.eclipse.jgit.dircache.DirCacheIterator;
 import org.eclipse.jgit.errors.CancelledException;
 import org.eclipse.jgit.errors.CorruptObjectException;
@@ -244,8 +241,11 @@ public class GC {
 	 */
 	// TODO(ms): in 5.0 change signature and return Future<Collection<PackFile>>
 	public Collection<PackFile> gc() throws IOException, ParseException {
-		final GcLog gcLog = background ? new GcLog(repo) : null;
-		if (gcLog != null && !gcLog.lock(background)) {
+		if (!background) {
+			return doGc();
+		}
+		final GcLog gcLog = new GcLog(repo);
+		if (!gcLog.lock()) {
 			// there is already a background gc running
 			return Collections.emptyList();
 		}
@@ -253,48 +253,31 @@ public class GC {
 		Callable<Collection<PackFile>> gcTask = () -> {
 			try {
 				Collection<PackFile> newPacks = doGc();
-				if (automatic && tooManyLooseObjects() && gcLog != null) {
+				if (automatic && tooManyLooseObjects()) {
 					String message = JGitText.get().gcTooManyUnpruned;
 					gcLog.write(message);
 					gcLog.commit();
 				}
 				return newPacks;
 			} catch (IOException | ParseException e) {
-				if (background) {
-					if (gcLog == null) {
-						// Lacking a log, there's no way to report this.
-						return Collections.emptyList();
-					}
-					try {
-						gcLog.write(e.getMessage());
-						StringWriter sw = new StringWriter();
-						e.printStackTrace(new PrintWriter(sw));
-						gcLog.write(sw.toString());
-						gcLog.commit();
-					} catch (IOException e2) {
-						e2.addSuppressed(e);
-						LOG.error(e2.getMessage(), e2);
-					}
-				} else {
-					throw new JGitInternalException(e.getMessage(), e);
+				try {
+					gcLog.write(e.getMessage());
+					StringWriter sw = new StringWriter();
+					e.printStackTrace(new PrintWriter(sw));
+					gcLog.write(sw.toString());
+					gcLog.commit();
+				} catch (IOException e2) {
+					e2.addSuppressed(e);
+					LOG.error(e2.getMessage(), e2);
 				}
 			} finally {
-				if (gcLog != null) {
-					gcLog.unlock();
-				}
+				gcLog.unlock();
 			}
 			return Collections.emptyList();
 		};
-		Future<Collection<PackFile>> result = executor().submit(gcTask);
-		if (background) {
-			// TODO(ms): in 5.0 change signature and return the Future
-			return Collections.emptyList();
-		}
-		try {
-			return result.get();
-		} catch (InterruptedException | ExecutionException e) {
-			throw new IOException(e);
-		}
+		// TODO(ms): in 5.0 change signature and return the Future
+		executor().submit(gcTask);
+		return Collections.emptyList();
 	}
 
 	private ExecutorService executor() {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GcLog.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GcLog.java
index 994a4325a8..e5fc0c5bad 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GcLog.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GcLog.java
@@ -45,18 +45,15 @@ package org.eclipse.jgit.internal.storage.file;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 
-import java.io.BufferedReader;
 import java.io.File;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.NoSuchFileException;
 import java.nio.file.attribute.FileTime;
-import java.text.MessageFormat;
 import java.text.ParseException;
 import java.time.Instant;
 
 import org.eclipse.jgit.api.errors.JGitInternalException;
-import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.lib.ConfigConstants;
 import org.eclipse.jgit.util.GitDateParser;
 import org.eclipse.jgit.util.SystemReader;
@@ -103,22 +100,11 @@ class GcLog {
 		return gcLogExpire;
 	}
 
-	private boolean autoGcBlockedByOldLockFile(boolean background) {
+	private boolean autoGcBlockedByOldLockFile() {
 		try {
 			FileTime lastModified = Files.getLastModifiedTime(logFile.toPath());
 			if (lastModified.toInstant().compareTo(getLogExpiry()) > 0) {
 				// There is an existing log file, which is too recent to ignore
-				if (!background) {
-					try (BufferedReader reader = Files
-							.newBufferedReader(logFile.toPath())) {
-						char[] buf = new char[1000];
-						int len = reader.read(buf, 0, 1000);
-						String oldError = new String(buf, 0, len);
-
-						throw new JGitInternalException(MessageFormat.format(
-								JGitText.get().gcLogExists, oldError, logFile));
-					}
-				}
 				return true;
 			}
 		} catch (NoSuchFileException e) {
@@ -132,11 +118,9 @@ class GcLog {
 	/**
 	 * Lock the GC log file for updates
 	 *
-	 * @param background
-	 *            If true, and if gc.log already exists, unlock and return false
 	 * @return {@code true} if we hold the lock
 	 */
-	boolean lock(boolean background) {
+	boolean lock() {
 		try {
 			if (!lock.lock()) {
 				return false;
@@ -144,7 +128,7 @@ class GcLog {
 		} catch (IOException e) {
 			throw new JGitInternalException(e.getMessage(), e);
 		}
-		if (autoGcBlockedByOldLockFile(background)) {
+		if (autoGcBlockedByOldLockFile()) {
 			lock.unlock();
 			return false;
 		}