From b7351facd51aa8e14d4bd2e74e3f4f0b57c55f6f Mon Sep 17 00:00:00 2001 From: Hector Caballero Date: Tue, 4 Sep 2018 11:15:44 -0400 Subject: [PATCH 1/4] GC: Avoid logging errors when deleting non-empty folders I88304d34c and Ia555bce00 modified the way errors are handled when trying to delete non-empty reference folders. Before, this error was silently ignored as it was considered an expected output. Now, every failed folder delete is logged which can be noisy. Ignore the DirectoryNotEmptyException but log any other error avoiding deletion of an eligible folder. Signed-off-by: Hector Oswaldo Caballero Change-Id: I194512f67885231d62c03976ae683e5cc450ec7c --- .../src/org/eclipse/jgit/internal/storage/file/GC.java | 3 +++ 1 file changed, 3 insertions(+) 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 e4b0a46a3..a4a2baa55 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.PrintWriter; 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; @@ -909,6 +910,8 @@ public class GC { 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); From 2faccd5b3206e996c9e2cafb95d806d50a9b9d4d Mon Sep 17 00:00:00 2001 From: Christian Halstrick Date: Fri, 17 Aug 2018 17:46:09 +0200 Subject: [PATCH 2/4] Fix handling of option core.supportsAtomicCreateNewFile When core.supportsAtomicCreateNewFile was set to false and the repository was located on a filesystem which doesn't support the file attribute "unix:nlink" then FS_POSIX#createNewFile may report an error even if everything was ok. Modify FS_POSIX#createNewFile to silently ignore this situation. An example of such a filesystem is sshfs where reading "unix:nlink" always returns 1 (instead of throwing a exception). Bug: 537969 Change-Id: I6deda7672fa7945efa8706ea1cd652272604ff19 Also-by: Thomas Wolf --- .../src/org/eclipse/jgit/util/FS_POSIX.java | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) 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 76aa69776..9f273d1b8 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 @@ -371,22 +371,28 @@ public class FS_POSIX extends FS { 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 {0} was not 2 but {1}", //$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); + } } } } From 6bec391d3693a38f812473413db2b4f73acf70fd Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Fri, 8 Jun 2018 17:22:54 +0200 Subject: [PATCH 3/4] Use constant for ".lock" (cherry picked from commit 5f27032fb85694a093f827581216d4ffb99db68b) Change-Id: I6bc0e9a910b110418a82d8e574fb2aecc3a31d6a Signed-off-by: Matthias Sohn --- .../jgit/internal/storage/file/RefUpdateTest.java | 9 +++++---- org.eclipse.jgit/.settings/.api_filters | 2 +- .../org/eclipse/jgit/internal/storage/file/LockFile.java | 8 +++++--- .../eclipse/jgit/internal/storage/file/ReflogWriter.java | 5 +++-- org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java | 7 +++++++ .../src/org/eclipse/jgit/lib/Repository.java | 8 ++++++-- .../src/org/eclipse/jgit/transport/TransportSftp.java | 4 +++- 7 files changed, 30 insertions(+), 13 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefUpdateTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefUpdateTest.java index daef91d53..0f26b0fa6 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefUpdateTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefUpdateTest.java @@ -46,6 +46,7 @@ package org.eclipse.jgit.internal.storage.file; import static org.eclipse.jgit.junit.Assert.assertEquals; +import static org.eclipse.jgit.lib.Constants.LOCK_SUFFIX; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -758,11 +759,11 @@ public class RefUpdateTest extends SampleDataRepositoryTestCase { // Check that the involved refs are the same despite the failure assertExists(false, toName); if (!toLock.equals(toName)) - assertExists(false, toName + ".lock"); - assertExists(true, toLock + ".lock"); + assertExists(false, toName + LOCK_SUFFIX); + assertExists(true, toLock + LOCK_SUFFIX); if (!toLock.equals(fromName)) - assertExists(false, "logs/" + fromName + ".lock"); - assertExists(false, "logs/" + toName + ".lock"); + assertExists(false, "logs/" + fromName + LOCK_SUFFIX); + assertExists(false, "logs/" + toName + LOCK_SUFFIX); assertEquals(oldHeadId, db.resolve(Constants.HEAD)); assertEquals(oldfromId, db.resolve(fromName)); assertNull(db.resolve(toName)); diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters index c68b9a093..40255670a 100644 --- a/org.eclipse.jgit/.settings/.api_filters +++ b/org.eclipse.jgit/.settings/.api_filters @@ -3,7 +3,7 @@ - + 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 5af93cf2c..584c4714d 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 @@ -44,6 +44,8 @@ package org.eclipse.jgit.internal.storage.file; +import static org.eclipse.jgit.lib.Constants.LOCK_SUFFIX; + import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; @@ -75,7 +77,6 @@ import org.eclipse.jgit.util.FileUtils; * name. */ public class LockFile { - static final String SUFFIX = ".lock"; //$NON-NLS-1$ /** * Unlock the given file. @@ -105,14 +106,15 @@ public class LockFile { * @return lock file */ static File getLockFile(File file) { - return new File(file.getParentFile(), file.getName() + SUFFIX); + return new File(file.getParentFile(), + file.getName() + LOCK_SUFFIX); } /** Filter to skip over active lock files when listing a directory. */ static final FilenameFilter FILTER = new FilenameFilter() { @Override public boolean accept(File dir, String name) { - return !name.endsWith(SUFFIX); + return !name.endsWith(LOCK_SUFFIX); } }; 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 24d2c790e..892c1c85a 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 @@ -47,6 +47,7 @@ package org.eclipse.jgit.internal.storage.file; import static org.eclipse.jgit.lib.Constants.HEAD; import static org.eclipse.jgit.lib.Constants.LOGS; +import static org.eclipse.jgit.lib.Constants.LOCK_SUFFIX; import static org.eclipse.jgit.lib.Constants.R_HEADS; import static org.eclipse.jgit.lib.Constants.R_REFS; import static org.eclipse.jgit.lib.Constants.R_REMOTES; @@ -86,7 +87,7 @@ public class ReflogWriter { * @return the name of the ref's lock ref */ public static String refLockFor(final String name) { - return name + LockFile.SUFFIX; + return name + LOCK_SUFFIX; } private final Repository parent; @@ -287,4 +288,4 @@ public class ReflogWriter { || refName.startsWith(R_REMOTES) // || refName.equals(R_STASH); } -} \ 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 ff80672f8..bda1a2745 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java @@ -665,6 +665,13 @@ public final class Constants { public static final ObjectId EMPTY_BLOB_ID = ObjectId .fromString("e69de29bb2d1d6434b8b29ae775ad8c2e48c5391"); + /** + * Suffix of lock file name + * + * @since 4.7 + */ + public static final String LOCK_SUFFIX = ".lock"; //$NON-NLS-1$ + private Constants() { // Hide the default constructor } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java index ea37e7938..aa70f4209 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java @@ -48,6 +48,8 @@ package org.eclipse.jgit.lib; +import static org.eclipse.jgit.lib.Constants.LOCK_SUFFIX; + import java.io.BufferedOutputStream; import java.io.File; import java.io.FileNotFoundException; @@ -1293,10 +1295,12 @@ public abstract class Repository implements AutoCloseable { */ public static boolean isValidRefName(final String refName) { final int len = refName.length(); - if (len == 0) + if (len == 0) { return false; - if (refName.endsWith(".lock")) //$NON-NLS-1$ + } + if (refName.endsWith(LOCK_SUFFIX)) { return false; + } // Refs may be stored as loose files so invalid paths // on the local system must also be invalid refs. diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportSftp.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportSftp.java index c46f94b7d..99e4809cf 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportSftp.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportSftp.java @@ -43,6 +43,8 @@ package org.eclipse.jgit.transport; +import static org.eclipse.jgit.lib.Constants.LOCK_SUFFIX; + import java.io.BufferedReader; import java.io.FileNotFoundException; import java.io.IOException; @@ -341,7 +343,7 @@ public class TransportSftp extends SshTransport implements WalkTransport { @Override void writeFile(final String path, final byte[] data) throws IOException { - final String lock = path + ".lock"; //$NON-NLS-1$ + final String lock = path + LOCK_SUFFIX; try { super.writeFile(lock, data); try { From 06e06fc291a8ccc0e68b7d84b54097a9635df740 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sun, 26 Aug 2018 19:44:29 +0200 Subject: [PATCH 4/4] Fix atomic lock file creation on NFS FS_POSIX.createNewFile(File) failed to properly implement atomic file creation on NFS using the algorithm [1]: - name of the hard link must be unique to prevent that two processes using different NFS clients try to create the same link. This would render nlink useless to detect if there was a race. - the hard link must be retained for the lifetime of the file since we don't know when the state of the involved NFS clients will be synchronized. This depends on NFS configuration options. To fix these issues we need to change the signature of createNewFile which would break API. Hence deprecate the old method FS.createNewFile(File) and add a new method createNewFileAtomic(File). The new method returns a LockToken which needs to be retained by the caller (LockFile) until all involved NFS clients synchronized their state. Since we don't know when the NFS caches are synchronized we need to retain the token until the corresponding file is no longer needed. The LockToken must be closed after the LockFile using it has been committed or unlocked. On Posix, if core.supportsAtomicCreateNewFile = false this will delete the hard link which guarded the atomic creation of the file. When acquiring the lock fails ensure that the hard link is removed. [1] https://www.time-travellers.org/shane/papers/NFS_considered_harmful.html also see file creation flag O_EXCL in http://man7.org/linux/man-pages/man2/open.2.html Change-Id: I84fcb16143a5f877e9b08c6ee0ff8fa4ea68a90d Signed-off-by: Matthias Sohn --- .../eclipse/jgit/internal/JGitText.properties | 3 + .../org/eclipse/jgit/internal/JGitText.java | 3 + .../jgit/internal/storage/file/LockFile.java | 29 ++++++- .../src/org/eclipse/jgit/util/FS.java | 71 ++++++++++++++++- .../src/org/eclipse/jgit/util/FS_POSIX.java | 76 ++++++++++++++++++- 5 files changed, 176 insertions(+), 6 deletions(-) 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 16f184bab..873cf526b 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -119,6 +119,7 @@ checkoutUnexpectedResult=Checkout returned unexpected result {0} 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} commandRejectedByHook=Rejected by "{0}" hook.\n{1} commandWasCalledInTheWrongState=Command {0} was called in the wrong state @@ -284,6 +285,7 @@ expectedLessThanGot=expected less than ''{0}'', got ''{1}'' 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: @@ -667,6 +669,7 @@ unknownObjectType2=unknown unknownRepositoryFormat=Unknown repository format unknownRepositoryFormat2=Unknown repository format "{0}"; expected "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 4b7459c15..9ea0aa570 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -177,6 +177,7 @@ public class JGitText extends TranslationBundle { /***/ public String checkoutUnexpectedResult; /***/ public String classCastNotA; /***/ public String cloneNonEmptyDirectory; + /***/ public String closeLockTokenFailed; /***/ public String closed; /***/ public String collisionOn; /***/ public String commandRejectedByHook; @@ -343,6 +344,7 @@ public class JGitText extends TranslationBundle { /***/ public String expectedPktLineWithService; /***/ public String expectedReceivedContentType; /***/ public String expectedReportForRefNotReceived; + /***/ public String failedAtomicFileCreation; /***/ public String failedToDetermineFilterDefinition; /***/ public String failedUpdatingRefs; /***/ public String failureDueToOneOfTheFollowing; @@ -726,6 +728,7 @@ public class JGitText extends TranslationBundle { /***/ public String unknownRepositoryFormat; /***/ public String unknownRepositoryFormat2; /***/ 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/LockFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/LockFile.java index 584c4714d..15c528051 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 @@ -64,7 +64,10 @@ import org.eclipse.jgit.internal.JGitText; 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. @@ -77,6 +80,7 @@ import org.eclipse.jgit.util.FileUtils; * name. */ public class LockFile { + private final static Logger LOG = LoggerFactory.getLogger(LockFile.class); /** * Unlock the given file. @@ -132,6 +136,8 @@ public class LockFile { private FileSnapshot commitSnapshot; + private LockToken token; + /** * Create a new lock for any file. * @@ -170,7 +176,8 @@ public class LockFile { */ 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); @@ -178,6 +185,8 @@ public class LockFile { unlock(); throw ioe; } + } else { + closeToken(); } return haveLck; } @@ -458,6 +467,7 @@ public class LockFile { try { FileUtils.rename(lck, ref, StandardCopyOption.ATOMIC_MOVE); haveLck = false; + closeToken(); return true; } catch (IOException e) { unlock(); @@ -465,6 +475,13 @@ public class LockFile { } } + private void closeToken() { + if (token != null) { + token.close(); + token = null; + } + } + private void saveStatInformation() { if (needSnapshot) commitSnapshot = FileSnapshot.save(lck); @@ -503,8 +520,9 @@ public class LockFile { 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; } @@ -514,7 +532,10 @@ public class LockFile { 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/util/FS.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java index 98e73216b..6b537a477 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 @@ package org.eclipse.jgit.util; 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.InputStreamReader; 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.Arrays; 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; @@ -800,13 +804,78 @@ public abstract class FS { * the file to be created * @return true if the file was created, false if * the file already existed - * @throws IOException + * @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 link; + + LockToken(boolean isCreated, Optional 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$ + : "]"); //$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 false + * + * @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 FileUtils#relativize(String, String)}. * 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 9f273d1b8..607e07860 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.Files; 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; @@ -360,9 +365,12 @@ public class FS_POSIX extends FS { * 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; @@ -395,4 +403,70 @@ public class FS_POSIX extends FS { } } } + + /** + * {@inheritDoc} + *

+ * 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()); + } }