Merge branch 'stable-5.3'

* stable-5.3:
  Prepare 5.3.1-SNAPSHOT builds
  JGit v5.3.0.201903130848-r
  Prepare 5.1.7-SNAPSHOT builds
  JGit v5.1.6.201903130242-r
  Prepare 4.11.8-SNAPSHOT builds
  JGit v4.11.7.201903122105-r
  Prepare 4.9.10-SNAPSHOT builds
  JGit v4.9.9.201903122025-r
  Prepare 4.7.9-SNAPSHOT builds
  JGit v4.7.8.201903121755-r
  Prepare 4.5.7-SNAPSHOT builds
  JGit v4.5.6.201903121547-r
  Check for packfile validity and fd before reading
  Move throw of PackInvalidException outside the catch
  Use FileSnapshot to get lastModified on PackFile
  Include size when comparing FileSnapshot
  Do not reuse packfiles when changed on filesystem
  Silence API warnings for new API introduced for fixes

Change-Id: Ief9f6a9e89c4a1ef122daedbfd933676393fab06
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

6 files changed, 95 insertions, 91 deletions

diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters
index ac2d094327..433175b980 100644
--- a/org.eclipse.jgit/.settings/.api_filters
+++ b/org.eclipse.jgit/.settings/.api_filters
@@ -1,84 +1,10 @@
 <?xml version="1.0" encoding="UTF-8" standalone="no"?>
 <component id="org.eclipse.jgit" version="2">
-    <resource path="src/org/eclipse/jgit/lib/ObjectIdRef.java" type="org.eclipse.jgit.lib.ObjectIdRef">
-        <filter comment="removing protected constructor breaks implementers which is ok in a minor release following OSGi semantic versioning" id="338722907">
+    <resource path="src/org/eclipse/jgit/util/FS.java" type="org.eclipse.jgit.util.FS">
+        <filter id="1142947843">
             <message_arguments>
-                <message_argument value="org.eclipse.jgit.lib.ObjectIdRef"/>
-                <message_argument value="ObjectIdRef(Ref.Storage, String, ObjectId)"/>
-            </message_arguments>
-        </filter>
-    </resource>
-    <resource path="src/org/eclipse/jgit/lib/Ref.java" type="org.eclipse.jgit.lib.Ref">
-        <filter id="404000815">
-            <message_arguments>
-                <message_argument value="org.eclipse.jgit.lib.Ref"/>
-                <message_argument value="getUpdateIndex()"/>
-            </message_arguments>
-        </filter>
-    </resource>
-    <resource path="src/org/eclipse/jgit/lib/RefDatabase.java" type="org.eclipse.jgit.lib.RefDatabase">
-        <filter comment="class is extended by extenders but not clients of the API" id="421650549">
-            <message_arguments>
-                <message_argument value="org.eclipse.jgit.lib.RefDatabase"/>
-                <message_argument value="exactRef(String)"/>
-            </message_arguments>
-        </filter>
-        <filter id="421654647">
-            <message_arguments>
-                <message_argument value="org.eclipse.jgit.lib.RefDatabase"/>
-                <message_argument value="getRef(String)"/>
-            </message_arguments>
-        </filter>
-    </resource>
-    <resource path="src/org/eclipse/jgit/transport/BaseReceivePack.java" type="org.eclipse.jgit.transport.BaseReceivePack">
-        <filter id="421650549">
-            <message_arguments>
-                <message_argument value="org.eclipse.jgit.transport.BaseReceivePack"/>
-                <message_argument value="getAdvertisedRefs()"/>
-            </message_arguments>
-        </filter>
-        <filter id="421650549">
-            <message_arguments>
-                <message_argument value="org.eclipse.jgit.transport.BaseReceivePack"/>
-                <message_argument value="getPushCertificate()"/>
-            </message_arguments>
-        </filter>
-        <filter id="421650549">
-            <message_arguments>
-                <message_argument value="org.eclipse.jgit.transport.BaseReceivePack"/>
-                <message_argument value="getRepository()"/>
-            </message_arguments>
-        </filter>
-        <filter id="421650549">
-            <message_arguments>
-                <message_argument value="org.eclipse.jgit.transport.BaseReceivePack"/>
-                <message_argument value="getRevWalk()"/>
-            </message_arguments>
-        </filter>
-        <filter id="421650549">
-            <message_arguments>
-                <message_argument value="org.eclipse.jgit.transport.BaseReceivePack"/>
-                <message_argument value="setAdvertisedRefs(Map&lt;String,Ref&gt;, Set&lt;ObjectId&gt;)"/>
-            </message_arguments>
-        </filter>
-        <filter id="421650549">
-            <message_arguments>
-                <message_argument value="org.eclipse.jgit.transport.BaseReceivePack"/>
-                <message_argument value="setPushCertificate(PushCertificate)"/>
-            </message_arguments>
-        </filter>
-    </resource>
-    <resource path="src/org/eclipse/jgit/transport/TransferConfig.java" type="org.eclipse.jgit.transport.TransferConfig">
-        <filter id="1159725059">
-            <message_arguments>
-                <message_argument value="5.1.4"/>
-                <message_argument value="TransferConfig(Config)"/>
-            </message_arguments>
-        </filter>
-        <filter id="1159725059">
-            <message_arguments>
-                <message_argument value="5.1.4"/>
-                <message_argument value="TransferConfig(Repository)"/>
+                <message_argument value="4.5.6"/>
+                <message_argument value="fileAttributes(File)"/>
             </message_arguments>
         </filter>
     </resource>
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 10adc6ccc0..f26eba3360 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;
@@ -70,13 +71,20 @@ import org.eclipse.jgit.util.FS;
  */
 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.
 	 * <p>
 	 * This instance is useful for application code that wants to lazily read a
 	 * 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;
 	}
 
 	/**
@@ -153,6 +171,13 @@ public class FileSnapshot {
 	}
 
 	/**
+	 * @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.
 	 *
 	 * @param path
@@ -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 d4a8adcbbf..3ffb8c424d 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
@@ -914,13 +914,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 7549274efd..ee3c0a8dd9 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<PackIndex.MutableEntry> {
 
 	int packLastModified;
 
+	private FileSnapshot fileSnapshot;
+
 	private volatile boolean invalid;
 
 	private boolean invalidBitmap;
@@ -164,7 +166,8 @@ public class PackFile implements Iterable<PackIndex.MutableEntry> {
 	 */
 	public PackFile(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<PackIndex.MutableEntry> {
 		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<PackIndex.MutableEntry> {
 	}
 
 	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<PackIndex.MutableEntry> {
 
 	ByteArrayWindow read(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 e864ba76c1..536e4f1dfa 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java
@@ -57,6 +57,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;
@@ -444,6 +445,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).
 	 *
 	 * @return the user's home directory; null if the user does not have one.
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 ecfd31647d..97f480dd36 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FileUtils.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FileUtils.java
@@ -660,6 +660,19 @@ public class FileUtils {
 	}
 
 	/**
+	 * 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
 	 * @throws IOException