Merge branch 'stable-5.10' into stable-5.11
* stable-5.10:
Retry loose object read upon "Stale file handle" exception
Ignore missing javadoc in test bundles
Change-Id: Ia385fa6b5d2fee64476793e06860a279bf2f6e36
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 3dd5840..4c4634a 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.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 d269457..316e336 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 @@
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.util.FS;
import org.junit.Assume;
import org.junit.Test;
+import org.mockito.Mockito;
public class ObjectDirectoryTest extends RepositoryTestCase {
@@ -195,6 +200,42 @@
}
@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 16fe12fa..b6b10ea 100644
--- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
+++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
@@ -426,6 +426,7 @@
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 ba88aea..b942c09 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
@@ -454,6 +454,7 @@
/***/ 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 e7cb285..33621a1 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.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 @@
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 @@
}
void close() {
- unpackedObjectCache.clear();
+ unpackedObjectCache().clear();
}
/** {@inheritDoc} */
@@ -79,7 +87,7 @@
}
boolean hasCached(AnyObjectId id) {
- return unpackedObjectCache.isUnpacked(id);
+ return unpackedObjectCache().isUnpacked(id);
}
/**
@@ -133,29 +141,77 @@
}
ObjectLoader open(WindowCursor curs, AnyObjectId id) throws IOException {
- File path = fileFor(id);
- try (FileInputStream in = new FileInputStream(path)) {
- unpackedObjectCache.add(id);
- return UnpackedObject.open(in, path, id, curs);
- } catch (FileNotFoundException noFile) {
- if (path.exists()) {
- throw noFile;
+ 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;
}
+ 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);
+ return UnpackedObject.open(in, path, id, curs);
+ }
+ }
+
+ /**
+ * <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 @@
Files.move(FileUtils.toPath(tmp), FileUtils.toPath(dst),
StandardCopyOption.ATOMIC_MOVE);
dst.setReadOnly();
- unpackedObjectCache.add(id);
+ unpackedObjectCache().add(id);
return InsertLooseObjectResult.INSERTED;
}
