Merge branch 'stable-5.0' into stable-5.1
* stable-5.0:
Fix atomic lock file creation on NFS
Use constant for ".lock"
Fix handling of option core.supportsAtomicCreateNewFile
GC: Avoid logging errors when deleting non-empty folders
Change-Id: I3c8892e33516bdcadd983e7ab668635b7fa3d4c5
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters
index 9f4b761..08fa8b3 100644
--- a/org.eclipse.jgit/.settings/.api_filters
+++ b/org.eclipse.jgit/.settings/.api_filters
@@ -1,17 +1,29 @@
<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<component id="org.eclipse.jgit" version="2">
- <resource path="src/org/eclipse/jgit/lib/ConfigConstants.java" type="org.eclipse.jgit.lib.ConfigConstants">
- <filter id="337768515">
+ <resource path="META-INF/MANIFEST.MF">
+ <filter id="924844039">
<message_arguments>
- <message_argument value="org.eclipse.jgit.lib.ConfigConstants"/>
+ <message_argument value="5.1.0"/>
+ <message_argument value="5.1.0"/>
</message_arguments>
</filter>
</resource>
- <resource path="src/org/eclipse/jgit/transport/GitProtocolConstants.java" type="org.eclipse.jgit.transport.GitProtocolConstants">
- <filter id="337768515">
+ <resource path="src/org/eclipse/jgit/util/FS.java" type="org.eclipse.jgit.util.FS">
+ <filter id="1141899266">
<message_arguments>
- <message_argument value="org.eclipse.jgit.transport.GitProtocolConstants"/>
+ <message_argument value="4.7"/>
+ <message_argument value="5.1"/>
+ <message_argument value="createNewFileAtomic(File)"/>
</message_arguments>
</filter>
</resource>
-</component>
\ No newline at end of file
+ <resource path="src/org/eclipse/jgit/util/FS.java" type="org.eclipse.jgit.util.FS$LockToken">
+ <filter id="1141899266">
+ <message_arguments>
+ <message_argument value="4.7"/>
+ <message_argument value="5.1"/>
+ <message_argument value="LockToken"/>
+ </message_arguments>
+ </filter>
+ </resource>
+</component>
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 abe5161..58f4519 100644
--- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
+++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
@@ -125,6 +125,7 @@
classCastNotA=Not a {0}
cloneNonEmptyDirectory=Destination path "{0}" already exists and is not an empty directory
closed=closed
+closeLockTokenFailed=Closing LockToken ''{0}'' failed
collisionOn=Collision on {0}
commandClosedStderrButDidntExit=Command {0} closed stderr stream but didn''t exit within timeout {1} seconds
commandRejectedByHook=Rejected by "{0}" hook.\n{1}
@@ -300,6 +301,7 @@
expectedPktLineWithService=expected pkt-line with ''# service=-'', got ''{0}''
expectedReceivedContentType=expected Content-Type {0}; received Content-Type {1}
expectedReportForRefNotReceived={0}: expected report for ref {1} not received
+failedAtomicFileCreation=Atomic file creation failed, number of hard links to file {0} was not 2 but {1}"
failedToDetermineFilterDefinition=An exception occured while determining filter definitions
failedUpdatingRefs=failed updating refs
failureDueToOneOfTheFollowing=Failure due to one of the following:
@@ -734,6 +736,7 @@
unknownRepositoryFormat2=Unknown repository format "{0}"; expected "0".
unknownTransportCommand=unknown command {0}
unknownZlibError=Unknown zlib error.
+unlockLockFileFailed=Unlocking LockFile ''{0}'' failed
unmergedPath=Unmerged path: {0}
unmergedPaths=Repository contains unmerged paths
unpackException=Exception while parsing pack stream
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 871d28d..96f428a 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
@@ -185,6 +185,7 @@ public static JGitText get() {
/***/ public String checkoutUnexpectedResult;
/***/ public String classCastNotA;
/***/ public String cloneNonEmptyDirectory;
+ /***/ public String closeLockTokenFailed;
/***/ public String closed;
/***/ public String collisionOn;
/***/ public String commandClosedStderrButDidntExit;
@@ -361,6 +362,7 @@ public static JGitText get() {
/***/ public String expectedPktLineWithService;
/***/ public String expectedReceivedContentType;
/***/ public String expectedReportForRefNotReceived;
+ /***/ public String failedAtomicFileCreation;
/***/ public String failedToDetermineFilterDefinition;
/***/ public String failedUpdatingRefs;
/***/ public String failureDueToOneOfTheFollowing;
@@ -795,6 +797,7 @@ public static JGitText get() {
/***/ public String unknownRepositoryFormat2;
/***/ public String unknownTransportCommand;
/***/ public String unknownZlibError;
+ /***/ public String unlockLockFileFailed;
/***/ public String unmergedPath;
/***/ public String unmergedPaths;
/***/ public String unpackException;
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java
index ed32c63..7bfec3f 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java
@@ -54,6 +54,7 @@
import java.io.StringWriter;
import java.nio.channels.Channels;
import java.nio.channels.FileChannel;
+import java.nio.file.DirectoryNotEmptyException;
import java.nio.file.DirectoryStream;
import java.nio.file.Files;
import java.nio.file.Path;
@@ -948,6 +949,8 @@ private boolean isDirectory(Path p) {
private void delete(Path d) {
try {
Files.delete(d);
+ } catch (DirectoryNotEmptyException e) {
+ // Don't log
} catch (IOException e) {
LOG.error(MessageFormat.format(JGitText.get().cannotDeleteFile, d),
e);
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 da9a050..b80c58c 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
@@ -63,7 +63,10 @@
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.util.FS;
+import org.eclipse.jgit.util.FS.LockToken;
import org.eclipse.jgit.util.FileUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* Git style file locking and replacement.
@@ -76,6 +79,7 @@
* name.
*/
public class LockFile {
+ private final static Logger LOG = LoggerFactory.getLogger(LockFile.class);
/**
* Unlock the given file.
@@ -133,6 +137,8 @@ public boolean accept(File dir, String name) {
private FileSnapshot commitSnapshot;
+ private LockToken token;
+
/**
* Create a new lock for any file.
*
@@ -155,7 +161,8 @@ public LockFile(File f) {
*/
public boolean lock() throws IOException {
FileUtils.mkdirs(lck.getParentFile(), true);
- if (FS.DETECTED.createNewFile(lck)) {
+ token = FS.DETECTED.createNewFileAtomic(lck);
+ if (token.isCreated()) {
haveLck = true;
try {
os = new FileOutputStream(lck);
@@ -163,6 +170,8 @@ public boolean lock() throws IOException {
unlock();
throw ioe;
}
+ } else {
+ closeToken();
}
return haveLck;
}
@@ -441,6 +450,7 @@ public boolean commit() {
try {
FileUtils.rename(lck, ref, StandardCopyOption.ATOMIC_MOVE);
haveLck = false;
+ closeToken();
return true;
} catch (IOException e) {
unlock();
@@ -448,6 +458,13 @@ public boolean commit() {
}
}
+ private void closeToken() {
+ if (token != null) {
+ token.close();
+ token = null;
+ }
+ }
+
private void saveStatInformation() {
if (needSnapshot)
commitSnapshot = FileSnapshot.save(lck);
@@ -490,8 +507,9 @@ public void unlock() {
if (os != null) {
try {
os.close();
- } catch (IOException ioe) {
- // Ignore this
+ } catch (IOException e) {
+ LOG.error(MessageFormat
+ .format(JGitText.get().unlockLockFileFailed, lck), e);
}
os = null;
}
@@ -501,7 +519,10 @@ public void unlock() {
try {
FileUtils.delete(lck, FileUtils.RETRY);
} catch (IOException e) {
- // couldn't delete the file even after retry.
+ LOG.error(MessageFormat
+ .format(JGitText.get().unlockLockFileFailed, lck), e);
+ } finally {
+ closeToken();
}
}
}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ReflogWriter.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ReflogWriter.java
index 495c752..131f716 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ReflogWriter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ReflogWriter.java
@@ -271,4 +271,4 @@ private boolean shouldAutoCreateLog(String refName) {
|| refName.startsWith(R_REMOTES)
|| refName.startsWith(R_NOTES);
}
-}
+}
\ No newline at end of file
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java
index f02e95a..9be4c9f 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java
@@ -712,7 +712,7 @@ public static int decodeTypeString(final AnyObjectId id,
/**
* Suffix of lock file name
*
- * @since 5.0
+ * @since 4.8
*/
public static final String LOCK_SUFFIX = ".lock"; //$NON-NLS-1$
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 3372bbe..6d3be7c 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java
@@ -45,6 +45,7 @@
import java.io.BufferedReader;
import java.io.ByteArrayInputStream;
+import java.io.Closeable;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
@@ -52,6 +53,8 @@
import java.io.OutputStream;
import java.io.PrintStream;
import java.nio.charset.Charset;
+import java.nio.file.Files;
+import java.nio.file.Path;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.text.MessageFormat;
@@ -59,6 +62,7 @@
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
+import java.util.Optional;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;
@@ -873,13 +877,78 @@ public void createSymLink(File path, String target) throws IOException {
* @return <code>true</code> if the file was created, <code>false</code> if
* the file already existed
* @throws java.io.IOException
+ * @deprecated use {@link #createNewFileAtomic(File)} instead
* @since 4.5
*/
+ @Deprecated
public boolean createNewFile(File path) throws IOException {
return path.createNewFile();
}
/**
+ * A token representing a file created by
+ * {@link #createNewFileAtomic(File)}. The token must be retained until the
+ * file has been deleted in order to guarantee that the unique file was
+ * created atomically. As soon as the file is no longer needed the lock
+ * token must be closed.
+ *
+ * @since 4.7
+ */
+ public static class LockToken implements Closeable {
+ private boolean isCreated;
+
+ private Optional<Path> link;
+
+ LockToken(boolean isCreated, Optional<Path> link) {
+ this.isCreated = isCreated;
+ this.link = link;
+ }
+
+ /**
+ * @return {@code true} if the file was created successfully
+ */
+ public boolean isCreated() {
+ return isCreated;
+ }
+
+ @Override
+ public void close() {
+ if (link.isPresent()) {
+ try {
+ Files.delete(link.get());
+ } catch (IOException e) {
+ LOG.error(MessageFormat.format(JGitText.get().closeLockTokenFailed,
+ this), e);
+ }
+ }
+ }
+
+ @Override
+ public String toString() {
+ return "LockToken [lockCreated=" + isCreated + //$NON-NLS-1$
+ ", link=" //$NON-NLS-1$
+ + (link.isPresent() ? link.get().getFileName() + "]" //$NON-NLS-1$
+ : "<null>]"); //$NON-NLS-1$
+ }
+ }
+
+ /**
+ * Create a new file. See {@link java.io.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 LockToken this token must be closed after the created file was
+ * deleted
+ * @throws IOException
+ * @since 4.7
+ */
+ public LockToken createNewFileAtomic(File path) throws IOException {
+ return new LockToken(path.createNewFile(), Optional.empty());
+ }
+
+ /**
* See
* {@link org.eclipse.jgit.util.FileUtils#relativizePath(String, String, String, boolean)}.
*
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 3abf4dc..8795329 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
@@ -52,14 +52,19 @@
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.attribute.PosixFilePermission;
+import java.text.MessageFormat;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
+import java.util.Optional;
import java.util.Set;
+import java.util.UUID;
+import org.eclipse.jgit.annotations.Nullable;
import org.eclipse.jgit.api.errors.JGitInternalException;
import org.eclipse.jgit.errors.CommandFailedException;
import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.internal.JGitText;
import org.eclipse.jgit.lib.ConfigConstants;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.Repository;
@@ -372,9 +377,12 @@ public boolean supportsAtomicCreateNewFile() {
* 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
+ * @see "https://www.time-travellers.org/shane/papers/NFS_considered_harmful.html"
+ *
+ * @deprecated use {@link FS_POSIX#createNewFileAtomic(File)} instead
* @since 4.5
*/
+ @Deprecated
public boolean createNewFile(File lock) throws IOException {
if (!lock.createNewFile()) {
return false;
@@ -383,22 +391,94 @@ public boolean createNewFile(File lock) throws IOException {
return true;
}
Path lockPath = lock.toPath();
- Path link = Files.createLink(Paths.get(lock.getAbsolutePath() + ".lnk"), //$NON-NLS-1$
- lockPath);
+ Path link = null;
try {
+ link = Files.createLink(
+ Paths.get(lock.getAbsolutePath() + ".lnk"), //$NON-NLS-1$
+ lockPath);
Integer nlink = (Integer) (Files.getAttribute(lockPath,
"unix:nlink")); //$NON-NLS-1$
- if (nlink != 2) {
+ if (nlink > 2) {
LOG.warn("nlink of link to lock file {} was not 2 but {}", //$NON-NLS-1$
lock.getPath(), nlink);
return false;
+ } else if (nlink < 2) {
+ supportsUnixNLink = false;
}
return true;
} catch (UnsupportedOperationException | IllegalArgumentException e) {
supportsUnixNLink = false;
return true;
} finally {
- Files.delete(link);
+ if (link != null) {
+ Files.delete(link);
+ }
}
}
+
+ /**
+ * {@inheritDoc}
+ * <p>
+ * An implementation of the File#createNewFile() semantics which can create
+ * a unique file atomically 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 hard link 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. The hard link needs to be retained
+ * until the corresponding file is no longer needed in order to prevent that
+ * another process can create the same file concurrently using another NFS
+ * client which might not yet see the file due to caching.
+ *
+ * @see "https://www.time-travellers.org/shane/papers/NFS_considered_harmful.html"
+ * @param file
+ * the unique file to be created atomically
+ * @return LockToken this lock token must be held until the file is no
+ * longer needed
+ * @throws IOException
+ * @since 5.0
+ */
+ @Override
+ public LockToken createNewFileAtomic(File file) throws IOException {
+ if (!file.createNewFile()) {
+ return token(false, null);
+ }
+ if (supportsAtomicCreateNewFile() || !supportsUnixNLink) {
+ return token(true, null);
+ }
+ Path link = null;
+ Path path = file.toPath();
+ try {
+ link = Files.createLink(Paths.get(uniqueLinkPath(file)), path);
+ Integer nlink = (Integer) (Files.getAttribute(path,
+ "unix:nlink")); //$NON-NLS-1$
+ if (nlink.intValue() > 2) {
+ LOG.warn(MessageFormat.format(
+ JGitText.get().failedAtomicFileCreation, path, nlink));
+ return token(false, link);
+ } else if (nlink.intValue() < 2) {
+ supportsUnixNLink = false;
+ }
+ return token(true, link);
+ } catch (UnsupportedOperationException | IllegalArgumentException e) {
+ supportsUnixNLink = false;
+ return token(true, link);
+ }
+ }
+
+ private static LockToken token(boolean created, @Nullable Path p) {
+ return ((p != null) && Files.exists(p))
+ ? new LockToken(created, Optional.of(p))
+ : new LockToken(created, Optional.empty());
+ }
+
+ private static String uniqueLinkPath(File file) {
+ UUID id = UUID.randomUUID();
+ return file.getAbsolutePath() + "." //$NON-NLS-1$
+ + Long.toHexString(id.getMostSignificantBits())
+ + Long.toHexString(id.getLeastSignificantBits());
+ }
}
