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 @@ public void testShallowFile() } @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 static JGitText get() { /***/ 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 @@ 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 @@ void create() throws IOException { } void close() { - unpackedObjectCache.clear(); + unpackedObjectCache().clear(); } /** {@inheritDoc} */ @@ -79,7 +87,7 @@ public String toString() { } boolean hasCached(AnyObjectId id) { - return unpackedObjectCache.isUnpacked(id); + return unpackedObjectCache().isUnpacked(id); } /** @@ -133,29 +141,77 @@ boolean resolve(Set<ObjectId> matches, AbbreviatedObjectId id, } 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 @@ private InsertLooseObjectResult tryMove(File tmp, File dst, ObjectId id) Files.move(FileUtils.toPath(tmp), FileUtils.toPath(dst), StandardCopyOption.ATOMIC_MOVE); dst.setReadOnly(); - unpackedObjectCache.add(id); + unpackedObjectCache().add(id); return InsertLooseObjectResult.INSERTED; }