diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileSnapshot.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileSnapshot.java
index 2165aa2b7..b5485c424 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileSnapshot.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileSnapshot.java
@@ -45,6 +45,7 @@ package org.eclipse.jgit.internal.storage.file;
import java.io.File;
import java.io.IOException;
+import java.nio.file.attribute.BasicFileAttributes;
import java.text.DateFormat;
import java.text.SimpleDateFormat;
import java.util.Date;
@@ -69,6 +70,13 @@ import org.eclipse.jgit.util.FS;
* file is less than 3 seconds ago.
*/
public class FileSnapshot {
+ /**
+ * An unknown file size.
+ *
+ * This value is used when a comparison needs to happen purely on the lastUpdate.
+ */
+ public static final long UNKNOWN_SIZE = -1;
+
/**
* A FileSnapshot that is considered to always be modified.
*
@@ -76,7 +84,7 @@ public class FileSnapshot {
* file, but only after {@link #isModified(File)} gets invoked. The returned
* snapshot contains only invalid status information.
*/
- public static final FileSnapshot DIRTY = new FileSnapshot(-1, -1);
+ public static final FileSnapshot DIRTY = new FileSnapshot(-1, -1, UNKNOWN_SIZE);
/**
* A FileSnapshot that is clean if the file does not exist.
@@ -85,7 +93,7 @@ public class FileSnapshot {
* file to be clean. {@link #isModified(File)} will return false if the file
* path does not exist.
*/
- public static final FileSnapshot MISSING_FILE = new FileSnapshot(0, 0) {
+ public static final FileSnapshot MISSING_FILE = new FileSnapshot(0, 0, 0) {
@Override
public boolean isModified(File path) {
return FS.DETECTED.exists(path);
@@ -105,12 +113,16 @@ public class FileSnapshot {
public static FileSnapshot save(File path) {
long read = System.currentTimeMillis();
long modified;
+ long size;
try {
- modified = FS.DETECTED.lastModified(path);
+ BasicFileAttributes fileAttributes = FS.DETECTED.fileAttributes(path);
+ modified = fileAttributes.lastModifiedTime().toMillis();
+ size = fileAttributes.size();
} catch (IOException e) {
modified = path.lastModified();
+ size = path.length();
}
- return new FileSnapshot(read, modified);
+ return new FileSnapshot(read, modified, size);
}
/**
@@ -125,7 +137,7 @@ public class FileSnapshot {
*/
public static FileSnapshot save(long modified) {
final long read = System.currentTimeMillis();
- return new FileSnapshot(read, modified);
+ return new FileSnapshot(read, modified, -1);
}
/** Last observed modification time of the path. */
@@ -137,10 +149,16 @@ public class FileSnapshot {
/** True once {@link #lastRead} is far later than {@link #lastModified}. */
private boolean cannotBeRacilyClean;
- private FileSnapshot(long read, long modified) {
+ /** Underlying file-system size in bytes.
+ *
+ * When set to {@link #UNKNOWN_SIZE} the size is not considered for modification checks. */
+ private final long size;
+
+ private FileSnapshot(long read, long modified, long size) {
this.lastRead = read;
this.lastModified = modified;
this.cannotBeRacilyClean = notRacyClean(read);
+ this.size = size;
}
/**
@@ -152,6 +170,13 @@ public class FileSnapshot {
return lastModified;
}
+ /**
+ * @return file size in bytes of last snapshot update
+ */
+ public long size() {
+ return size;
+ }
+
/**
* Check if the path may have been modified since the snapshot was saved.
*
@@ -161,12 +186,16 @@ public class FileSnapshot {
*/
public boolean isModified(File path) {
long currLastModified;
+ long currSize;
try {
- currLastModified = FS.DETECTED.lastModified(path);
+ BasicFileAttributes fileAttributes = FS.DETECTED.fileAttributes(path);
+ currLastModified = fileAttributes.lastModifiedTime().toMillis();
+ currSize = fileAttributes.size();
} catch (IOException e) {
currLastModified = path.lastModified();
+ currSize = path.length();
}
- return isModified(currLastModified);
+ return (currSize != UNKNOWN_SIZE && currSize != size) || isModified(currLastModified);
}
/**
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java
index e210a6821..f99047571 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java
@@ -918,13 +918,13 @@ public class ObjectDirectory extends FileObjectDatabase {
}
final String packName = base + PACK.getExtension();
+ final File packFile = new File(packDirectory, packName);
final PackFile oldPack = forReuse.remove(packName);
- if (oldPack != null) {
+ if (oldPack != null && oldPack.getFileSnapshot().isModified(packFile)) {
list.add(oldPack);
continue;
}
- final File packFile = new File(packDirectory, packName);
list.add(new PackFile(packFile, extensions));
foundNew = true;
}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java
index 4a96b6ef7..341157f89 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java
@@ -131,6 +131,8 @@ public class PackFile implements Iterable {
int packLastModified;
+ private FileSnapshot fileSnapshot;
+
private volatile boolean invalid;
private boolean invalidBitmap;
@@ -164,7 +166,8 @@ public class PackFile implements Iterable {
*/
public PackFile(final File packFile, int extensions) {
this.packFile = packFile;
- this.packLastModified = (int) (packFile.lastModified() >> 10);
+ this.fileSnapshot = FileSnapshot.save(packFile);
+ this.packLastModified = (int) (fileSnapshot.lastModified() >> 10);
this.extensions = extensions;
// Multiply by 31 here so we can more directly combine with another
@@ -359,6 +362,16 @@ public class PackFile implements Iterable {
return getReverseIdx().findObject(offset);
}
+ /**
+ * Return the @{@link FileSnapshot} associated to the underlying packfile
+ * that has been used when the object was created.
+ *
+ * @return the packfile @{@link FileSnapshot} that the object is loaded from.
+ */
+ FileSnapshot getFileSnapshot() {
+ return fileSnapshot;
+ }
+
private final byte[] decompress(final long position, final int sz,
final WindowCursor curs) throws IOException, DataFormatException {
byte[] dstbuf;
@@ -647,9 +660,10 @@ public class PackFile implements Iterable {
}
private void doOpen() throws IOException {
+ if (invalid) {
+ throw new PackInvalidException(packFile);
+ }
try {
- if (invalid)
- throw new PackInvalidException(packFile);
synchronized (readLock) {
fd = new RandomAccessFile(packFile, "r"); //$NON-NLS-1$
length = fd.length();
@@ -705,6 +719,14 @@ public class PackFile implements Iterable {
ByteArrayWindow read(final long pos, int size) throws IOException {
synchronized (readLock) {
+ if (invalid || fd == null) {
+ // Due to concurrency between a read and another packfile invalidation thread
+ // one thread could come up to this point and then fail with NPE.
+ // Detect the situation and throw a proper exception so that can be properly
+ // managed by the main packfile search loop and the Git client won't receive
+ // any failures.
+ throw new PackInvalidException(packFile);
+ }
if (length < pos + size)
size = (int) (length - pos);
final byte[] buf = new byte[size];
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java
index b442b3c32..76e6cbc0e 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java
@@ -55,6 +55,7 @@ import java.io.PrintStream;
import java.nio.charset.Charset;
import java.nio.file.Files;
import java.nio.file.Path;
+import java.nio.file.attribute.BasicFileAttributes;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.text.MessageFormat;
@@ -430,6 +431,19 @@ public abstract class FS {
*/
public abstract boolean retryFailedLockFileCommit();
+ /**
+ * Return all the attributes of a file, without following symbolic links.
+ *
+ * @param file
+ * @return {@link BasicFileAttributes} of the file
+ * @throws IOException in case of any I/O errors accessing the file
+ *
+ * @since 4.5.6
+ */
+ public BasicFileAttributes fileAttributes(File file) throws IOException {
+ return FileUtils.fileAttributes(file);
+ }
+
/**
* Determine the user's home directory (location where preferences are).
*
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FileUtils.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FileUtils.java
index 08c278e38..76a7eb6c8 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FileUtils.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FileUtils.java
@@ -684,6 +684,19 @@ public class FileUtils {
.toMillis();
}
+ /**
+ * Return all the attributes of a file, without following symbolic links.
+ *
+ * @param file
+ * @return {@link BasicFileAttributes} of the file
+ * @throws IOException in case of any I/O errors accessing the file
+ *
+ * @since 4.5.6
+ */
+ static BasicFileAttributes fileAttributes(File file) throws IOException {
+ return Files.readAttributes(file.toPath(), BasicFileAttributes.class, LinkOption.NOFOLLOW_LINKS);
+ }
+
/**
* @param file
* @param time