Fix GC run in foreground to not use executor

Since I3870cadb4, GC task was always delegated to an executor even when
background option was set to false. This was an issue because if more
than one GC object was instantiated and executed in parallel, only one GC
was actually running because of the single thread executor.

Change-Id: I8c587d22d63c1601b7d75914692644a385cd86d6
Signed-off-by: Hugo Arès <hugo.ares@ericsson.com>

org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java

@@ -78,16 +78,13 @@ 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.Executors;
-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;
@@ -232,8 +229,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 (!background) {
-		if (gcLog != null && !gcLog.lock(background)) {
+			return doGc();
+		}
+		final GcLog gcLog = new GcLog(repo);
+		if (!gcLog.lock()) {
 			// there is already a background gc running
 			return Collections.emptyList();
 		}
@@ -241,48 +241,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) {
+				try {
-					if (gcLog == null) {
+					gcLog.write(e.getMessage());
-						// Lacking a log, there's no way to report this.
+					StringWriter sw = new StringWriter();
-						return Collections.emptyList();
+					e.printStackTrace(new PrintWriter(sw));
-					}
+					gcLog.write(sw.toString());
-					try {
+					gcLog.commit();
-						gcLog.write(e.getMessage());
+				} catch (IOException e2) {
-						StringWriter sw = new StringWriter();
+					e2.addSuppressed(e);
-						e.printStackTrace(new PrintWriter(sw));
+					LOG.error(e2.getMessage(), e2);
-						gcLog.write(sw.toString());
-						gcLog.commit();
-					} catch (IOException e2) {
-						e2.addSuppressed(e);
-						LOG.error(e2.getMessage(), e2);
-					}
-				} else {
-					throw new JGitInternalException(e.getMessage(), e);
 				}
 			} finally {
-				if (gcLog != null) {
+				gcLog.unlock();
-					gcLog.unlock();
-				}
 			}
 			return Collections.emptyList();
 		};
-		Future<Collection<PackFile>> result = executor.submit(gcTask);
+		// TODO(ms): in 5.0 change signature and return the Future
-		if (background) {
+		executor.submit(gcTask);
-			// TODO(ms): in 5.0 change signature and return the Future
+		return Collections.emptyList();
-			return Collections.emptyList();
-		}
-		try {
-			return result.get();
-		} catch (InterruptedException | ExecutionException e) {
-			throw new IOException(e);
-		}
 	}
 
 	private Collection<PackFile> doGc() throws IOException, ParseException {

org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GcLog.java

@@ -44,20 +44,17 @@
 package org.eclipse.jgit.internal.storage.file;
 
 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;
 
 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;
 
@@ -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;
 		}