Fix LockFile semantics when running on NFS

When running on NFS there was a chance that JGits LockFile
semantic is broken because File#createNewFile() may allow
multiple clients to create the same file in parallel. This
change provides a fix which is only used when the new config
option core.supportsAtomicCreateNewFile is set to false. The
default for this option is true. This option can only be set in the
global or the system config file. The repository config file is not
taken into account in this case.

If the config option core.supportsAtomicCreateNewFile is true
then File#createNewFile() is trusted and the behaviour doesn't
change.

But if core.supportsAtomicCreateNewFile is set to false then after
successful creation of the lock file a hardlink to that lock file is
created and the attribute nlink of the lock file is checked to be 2. If
multiple clients manage to create the same lock file nlink would be
greater than 2 showing the error.

This expensive workaround is described in
 https://www.time-travellers.org/shane/papers/NFS_considered_harmful.html
section III.d) "Exclusive File Creation"

Change-Id: I3d2cc48d8eb280d5f7039eb94da37804f903be6a

4 files changed, 125 insertions, 1 deletions

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/LockFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/LockFile.java
index ce9677a62d..51af67e0bd 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/LockFile.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/LockFile.java
@@ -168,7 +168,7 @@ public class LockFile {
 	 */
 	public boolean lock() throws IOException {
 		FileUtils.mkdirs(lck.getParentFile(), true);
-		if (lck.createNewFile()) {
+		if (FS.DETECTED.createNewFile(lck)) {
 			haveLck = true;
 			try {
 				os = new FileOutputStream(lck);
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java
index 9e3e0b78fd..e3f8ba5b5b 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java
@@ -290,6 +290,13 @@ public class ConfigConstants {
 	public static final String CONFIG_KEY_TRUSTFOLDERSTAT = "trustfolderstat";
 
 	/**
+	 * The "supportsAtomicFileCreation" key in the "core section"
+	 *
+	 * @since 4.5
+	 */
+	public static final String CONFIG_KEY_SUPPORTSATOMICFILECREATION = "supportsatomicfilecreation";
+
+	/**
 	 * The "noprefix" key in the "diff section"
 	 * @since 3.0
 	 */
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 a55b5c0ac5..e1fd1cb889 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java
@@ -237,6 +237,21 @@ public abstract class FS {
 	public abstract boolean supportsExecute();
 
 	/**
+	 * Does this file system support atomic file creation via
+	 * java.io.File#createNewFile()? In certain environments (e.g. on NFS) it is
+	 * not guaranteed that when two file system clients run createNewFile() in
+	 * parallel only one will succeed. In such cases both clients may think they
+	 * created a new file.
+	 *
+	 * @return true if this implementation support atomic creation of new
+	 *         Files by {@link File#createNewFile()}
+	 * @since 4.5
+	 */
+	public boolean supportsAtomicCreateNewFile() {
+		return true;
+	}
+
+	/**
 	 * Does this operating system and JRE supports symbolic links. The
 	 * capability to handle symbolic links is detected at runtime.
 	 *
@@ -777,6 +792,22 @@ public abstract class FS {
 	}
 
 	/**
+	 * Create a new file. See {@link File#createNewFile()}. Subclasses of this
+	 * class may take care to provide a safe implementation for this even if
+	 * {@link #supportsAtomicCreateNewFile()} is <code>false</code>
+	 *
+	 * @param path
+	 *            the file to be created
+	 * @return <code>true</code> if the file was created, <code>false</code> if
+	 *         the file already existed
+	 * @throws IOException
+	 * @since 4.5
+	 */
+	public boolean createNewFile(File path) throws IOException {
+		return path.createNewFile();
+	}
+
+	/**
 	 * See {@link FileUtils#relativize(String, String)}.
 	 *
 	 * @param base
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java
index cb4868cc7a..4ecaf9c8ee 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java
@@ -50,6 +50,7 @@ import java.io.PrintStream;
 import java.nio.charset.Charset;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.nio.file.attribute.PosixFilePermission;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -58,8 +59,11 @@ import java.util.Set;
 
 import org.eclipse.jgit.api.errors.JGitInternalException;
 import org.eclipse.jgit.errors.CommandFailedException;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.ConfigConstants;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.storage.file.FileBasedConfig;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -74,6 +78,10 @@ public class FS_POSIX extends FS {
 	private static final int DEFAULT_UMASK = 0022;
 	private volatile int umask = -1;
 
+	private volatile boolean supportsUnixNLink = true;
+
+	private volatile Boolean supportsAtomicCreateNewFile;
+
 	/** Default constructor. */
 	protected FS_POSIX() {
 	}
@@ -91,6 +99,33 @@ public class FS_POSIX extends FS {
 		}
 	}
 
+	private void determineAtomicFileCreationSupport() {
+		// @TODO: enhance SystemReader to support this without copying code
+		Boolean ret = getAtomicFileCreationSupportOption(
+				SystemReader.getInstance().openUserConfig(null, this));
+		if (ret == null && StringUtils.isEmptyOrNull(SystemReader.getInstance()
+				.getenv(Constants.GIT_CONFIG_NOSYSTEM_KEY))) {
+			ret = getAtomicFileCreationSupportOption(
+					SystemReader.getInstance().openSystemConfig(null, this));
+		}
+		supportsAtomicCreateNewFile = (ret == null) || ret;
+	}
+
+	private Boolean getAtomicFileCreationSupportOption(FileBasedConfig config) {
+		try {
+			config.load();
+			String value = config.getString(ConfigConstants.CONFIG_CORE_SECTION,
+					null,
+					ConfigConstants.CONFIG_KEY_SUPPORTSATOMICFILECREATION);
+			if (value == null) {
+				return null;
+			}
+			return Boolean.valueOf(StringUtils.toBoolean(value));
+		} catch (IOException | ConfigInvalidException e) {
+			return Boolean.TRUE;
+		}
+	}
+
 	@Override
 	public FS newInstance() {
 		return new FS_POSIX(this);
@@ -301,4 +336,55 @@ public class FS_POSIX extends FS {
 			return hookPath.toFile();
 		return null;
 	}
+
+	@Override
+	public boolean supportsAtomicCreateNewFile() {
+		if (supportsAtomicCreateNewFile == null) {
+			determineAtomicFileCreationSupport();
+		}
+		return supportsAtomicCreateNewFile.booleanValue();
+	}
+
+	@SuppressWarnings("boxing")
+	/**
+	 * An implementation of the File#createNewFile() semantics which works also
+	 * on NFS. If the config option
+	 * {@code core.supportsAtomicCreateNewFile = true} (which is the default)
+	 * then simply File#createNewFile() is called.
+	 *
+	 * But if {@code core.supportsAtomicCreateNewFile = false} then after
+	 * successful creation of the lock file a hardlink to that lock file is
+	 * created and the attribute nlink of the lock file is checked to be 2. If
+	 * multiple clients manage to create the same lock file nlink would be
+	 * greater than 2 showing the error.
+	 *
+	 * @see https://www.time-travellers.org/shane/papers/NFS_considered_harmful.html
+	 * @since 4.5
+	 */
+	public boolean createNewFile(File lock) throws IOException {
+		if (!lock.createNewFile()) {
+			return false;
+		}
+		if (supportsAtomicCreateNewFile() || !supportsUnixNLink) {
+			return true;
+		}
+		Path lockPath = lock.toPath();
+		Path link = Files.createLink(Paths.get(lock.getAbsolutePath() + ".lnk"), //$NON-NLS-1$
+				lockPath);
+		try {
+			Integer nlink = (Integer) (Files.getAttribute(lockPath,
+					"unix:nlink")); //$NON-NLS-1$
+			if (nlink != 2) {
+				LOG.warn("nlink of link to lock file {0} was not 2 but {1}", //$NON-NLS-1$
+						lock.getPath(), nlink);
+				return false;
+			}
+			return true;
+		} catch (UnsupportedOperationException | IllegalArgumentException e) {
+			supportsUnixNLink = false;
+			return true;
+		} finally {
+			Files.delete(link);
+		}
+	}
 }