Merge "Merge branch 'stable-5.11' into stable-5.12" into stable-5.12

5 files changed, 113 insertions, 14 deletions

diff --git a/org.eclipse.jgit.lfs.server.test/.settings/org.eclipse.jdt.core.prefs b/org.eclipse.jgit.lfs.server.test/.settings/org.eclipse.jdt.core.prefs
index 3dd5840397..4c4634ada3 100644
--- a/org.eclipse.jgit.lfs.server.test/.settings/org.eclipse.jdt.core.prefs
+++ b/org.eclipse.jgit.lfs.server.test/.settings/org.eclipse.jdt.core.prefs
@@ -52,7 +52,7 @@ org.eclipse.jdt.core.compiler.problem.missingJavadocComments=ignore
 org.eclipse.jdt.core.compiler.problem.missingJavadocCommentsOverriding=disabled
 org.eclipse.jdt.core.compiler.problem.missingJavadocCommentsVisibility=protected
 org.eclipse.jdt.core.compiler.problem.missingJavadocTagDescription=return_tag
-org.eclipse.jdt.core.compiler.problem.missingJavadocTags=error
+org.eclipse.jdt.core.compiler.problem.missingJavadocTags=ignore
 org.eclipse.jdt.core.compiler.problem.missingJavadocTagsMethodTypeParameters=disabled
 org.eclipse.jdt.core.compiler.problem.missingJavadocTagsOverriding=disabled
 org.eclipse.jdt.core.compiler.problem.missingJavadocTagsVisibility=private
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/ObjectDirectoryTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/ObjectDirectoryTest.java
index d269457eb1..316e33639d 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/ObjectDirectoryTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/ObjectDirectoryTest.java
@@ -44,8 +44,12 @@ package org.eclipse.jgit.internal.storage.file;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
 
 import java.io.File;
 import java.io.IOException;
@@ -69,6 +73,7 @@ import org.eclipse.jgit.storage.file.FileBasedConfig;
 import org.eclipse.jgit.util.FS;
 import org.junit.Assume;
 import org.junit.Test;
+import org.mockito.Mockito;
 
 public class ObjectDirectoryTest extends RepositoryTestCase {
 
@@ -195,6 +200,42 @@ public class ObjectDirectoryTest extends RepositoryTestCase {
 	}
 
 	@Test
+	public void testOpenLooseObjectSuppressStaleFileHandleException()
+			throws Exception {
+		ObjectId id = ObjectId
+				.fromString("873fb8d667d05436d728c52b1d7a09528e6eb59b");
+		WindowCursor curs = new WindowCursor(db.getObjectDatabase());
+
+		LooseObjects mock = mock(LooseObjects.class);
+		UnpackedObjectCache unpackedObjectCacheMock = mock(
+				UnpackedObjectCache.class);
+
+		Mockito.when(mock.getObjectLoader(any(), any(), any()))
+				.thenThrow(new IOException("Stale File Handle"));
+		Mockito.when(mock.open(curs, id)).thenCallRealMethod();
+		Mockito.when(mock.unpackedObjectCache())
+				.thenReturn(unpackedObjectCacheMock);
+
+		assertNull(mock.open(curs, id));
+		verify(unpackedObjectCacheMock).remove(id);
+	}
+
+	@Test
+	public void testOpenLooseObjectPropagatesIOExceptions() throws Exception {
+		ObjectId id = ObjectId
+				.fromString("873fb8d667d05436d728c52b1d7a09528e6eb59b");
+		WindowCursor curs = new WindowCursor(db.getObjectDatabase());
+
+		LooseObjects mock = mock(LooseObjects.class);
+
+		Mockito.when(mock.getObjectLoader(any(), any(), any()))
+				.thenThrow(new IOException("some IO failure"));
+		Mockito.when(mock.open(curs, id)).thenCallRealMethod();
+
+		assertThrows(IOException.class, () -> mock.open(curs, id));
+	}
+
+	@Test
 	public void testShallowFileCorrupt() throws Exception {
 		FileRepository repository = createBareRepository();
 		ObjectDirectory dir = repository.getObjectDatabase();
diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
index 962324e0f7..6180737ffe 100644
--- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
+++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
@@ -442,6 +442,7 @@ logInconsistentFiletimeDiff={}: inconsistent duration from file timestamps on {}
 logLargerFiletimeDiff={}: inconsistent duration from file timestamps on {}, {}: diff = {} > {} (last good value). Aborting measurement.
 logSmallerFiletime={}: got smaller file timestamp on {}, {}: {} < {}. Aborting measurement at resolution {}.
 logXDGConfigHomeInvalid=Environment variable XDG_CONFIG_HOME contains an invalid path {}
+looseObjectHandleIsStale=loose-object {0} file handle is stale. retry {1} of {2}
 maxCountMustBeNonNegative=max count must be >= 0
 mergeConflictOnNonNoteEntries=Merge conflict on non-note entries: base = {0}, ours = {1}, theirs = {2}
 mergeConflictOnNotes=Merge conflict on note {0}. base = {1}, ours = {2}, theirs = {2}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
index fd54986f94..e1fa14435d 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
@@ -470,6 +470,7 @@ public class JGitText extends TranslationBundle {
 	/***/ public String logLargerFiletimeDiff;
 	/***/ public String logSmallerFiletime;
 	/***/ public String logXDGConfigHomeInvalid;
+	/***/ public String looseObjectHandleIsStale;
 	/***/ public String maxCountMustBeNonNegative;
 	/***/ public String mergeConflictOnNonNoteEntries;
 	/***/ public String mergeConflictOnNotes;
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/LooseObjects.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/LooseObjects.java
index e7cb285c34..33621a1e9f 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/LooseObjects.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/LooseObjects.java
@@ -17,8 +17,10 @@ import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.NoSuchFileException;
 import java.nio.file.StandardCopyOption;
+import java.text.MessageFormat;
 import java.util.Set;
 
+import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.internal.storage.file.FileObjectDatabase.InsertLooseObjectResult;
 import org.eclipse.jgit.lib.AbbreviatedObjectId;
 import org.eclipse.jgit.lib.AnyObjectId;
@@ -40,6 +42,12 @@ class LooseObjects {
 	private static final Logger LOG = LoggerFactory
 			.getLogger(LooseObjects.class);
 
+	/**
+	 * Maximum number of attempts to read a loose object for which a stale file
+	 * handle exception is thrown
+	 */
+	private final static int MAX_LOOSE_OBJECT_STALE_READ_ATTEMPTS = 5;
+
 	private final File directory;
 
 	private final UnpackedObjectCache unpackedObjectCache;
@@ -69,7 +77,7 @@ class LooseObjects {
 	}
 
 	void close() {
-		unpackedObjectCache.clear();
+		unpackedObjectCache().clear();
 	}
 
 	/** {@inheritDoc} */
@@ -79,7 +87,7 @@ class LooseObjects {
 	}
 
 	boolean hasCached(AnyObjectId id) {
-		return unpackedObjectCache.isUnpacked(id);
+		return unpackedObjectCache().isUnpacked(id);
 	}
 
 	/**
@@ -133,29 +141,77 @@ class LooseObjects {
 	}
 
 	ObjectLoader open(WindowCursor curs, AnyObjectId id) throws IOException {
-		File path = fileFor(id);
+		int readAttempts = 0;
+		while (readAttempts < MAX_LOOSE_OBJECT_STALE_READ_ATTEMPTS) {
+			readAttempts++;
+			File path = fileFor(id);
+			try {
+				return getObjectLoader(curs, path, id);
+			} catch (FileNotFoundException noFile) {
+				if (path.exists()) {
+					throw noFile;
+				}
+				break;
+			} catch (IOException e) {
+				if (!FileUtils.isStaleFileHandleInCausalChain(e)) {
+					throw e;
+				}
+				if (LOG.isDebugEnabled()) {
+					LOG.debug(MessageFormat.format(
+							JGitText.get().looseObjectHandleIsStale, id.name(),
+							Integer.valueOf(readAttempts), Integer.valueOf(
+									MAX_LOOSE_OBJECT_STALE_READ_ATTEMPTS)));
+				}
+			}
+		}
+		unpackedObjectCache().remove(id);
+		return null;
+	}
+
+	/**
+	 * Provides a loader for an objectId
+	 *
+	 * @param curs
+	 *            cursor on the database
+	 * @param path
+	 *            the path of the loose object
+	 * @param id
+	 *            the object id
+	 * @return a loader for the loose file object
+	 * @throws IOException
+	 *             when file does not exist or it could not be opened
+	 */
+	ObjectLoader getObjectLoader(WindowCursor curs, File path, AnyObjectId id)
+			throws IOException {
 		try (FileInputStream in = new FileInputStream(path)) {
-			unpackedObjectCache.add(id);
+			unpackedObjectCache().add(id);
 			return UnpackedObject.open(in, path, id, curs);
-		} catch (FileNotFoundException noFile) {
-			if (path.exists()) {
-				throw noFile;
-			}
-			unpackedObjectCache.remove(id);
-			return null;
 		}
 	}
 
+	/**
+	 * <p>
+	 * Getter for the field <code>unpackedObjectCache</code>.
+	 * </p>
+	 * This accessor is particularly useful to allow mocking of this class for
+	 * testing purposes.
+	 *
+	 * @return the cache of the objects currently unpacked.
+	 */
+	UnpackedObjectCache unpackedObjectCache() {
+		return unpackedObjectCache;
+	}
+
 	long getSize(WindowCursor curs, AnyObjectId id) throws IOException {
 		File f = fileFor(id);
 		try (FileInputStream in = new FileInputStream(f)) {
-			unpackedObjectCache.add(id);
+			unpackedObjectCache().add(id);
 			return UnpackedObject.getSize(in, id, curs);
 		} catch (FileNotFoundException noFile) {
 			if (f.exists()) {
 				throw noFile;
 			}
-			unpackedObjectCache.remove(id);
+			unpackedObjectCache().remove(id);
 			return -1;
 		}
 	}
@@ -207,7 +263,7 @@ class LooseObjects {
 		Files.move(FileUtils.toPath(tmp), FileUtils.toPath(dst),
 				StandardCopyOption.ATOMIC_MOVE);
 		dst.setReadOnly();
-		unpackedObjectCache.add(id);
+		unpackedObjectCache().add(id);
 		return InsertLooseObjectResult.INSERTED;
 	}