From a606dc363d0f6b09e4527cca6b645d3cb1ec407d Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Tue, 25 Nov 2014 15:10:41 -0800 Subject: [PATCH] Cleanup double stat update of symlinks in DirCacheCheckout When writing a symlink the stat data should only be written once into the DirCacheEntry, based on the symlink itself and not the possibly resolved destination observed by java.io.File. Refactor the code to handle symlinks and early return. This removes the risk the blob stat info update is used against a newly checked out symlink. Hoist the file length stat update immediately after writing the file, before a rename. This eliminates any race caused by another process updating the file length after the rename and having it to fall into the racily clean path. Change-Id: I978ad9719c018ce1cf26947efbabaa8b9dff2217 --- .../jgit/dircache/DirCacheCheckout.java | 64 +++++++++---------- 1 file changed, 31 insertions(+), 33 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java index b1c75f263..1693cec51 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java @@ -1233,42 +1233,40 @@ public static void checkoutEntry(Repository repo, DirCacheEntry entry, fs.createSymLink(f, target); entry.setLength(bytes.length); entry.setLastModified(fs.lastModified(f)); - } else { - File tmpFile = File.createTempFile( - "._" + f.getName(), null, parentDir); //$NON-NLS-1$ - FileOutputStream rawChannel = new FileOutputStream(tmpFile); - OutputStream channel; - if (opt.getAutoCRLF() == AutoCRLF.TRUE) - channel = new AutoCRLFOutputStream(rawChannel); - else - channel = rawChannel; - try { - ol.copyTo(channel); - } finally { - channel.close(); - } - if (opt.isFileMode() && fs.supportsExecute()) { - if (FileMode.EXECUTABLE_FILE.equals(entry.getRawMode())) { - if (!fs.canExecute(tmpFile)) - fs.setExecute(tmpFile, true); - } else { - if (fs.canExecute(tmpFile)) - fs.setExecute(tmpFile, false); - } - } - try { - FileUtils.rename(tmpFile, f); - } catch (IOException e) { - throw new IOException(MessageFormat.format( - JGitText.get().renameFileFailed, tmpFile.getPath(), - f.getPath())); + return; + } + + File tmpFile = File.createTempFile( + "._" + f.getName(), null, parentDir); //$NON-NLS-1$ + OutputStream channel = new FileOutputStream(tmpFile); + if (opt.getAutoCRLF() == AutoCRLF.TRUE) + channel = new AutoCRLFOutputStream(channel); + try { + ol.copyTo(channel); + } finally { + channel.close(); + } + entry.setLength(opt.getAutoCRLF() == AutoCRLF.TRUE + ? f.length() // AutoCRLF wants on-disk-size + : (int) ol.getSize()); + + if (opt.isFileMode() && fs.supportsExecute()) { + if (FileMode.EXECUTABLE_FILE.equals(entry.getRawMode())) { + if (!fs.canExecute(tmpFile)) + fs.setExecute(tmpFile, true); + } else { + if (fs.canExecute(tmpFile)) + fs.setExecute(tmpFile, false); } } + try { + FileUtils.rename(tmpFile, f); + } catch (IOException e) { + throw new IOException(MessageFormat.format( + JGitText.get().renameFileFailed, tmpFile.getPath(), + f.getPath())); + } entry.setLastModified(f.lastModified()); - if (opt.getAutoCRLF() != AutoCRLF.FALSE) - entry.setLength(f.length()); // AutoCRLF wants on-disk-size - else - entry.setLength((int) ol.getSize()); } private static void checkValidPath(CanonicalTreeParser t)