diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters new file mode 100644 index 000000000..803a29a5e --- /dev/null +++ b/org.eclipse.jgit/.settings/.api_filters @@ -0,0 +1,29 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + 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 576d0c769..eaaf7f01b 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 @@ 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} 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 @@ 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: @@ -731,6 +733,7 @@ unknownRepositoryFormat=Unknown repository format 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 a52fab926..5776ef211 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; @@ -792,6 +794,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 de1b8c1f8..0ecfb2868 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; @@ -947,6 +948,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 da9a050bc..b80c58ca9 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 495c75236..131f716cf 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 ecebd5408..f1a306e03 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java @@ -694,7 +694,7 @@ public static byte[] encode(String str) { /** * 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 3372bbe6a..6d3be7c68 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,12 +877,77 @@ public void createSymLink(File path, String target) throws IOException { * @return true if the file was created, false 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 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 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 3abf4dc5e..8795329bf 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} + *

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