diff options
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 848d20aba9..33e7452d96 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 46d96df9c3..8622e0a1a8 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; } |