From 7796b32023a04032ab7cd47bfb46e4d3d7eddfbb Mon Sep 17 00:00:00 2001 From: Baptiste Mathus Date: Mon, 14 Jan 2019 16:42:08 +0100 Subject: [PATCH] Revert "[JENKINS-55016] Fix flakiness in UtilTest" --- core/src/main/java/hudson/Util.java | 277 +++++++++++++-- .../jenkins/util/io/CompositeIOException.java | 74 ---- .../java/jenkins/util/io/PathRemover.java | 285 --------------- .../RemoveWindowsDirectoryJunctionTest.java | 3 - core/src/test/java/hudson/UtilTest.java | 309 ++++++++++++++++- .../java/jenkins/util/io/FileLockerRule.java | 80 ----- .../java/jenkins/util/io/PathRemoverTest.java | 328 ------------------ 7 files changed, 557 insertions(+), 799 deletions(-) delete mode 100644 core/src/main/java/jenkins/util/io/CompositeIOException.java delete mode 100644 core/src/main/java/jenkins/util/io/PathRemover.java delete mode 100644 core/src/test/java/jenkins/util/io/FileLockerRule.java delete mode 100644 core/src/test/java/jenkins/util/io/PathRemoverTest.java diff --git a/core/src/main/java/hudson/Util.java b/core/src/main/java/hudson/Util.java index 5c258d9469..6c075f66f4 100644 --- a/core/src/main/java/hudson/Util.java +++ b/core/src/main/java/hudson/Util.java @@ -23,13 +23,14 @@ */ package hudson; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + import hudson.model.TaskListener; import jenkins.util.MemoryReductionUtil; import hudson.util.QuotedStringTokenizer; import hudson.util.VariableResolver; import jenkins.util.SystemProperties; -import jenkins.util.io.PathRemover; import org.apache.commons.codec.digest.DigestUtils; import org.apache.commons.io.IOUtils; import org.apache.commons.io.output.NullOutputStream; @@ -60,9 +61,14 @@ import java.nio.file.FileSystemException; import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.InvalidPathException; +import java.nio.file.LinkOption; +import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.attribute.PosixFilePermission; +import java.nio.file.attribute.BasicFileAttributes; +import java.nio.file.attribute.PosixFileAttributes; +import java.nio.file.attribute.DosFileAttributes; import java.nio.file.attribute.PosixFilePermissions; import java.security.DigestInputStream; import java.security.MessageDigest; @@ -241,7 +247,16 @@ public class Util { * if the operation fails. */ public static void deleteContentsRecursive(@Nonnull File file) throws IOException { - newPathRemover().forceRemoveDirectoryContents(fileToPath(file)); + for( int numberOfAttempts=1 ; ; numberOfAttempts++ ) { + try { + tryOnceDeleteContentsRecursive(file); + break; // success + } catch (IOException ex) { + boolean threadWasInterrupted = pauseBetweenDeletes(numberOfAttempts); + if( numberOfAttempts>= DELETION_MAX || threadWasInterrupted) + throw new IOException(deleteFailExceptionMessage(file, numberOfAttempts, threadWasInterrupted), ex); + } + } } /** @@ -252,7 +267,88 @@ public class Util { * @throws IOException if it exists but could not be successfully deleted */ public static void deleteFile(@Nonnull File f) throws IOException { - newPathRemover().forceRemoveFile(fileToPath(f)); + for( int numberOfAttempts=1 ; ; numberOfAttempts++ ) { + try { + tryOnceDeleteFile(f); + break; // success + } catch (IOException ex) { + boolean threadWasInterrupted = pauseBetweenDeletes(numberOfAttempts); + if( numberOfAttempts>= DELETION_MAX || threadWasInterrupted) + throw new IOException(deleteFailExceptionMessage(f, numberOfAttempts, threadWasInterrupted), ex); + } + } + } + + /** + * Deletes this file, working around most problems which might make + * this difficult. + * + * @param f + * What to delete. If a directory, it'll need to be empty. + * @throws IOException if it exists but could not be successfully deleted, + * or if it represents an invalid {@link Path}. + */ + private static void tryOnceDeleteFile(File f) throws IOException { + Path path = fileToPath(f); + try { + Files.deleteIfExists(path); + } catch (IOException e) { + // perhaps this file is read-only? + makeWritable(path); + /* + on Unix both the file and the directory that contains it has to be writable + for a file deletion to be successful. (Confirmed on Solaris 9) + + $ ls -la + total 6 + dr-xr-sr-x 2 hudson hudson 512 Apr 18 14:41 . + dr-xr-sr-x 3 hudson hudson 512 Apr 17 19:36 .. + -r--r--r-- 1 hudson hudson 469 Apr 17 19:36 manager.xml + -rw-r--r-- 1 hudson hudson 0 Apr 18 14:41 x + $ rm x + rm: x not removed: Permission denied + */ + Path parent = path.getParent(); + if (parent != null) { + makeWritable(parent); + } + try { + Files.deleteIfExists(path); + } catch (IOException e2) { + // see https://java.net/projects/hudson/lists/users/archive/2008-05/message/357 + // I suspect other processes putting files in this directory + File[] files = f.listFiles(); + if(files!=null && files.length>0) + throw new IOException("Unable to delete " + f.getPath()+" - files in dir: "+Arrays.asList(files), e2); + throw e2; + } + } + } + + /** + * Makes the file at the given path writable by any means possible. + */ + private static void makeWritable(@Nonnull Path path) throws IOException { + if (!Functions.isWindows()) { + try { + PosixFileAttributes attrs = Files.readAttributes(path, PosixFileAttributes.class); + Set newPermissions = attrs.permissions(); + newPermissions.add(PosixFilePermission.OWNER_WRITE); + Files.setPosixFilePermissions(path, newPermissions); + return; + } catch (NoSuchFileException e) { + return; + } catch (UnsupportedOperationException e) { + // PosixFileAttributes not supported, fall back to old IO. + } + } + + /** + * We intentionally do not check the return code of setWritable, because if it + * is false we prefer to rethrow the exception thrown by Files.deleteIfExists, + * which will have a more useful message than something we make up here. + */ + path.toFile().setWritable(true); } /** @@ -264,7 +360,137 @@ public class Util { * if the operation fails. */ public static void deleteRecursive(@Nonnull File dir) throws IOException { - newPathRemover().forceRemoveRecursive(fileToPath(dir)); + for( int numberOfAttempts=1 ; ; numberOfAttempts++ ) { + try { + tryOnceDeleteRecursive(dir); + break; // success + } catch (IOException ex) { + boolean threadWasInterrupted = pauseBetweenDeletes(numberOfAttempts); + if( numberOfAttempts>= DELETION_MAX || threadWasInterrupted) + throw new IOException(deleteFailExceptionMessage(dir, numberOfAttempts, threadWasInterrupted), ex); + } + } + } + + /** + * Deletes a file or folder, throwing the first exception encountered, but + * having a go at deleting everything. i.e. it does not stop on the + * first exception, but tries (to delete) everything once. + * + * @param dir + * What to delete. If a directory, the contents will be deleted + * too. + * @throws The first exception encountered. + */ + private static void tryOnceDeleteRecursive(File dir) throws IOException { + if(!isSymlink(dir)) + tryOnceDeleteContentsRecursive(dir); + tryOnceDeleteFile(dir); + } + + /** + * Deletes a folder's contents, throwing the first exception encountered, + * but having a go at deleting everything. i.e. it does not stop + * on the first exception, but tries (to delete) everything once. + * + * @param directory + * The directory whose contents will be deleted. + * @throws The first exception encountered. + */ + private static void tryOnceDeleteContentsRecursive(File directory) throws IOException { + File[] directoryContents = directory.listFiles(); + if(directoryContents==null) + return; // the directory didn't exist in the first place + IOException firstCaught = null; + for (File child : directoryContents) { + try { + tryOnceDeleteRecursive(child); + } catch (IOException justCaught) { + if( firstCaught==null) { + firstCaught = justCaught; + } + } + } + if( firstCaught!=null ) + throw firstCaught; + } + + /** + * Pauses between delete attempts, and says if it's ok to try again. + * This does not wait if the wait time is zero or if we have tried + * too many times already. + *

+ * See {@link #WAIT_BETWEEN_DELETION_RETRIES} for details of + * the pause duration.
+ * See {@link #GC_AFTER_FAILED_DELETE} for when {@link System#gc()} is called. + * + * @return false if it is ok to continue trying to delete things, true if + * we were interrupted (and should stop now). + */ + @SuppressFBWarnings(value = "DM_GC", justification = "Garbage collection happens only when " + + "GC_AFTER_FAILED_DELETE is true. It's an experimental feature in Jenkins.") + private static boolean pauseBetweenDeletes(int numberOfAttemptsSoFar) { + long delayInMs; + if( numberOfAttemptsSoFar>=DELETION_MAX ) return false; + /* If the Jenkins process had the file open earlier, and it has not + * closed it then Windows won't let us delete it until the Java object + * with the open stream is Garbage Collected, which can result in builds + * failing due to "file in use" on Windows despite working perfectly + * well on other OSs. */ + if (GC_AFTER_FAILED_DELETE) { + System.gc(); + } + if (WAIT_BETWEEN_DELETION_RETRIES>=0) { + delayInMs = WAIT_BETWEEN_DELETION_RETRIES; + } else { + delayInMs = -numberOfAttemptsSoFar*WAIT_BETWEEN_DELETION_RETRIES; + } + if (delayInMs<=0) + return Thread.interrupted(); + try { + Thread.sleep(delayInMs); + return false; + } catch (InterruptedException e) { + return true; + } + } + + /** + * Creates a "couldn't delete file" message that explains how hard we tried. + * See {@link #DELETION_MAX}, {@link #WAIT_BETWEEN_DELETION_RETRIES} + * and {@link #GC_AFTER_FAILED_DELETE} for more details. + */ + private static String deleteFailExceptionMessage(File whatWeWereTryingToRemove, int retryCount, boolean wasInterrupted) { + StringBuilder sb = new StringBuilder(); + sb.append("Unable to delete '"); + sb.append(whatWeWereTryingToRemove); + sb.append("'. Tried "); + sb.append(retryCount); + sb.append(" time"); + if( retryCount!=1 ) sb.append('s'); + if( DELETION_MAX>1 ) { + sb.append(" (of a maximum of "); + sb.append(DELETION_MAX); + sb.append(')'); + if( GC_AFTER_FAILED_DELETE ) + sb.append(" garbage-collecting"); + if( WAIT_BETWEEN_DELETION_RETRIES!=0 && GC_AFTER_FAILED_DELETE ) + sb.append(" and"); + if( WAIT_BETWEEN_DELETION_RETRIES!=0 ) { + sb.append(" waiting "); + sb.append(getTimeSpanString(Math.abs(WAIT_BETWEEN_DELETION_RETRIES))); + if( WAIT_BETWEEN_DELETION_RETRIES<0 ) { + sb.append("-"); + sb.append(getTimeSpanString(Math.abs(WAIT_BETWEEN_DELETION_RETRIES)*DELETION_MAX)); + } + } + if( WAIT_BETWEEN_DELETION_RETRIES!=0 || GC_AFTER_FAILED_DELETE) + sb.append(" between attempts"); + } + if( wasInterrupted ) + sb.append(". The delete operation was interrupted before it completed successfully"); + sb.append('.'); + return sb.toString(); } /* @@ -283,25 +509,36 @@ public class Util { * limitations under the License. */ /** - * Checks if the given file represents a symlink. Unlike {@link Files#isSymbolicLink(Path)}, this method also - * considers NTFS junction points as symbolic - * links. + * Checks if the given file represents a symlink. */ public static boolean isSymlink(@Nonnull File file) throws IOException { - return isSymlink(fileToPath(file)); - } - - @Restricted(NoExternalUse.class) - public static boolean isSymlink(@Nonnull Path path) { - if (Files.isSymbolicLink(path)) return true; /* - In Windows, a directory junction is not considered a symbolic link despite being nearly the same feature. - To avoid relying directly on Windows-specific filesystem API calls, we can instead determine if a path is - logically a symbolic link by comparing its absolute path to its real path. + * Windows Directory Junctions are effectively the same as Linux symlinks to directories. + * Unfortunately, the Java 7 NIO2 API function isSymbolicLink does not treat them as such. + * It thinks of them as normal directories. To use the NIO2 API & treat it like a symlink, + * you have to go through BasicFileAttributes and do the following check: + * isSymbolicLink() || isOther() + * The isOther() call will include Windows reparse points, of which a directory junction is. + * It also includes includes devices, but reading the attributes of a device with NIO fails + * or returns false for isOther(). (i.e. named pipes such as \\.\pipe\JenkinsTestPipe return + * false for isOther(), and drives such as \\.\PhysicalDrive0 throw an exception when + * calling readAttributes. */ try { - return path.toAbsolutePath().compareTo(path.toRealPath()) != 0; - } catch (IOException ignored) { + Path path = fileToPath(file); + BasicFileAttributes attrs = Files.readAttributes(path, BasicFileAttributes.class, LinkOption.NOFOLLOW_LINKS); + if (attrs.isSymbolicLink()) { + return true; + } else if (attrs instanceof DosFileAttributes) { + /* Returns true for non-symbolic link reparse points and devices. We could call + * WindowsFileAttributes#isReparsePoint with reflection instead to exclude devices, + * but as mentioned in the above comment this does not appear to be an issue. + */ + return attrs.isOther(); + } else { + return false; + } + } catch (NoSuchFileException e) { return false; } } @@ -1560,10 +1797,6 @@ public class Util { @Restricted(value = NoExternalUse.class) static boolean GC_AFTER_FAILED_DELETE = SystemProperties.getBoolean(Util.class.getName() + ".performGCOnFailedDelete"); - private static PathRemover newPathRemover() { - return PathRemover.newRobustRemover(DELETION_MAX - 1, GC_AFTER_FAILED_DELETE, WAIT_BETWEEN_DELETION_RETRIES); - } - /** * If this flag is true, native implementations of {@link FilePath#chmod} * and {@link hudson.util.IOUtils#mode} are used instead of NIO. diff --git a/core/src/main/java/jenkins/util/io/CompositeIOException.java b/core/src/main/java/jenkins/util/io/CompositeIOException.java deleted file mode 100644 index dfd347cbd6..0000000000 --- a/core/src/main/java/jenkins/util/io/CompositeIOException.java +++ /dev/null @@ -1,74 +0,0 @@ -/* - * The MIT License - * - * Copyright (c) 2018 CloudBees, Inc. - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN - * THE SOFTWARE. - */ - -package jenkins.util.io; - -import org.kohsuke.accmod.Restricted; -import org.kohsuke.accmod.restrictions.NoExternalUse; - -import javax.annotation.Nonnull; -import java.io.IOException; -import java.io.PrintStream; -import java.io.PrintWriter; -import java.io.UncheckedIOException; -import java.util.Arrays; -import java.util.List; - -@Restricted(NoExternalUse.class) -public class CompositeIOException extends IOException { - private final List exceptions; - - public CompositeIOException(String message, @Nonnull List exceptions) { - super(message); - this.exceptions = exceptions; - } - - public CompositeIOException(String message, IOException... exceptions) { - this(message, Arrays.asList(exceptions)); - } - - public List getExceptions() { - return exceptions; - } - - @Override - public void printStackTrace(PrintStream s) { - super.printStackTrace(s); - for (IOException exception : exceptions) { - exception.printStackTrace(s); - } - } - - @Override - public void printStackTrace(PrintWriter s) { - super.printStackTrace(s); - for (IOException exception : exceptions) { - exception.printStackTrace(s); - } - } - - public UncheckedIOException asUncheckedIOException() { - return new UncheckedIOException(this); - } -} diff --git a/core/src/main/java/jenkins/util/io/PathRemover.java b/core/src/main/java/jenkins/util/io/PathRemover.java deleted file mode 100644 index 763f8f8589..0000000000 --- a/core/src/main/java/jenkins/util/io/PathRemover.java +++ /dev/null @@ -1,285 +0,0 @@ -/* - * The MIT License - * - * Copyright (c) 2018 CloudBees, Inc. - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN - * THE SOFTWARE. - */ - -package jenkins.util.io; - -import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; -import hudson.Functions; -import hudson.Util; -import org.kohsuke.accmod.Restricted; -import org.kohsuke.accmod.restrictions.NoExternalUse; - -import javax.annotation.Nonnull; -import java.io.IOException; -import java.nio.file.DirectoryStream; -import java.nio.file.Files; -import java.nio.file.NoSuchFileException; -import java.nio.file.Path; -import java.nio.file.attribute.PosixFileAttributes; -import java.nio.file.attribute.PosixFilePermission; -import java.util.ArrayList; -import java.util.List; -import java.util.Optional; -import java.util.Set; -import java.util.stream.Collectors; -import java.util.stream.Stream; - -@Restricted(NoExternalUse.class) -public class PathRemover { - - public static PathRemover newSimpleRemover() { - return new PathRemover(ignored -> false); - } - - public static PathRemover newRemoverWithStrategy(@Nonnull RetryStrategy retryStrategy) { - return new PathRemover(retryStrategy); - } - - public static PathRemover newRobustRemover(int maxRetries, boolean gcAfterFailedRemove, long waitBetweenRetries) { - return new PathRemover(new PausingGCRetryStrategy(maxRetries < 1 ? 1 : maxRetries, gcAfterFailedRemove, waitBetweenRetries)); - } - - private final RetryStrategy retryStrategy; - - private PathRemover(@Nonnull RetryStrategy retryStrategy) { - this.retryStrategy = retryStrategy; - } - - public void forceRemoveFile(@Nonnull Path path) throws IOException { - for (int retryAttempts = 0; ; retryAttempts++) { - Optional maybeError = tryRemoveFile(path); - if (!maybeError.isPresent()) return; - if (retryStrategy.shouldRetry(retryAttempts)) continue; - IOException error = maybeError.get(); - throw new IOException(retryStrategy.failureMessage(path, retryAttempts), error); - } - } - - public void forceRemoveDirectoryContents(@Nonnull Path path) throws IOException { - for (int retryAttempt = 0; ; retryAttempt++) { - List errors = tryRemoveDirectoryContents(path); - if (errors.isEmpty()) return; - if (retryStrategy.shouldRetry(retryAttempt)) continue; - throw new CompositeIOException(retryStrategy.failureMessage(path, retryAttempt), errors); - } - } - - public void forceRemoveRecursive(@Nonnull Path path) throws IOException { - for (int retryAttempt = 0; ; retryAttempt++) { - List errors = tryRemoveRecursive(path); - if (errors.isEmpty()) return; - if (retryStrategy.shouldRetry(retryAttempt)) continue; - throw new CompositeIOException(retryStrategy.failureMessage(path, retryAttempt), errors); - } - } - - @Restricted(NoExternalUse.class) - @FunctionalInterface - public interface RetryStrategy { - boolean shouldRetry(int retriesAttempted); - - default String failureMessage(@Nonnull Path fileToRemove, int retryCount) { - StringBuilder sb = new StringBuilder() - .append("Unable to delete '") - .append(fileToRemove) - .append("'. Tried ") - .append(retryCount) - .append(" time"); - if (retryCount != 1) sb.append('s'); - sb.append('.'); - return sb.toString(); - } - } - - private static class PausingGCRetryStrategy implements RetryStrategy { - private final int maxRetries; - private final boolean gcAfterFailedRemove; - private final long waitBetweenRetries; - private final ThreadLocal interrupted = ThreadLocal.withInitial(() -> false); - - private PausingGCRetryStrategy(int maxRetries, boolean gcAfterFailedRemove, long waitBetweenRetries) { - this.maxRetries = maxRetries; - this.gcAfterFailedRemove = gcAfterFailedRemove; - this.waitBetweenRetries = waitBetweenRetries; - } - - @SuppressFBWarnings(value = "DM_GC", justification = "Garbage collection happens only when " - + "GC_AFTER_FAILED_DELETE is true. It's an experimental feature in Jenkins.") - private void gcIfEnabled() { - /* If the Jenkins process had the file open earlier, and it has not - * closed it then Windows won't let us delete it until the Java object - * with the open stream is Garbage Collected, which can result in builds - * failing due to "file in use" on Windows despite working perfectly - * well on other OSs. */ - if (gcAfterFailedRemove) System.gc(); - } - - @Override - public boolean shouldRetry(int retriesAttempted) { - if (retriesAttempted >= maxRetries) return false; - gcIfEnabled(); - long delayMillis = waitBetweenRetries >= 0 ? waitBetweenRetries : -(retriesAttempted + 1) * waitBetweenRetries; - if (delayMillis <= 0) return !Thread.interrupted(); - try { - Thread.sleep(delayMillis); - return true; - } catch (InterruptedException e) { - interrupted.set(true); - return false; - } - } - - @Override - public String failureMessage(@Nonnull Path fileToRemove, int retryCount) { - StringBuilder sb = new StringBuilder(); - sb.append("Unable to delete '"); - sb.append(fileToRemove); - sb.append("'. Tried "); - sb.append(retryCount + 1); - sb.append(" time"); - if (retryCount != 1) sb.append('s'); - if (maxRetries > 0) { - sb.append(" (of a maximum of "); - sb.append(maxRetries + 1); - sb.append(')'); - if (gcAfterFailedRemove) - sb.append(" garbage-collecting"); - if (waitBetweenRetries != 0 && gcAfterFailedRemove) - sb.append(" and"); - if (waitBetweenRetries != 0) { - sb.append(" waiting "); - sb.append(Util.getTimeSpanString(Math.abs(waitBetweenRetries))); - if (waitBetweenRetries < 0) { - sb.append("-"); - sb.append(Util.getTimeSpanString(Math.abs(waitBetweenRetries) * (maxRetries + 1))); - } - } - if (waitBetweenRetries != 0 || gcAfterFailedRemove) - sb.append(" between attempts"); - } - if (interrupted.get()) - sb.append(". The delete operation was interrupted before it completed successfully"); - sb.append('.'); - interrupted.set(false); - return sb.toString(); - } - } - - private static Optional tryRemoveFile(@Nonnull Path path) { - try { - removeOrMakeRemovableThenRemove(path); - return Optional.empty(); - } catch (IOException e) { - return Optional.of(e); - } - } - - private static List tryRemoveRecursive(@Nonnull Path path) { - List accumulatedErrors = Util.isSymlink(path) ? new ArrayList<>() : - tryRemoveDirectoryContents(path); - tryRemoveFile(path).ifPresent(accumulatedErrors::add); - return accumulatedErrors; - } - - private static List tryRemoveDirectoryContents(@Nonnull Path path) { - List accumulatedErrors = new ArrayList<>(); - if (!Files.isDirectory(path)) return accumulatedErrors; - try (DirectoryStream children = Files.newDirectoryStream(path)) { - for (Path child : children) { - accumulatedErrors.addAll(tryRemoveRecursive(child)); - } - } catch (IOException e) { - accumulatedErrors.add(e); - } - return accumulatedErrors; - } - - private static void removeOrMakeRemovableThenRemove(@Nonnull Path path) throws IOException { - try { - Files.deleteIfExists(path); - } catch (IOException e) { - makeRemovable(path); - try { - Files.deleteIfExists(path); - } catch (IOException e2) { - // see https://java.net/projects/hudson/lists/users/archive/2008-05/message/357 - // I suspect other processes putting files in this directory - if (Files.isDirectory(path)) { - List entries; - try (Stream children = Files.list(path)) { - entries = children.map(Path::toString).collect(Collectors.toList()); - } - throw new CompositeIOException("Unable to remove directory " + path + " with directory contents: " + entries, e, e2); - } - throw new CompositeIOException("Unable to remove file " + path, e, e2); - } - } - } - - private static void makeRemovable(@Nonnull Path path) throws IOException { - if (!Files.isWritable(path)) { - makeWritable(path); - } - /* - on Unix both the file and the directory that contains it has to be writable - for a file deletion to be successful. (Confirmed on Solaris 9) - - $ ls -la - total 6 - dr-xr-sr-x 2 hudson hudson 512 Apr 18 14:41 . - dr-xr-sr-x 3 hudson hudson 512 Apr 17 19:36 .. - -r--r--r-- 1 hudson hudson 469 Apr 17 19:36 manager.xml - -rw-r--r-- 1 hudson hudson 0 Apr 18 14:41 x - $ rm x - rm: x not removed: Permission denied - */ - Path parent = path.getParent(); - if (parent != null && !Files.isWritable(parent)) { - makeWritable(parent); - } - } - - private static void makeWritable(@Nonnull Path path) throws IOException { - if (!Functions.isWindows()) { - try { - PosixFileAttributes attrs = Files.readAttributes(path, PosixFileAttributes.class); - Set newPermissions = attrs.permissions(); - newPermissions.add(PosixFilePermission.OWNER_WRITE); - Files.setPosixFilePermissions(path, newPermissions); - } catch (NoSuchFileException ignored) { - return; - } catch (UnsupportedOperationException ignored) { - // PosixFileAttributes not supported, fall back to old IO. - } - } - - /* - * We intentionally do not check the return code of setWritable, because if it - * is false we prefer to rethrow the exception thrown by Files.deleteIfExists, - * which will have a more useful message than something we make up here. - */ - path.toFile().setWritable(true); - } - -} diff --git a/core/src/test/java/hudson/RemoveWindowsDirectoryJunctionTest.java b/core/src/test/java/hudson/RemoveWindowsDirectoryJunctionTest.java index a48c9d833c..cd631503c3 100644 --- a/core/src/test/java/hudson/RemoveWindowsDirectoryJunctionTest.java +++ b/core/src/test/java/hudson/RemoveWindowsDirectoryJunctionTest.java @@ -13,11 +13,8 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; -import org.jvnet.hudson.test.For; import org.jvnet.hudson.test.Issue; -@For(Util.class) -// https://superuser.com/q/343074 public class RemoveWindowsDirectoryJunctionTest { @Rule public TemporaryFolder tmp = new TemporaryFolder(); diff --git a/core/src/test/java/hudson/UtilTest.java b/core/src/test/java/hudson/UtilTest.java index 36586408e5..1002ac70a3 100644 --- a/core/src/test/java/hudson/UtilTest.java +++ b/core/src/test/java/hudson/UtilTest.java @@ -27,25 +27,38 @@ package hudson; import java.text.ParseException; import java.text.SimpleDateFormat; import java.util.Date; +import java.util.List; import java.util.Map; import java.util.HashMap; import java.util.Locale; import java.util.Properties; +import java.util.concurrent.Callable; +import java.util.concurrent.atomic.AtomicReference; import java.io.ByteArrayOutputStream; import java.io.File; +import java.io.FileInputStream; import java.io.IOException; +import java.io.InputStream; +import java.nio.file.FileSystemException; +import java.nio.file.Files; +import java.nio.file.Path; import java.nio.file.attribute.PosixFilePermissions; +import static org.hamcrest.CoreMatchers.allOf; +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.hasItem; +import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.CoreMatchers.startsWith; import static org.junit.Assert.*; +import org.apache.commons.io.FileUtils; import org.hamcrest.BaseMatcher; import org.hamcrest.Description; import org.junit.Assume; +import org.junit.Ignore; import org.junit.Test; -import org.junit.rules.ExpectedException; import org.jvnet.hudson.test.Issue; import hudson.util.StreamTaskListener; @@ -53,6 +66,8 @@ import hudson.util.StreamTaskListener; import org.junit.Rule; import org.junit.rules.TemporaryFolder; +import com.google.common.collect.Lists; + /** * @author Kohsuke Kawaguchi */ @@ -60,8 +75,6 @@ public class UtilTest { @Rule public TemporaryFolder tmp = new TemporaryFolder(); - @Rule public ExpectedException expectedException = ExpectedException.none(); - @Test public void testReplaceMacro() { Map m = new HashMap(); @@ -179,7 +192,7 @@ public class UtilTest { @Test public void testSymlink() throws Exception { - Assume.assumeFalse(Functions.isWindows()); + Assume.assumeTrue(!Functions.isWindows()); ByteArrayOutputStream baos = new ByteArrayOutputStream(); StreamTaskListener l = new StreamTaskListener(baos); @@ -227,7 +240,7 @@ public class UtilTest { @Test public void testIsSymlink() throws IOException, InterruptedException { - Assume.assumeFalse(Functions.isWindows()); + Assume.assumeTrue(!Functions.isWindows()); ByteArrayOutputStream baos = new ByteArrayOutputStream(); StreamTaskListener l = new StreamTaskListener(baos); @@ -268,6 +281,284 @@ public class UtilTest { assertTrue(Util.isSymlink(new File(d, "junction"))); } + @Test + public void testDeleteFile() throws Exception { + File f = tmp.newFile(); + // Test: File is deleted + mkfiles(f); + Util.deleteFile(f); + assertFalse("f exists after calling Util.deleteFile", f.exists()); + } + + @Test + public void testDeleteFile_onWindows() throws Exception { + Assume.assumeTrue(Functions.isWindows()); + final int defaultDeletionMax = Util.DELETION_MAX; + try { + File f = tmp.newFile(); + // Test: If we cannot delete a file, we throw explaining why + mkfiles(f); + lockFileForDeletion(f); + Util.DELETION_MAX = 1; + try { + Util.deleteFile(f); + fail("should not have been deletable"); + } catch (IOException x) { + assertThat(calcExceptionHierarchy(x), hasItem(FileSystemException.class)); + assertThat(x.getMessage(), containsString(f.getPath())); + } + } finally { + Util.DELETION_MAX = defaultDeletionMax; + unlockFilesForDeletion(); + } + } + + @Test + public void testDeleteFileReadOnly() throws Exception { + // Removing the calls to Util#makeWritable in Util#tryOnceDeleteFile should cause this test to fail. + Path file = tmp.newFolder().toPath().resolve("file.tmp"); + Files.createDirectories(file.getParent()); + Files.createFile(file); + // Using old IO so the test can run on Windows. + file.getParent().toFile().setWritable(false); + file.toFile().setWritable(false); + Util.deleteFile(file.toFile()); + assertFalse(Files.exists(file)); + } + + @Test + public void testDeleteFileDoesNotExist() throws Exception { + Path file = tmp.newFolder().toPath().resolve("file.tmp"); + assertFalse(Files.exists(file)); + // Should not throw an exception. + Util.deleteFile(file.toFile()); + } + + @Test + public void testDeleteContentsRecursive() throws Exception { + final File dir = tmp.newFolder(); + final File d1 = new File(dir, "d1"); + final File d2 = new File(dir, "d2"); + final File f1 = new File(dir, "f1"); + final File d1f1 = new File(d1, "d1f1"); + final File d2f2 = new File(d2, "d1f2"); + // Test: Files and directories are deleted + mkdirs(dir, d1, d2); + mkfiles(f1, d1f1, d2f2); + Util.deleteContentsRecursive(dir); + assertTrue("dir exists", dir.exists()); + assertFalse("d1 exists", d1.exists()); + assertFalse("d2 exists", d2.exists()); + assertFalse("f1 exists", f1.exists()); + } + + @Ignore("TODO often fails in CI") + @Test + public void testDeleteContentsRecursive_onWindows() throws Exception { + Assume.assumeTrue(Functions.isWindows()); + final File dir = tmp.newFolder(); + final File d1 = new File(dir, "d1"); + final File d2 = new File(dir, "d2"); + final File f1 = new File(dir, "f1"); + final File d1f1 = new File(d1, "d1f1"); + final File d2f2 = new File(d2, "d1f2"); + final int defaultDeletionMax = Util.DELETION_MAX; + final int defaultDeletionWait = Util.WAIT_BETWEEN_DELETION_RETRIES; + final boolean defaultDeletionGC = Util.GC_AFTER_FAILED_DELETE; + try { + // Test: If we cannot delete a file, we throw + // but still deletes everything it can + // even if we are not retrying deletes. + mkdirs(dir, d1, d2); + mkfiles(f1, d1f1, d2f2); + lockFileForDeletion(d1f1); + Util.GC_AFTER_FAILED_DELETE = false; + Util.DELETION_MAX = 2; + Util.WAIT_BETWEEN_DELETION_RETRIES = 0; + try { + Util.deleteContentsRecursive(dir); + fail("Expected IOException"); + } catch (IOException x) { + assertFalse("d2 should not exist", d2.exists()); + assertFalse("f1 should not exist", f1.exists()); + assertFalse("d1f2 should not exist", d2f2.exists()); + assertThat(x.getMessage(), containsString(dir.getPath())); + assertThat(x.getMessage(), allOf(not(containsString("interrupted")), containsString("Tried 2 times (of a maximum of 2)."), not(containsString("garbage-collecting")), not(containsString("wait")))); + } + } finally { + Util.DELETION_MAX = defaultDeletionMax; + Util.WAIT_BETWEEN_DELETION_RETRIES = defaultDeletionWait; + Util.GC_AFTER_FAILED_DELETE = defaultDeletionGC; + unlockFilesForDeletion(); + } + } + + @Test + public void testDeleteRecursive() throws Exception { + final File dir = tmp.newFolder(); + final File d1 = new File(dir, "d1"); + final File d2 = new File(dir, "d2"); + final File f1 = new File(dir, "f1"); + final File d1f1 = new File(d1, "d1f1"); + final File d2f2 = new File(d2, "d1f2"); + // Test: Files and directories are deleted + mkdirs(dir, d1, d2); + mkfiles(f1, d1f1, d2f2); + Util.deleteRecursive(dir); + assertFalse("dir exists", dir.exists()); + } + + @Ignore("JENKINS-55016") + @Test + public void testDeleteRecursive_onWindows() throws Exception { + Assume.assumeTrue(Functions.isWindows()); + final File dir = tmp.newFolder(); + final File d1 = new File(dir, "d1"); + final File d2 = new File(dir, "d2"); + final File f1 = new File(dir, "f1"); + final File d1f1 = new File(d1, "d1f1"); + final File d2f2 = new File(d2, "d1f2"); + final int defaultDeletionMax = Util.DELETION_MAX; + final int defaultDeletionWait = Util.WAIT_BETWEEN_DELETION_RETRIES; + final boolean defaultDeletionGC = Util.GC_AFTER_FAILED_DELETE; + try { + // Test: If we cannot delete a file, we throw + // but still deletes everything it can + // even if we are not retrying deletes. + // (And when we are not retrying deletes, + // we do not do the "between retries" delay) + mkdirs(dir, d1, d2); + mkfiles(f1, d1f1, d2f2); + lockFileForDeletion(d1f1); + Util.GC_AFTER_FAILED_DELETE = false; + Util.DELETION_MAX = 1; + Util.WAIT_BETWEEN_DELETION_RETRIES = 10000; // long enough to notice + long timeWhenDeletionStarted = System.currentTimeMillis(); + try { + Util.deleteRecursive(dir); + fail("Expected IOException"); + } catch (IOException x) { + long timeWhenDeletionEnded = System.currentTimeMillis(); + assertTrue("dir exists", dir.exists()); + assertTrue("d1 exists", d1.exists()); + assertTrue("d1f1 exists", d1f1.exists()); + assertFalse("d2 should not exist", d2.exists()); + assertFalse("f1 should not exist", f1.exists()); + assertFalse("d1f2 should not exist", d2f2.exists()); + assertThat(x.getMessage(), containsString(dir.getPath())); + assertThat(x.getMessage(), allOf(not(containsString("interrupted")), not(containsString("maximum of")), not(containsString("garbage-collecting")))); + long actualTimeSpentDeleting = timeWhenDeletionEnded - timeWhenDeletionStarted; + assertTrue("did not wait - took " + actualTimeSpentDeleting + "ms", actualTimeSpentDeleting<1000L); + } + unlockFileForDeletion(d1f1); + // Deletes get retried if they fail 1st time around, + // allowing the operation to succeed on subsequent attempts. + // Note: This is what bug JENKINS-15331 is all about. + mkdirs(dir, d1, d2); + mkfiles(f1, d1f1, d2f2); + lockFileForDeletion(d2f2); + Util.DELETION_MAX=4; + Util.WAIT_BETWEEN_DELETION_RETRIES = 100; + Thread unlockAfterDelay = new Thread("unlockFileAfterDelay") { + public void run() { + try { + Thread.sleep(Util.WAIT_BETWEEN_DELETION_RETRIES); + unlockFileForDeletion(d2f2); + } catch( Exception x ) { /* ignored */ } + } + }; + unlockAfterDelay.start(); + Util.deleteRecursive(dir); + assertFalse("dir should have been deleted", dir.exists()); + unlockAfterDelay.join(); + // An interrupt aborts the delete and makes it fail, even + // if we had been told to retry a lot. + mkdirs(dir, d1, d2); + mkfiles(f1, d1f1, d2f2); + lockFileForDeletion(d1f1); + Util.DELETION_MAX=10; + Util.WAIT_BETWEEN_DELETION_RETRIES = -1000; + Util.GC_AFTER_FAILED_DELETE = true; + final AtomicReference thrown = new AtomicReference(); + Thread deleteToBeInterrupted = new Thread("deleteToBeInterrupted") { + public void run() { + try { Util.deleteRecursive(dir); } + catch( Throwable x ) { thrown.set(x); } + } + }; + deleteToBeInterrupted.start(); + deleteToBeInterrupted.interrupt(); + deleteToBeInterrupted.join(500); + assertFalse("deletion stopped", deleteToBeInterrupted.isAlive()); + assertTrue("d1f1 still exists", d1f1.exists()); + unlockFileForDeletion(d1f1); + Throwable deletionInterruptedEx = thrown.get(); + assertThat(deletionInterruptedEx, instanceOf(IOException.class)); + assertThat(deletionInterruptedEx.getMessage(), allOf(containsString("interrupted"), containsString("maximum of " + Util.DELETION_MAX), containsString("garbage-collecting"))); + } finally { + Util.DELETION_MAX = defaultDeletionMax; + Util.WAIT_BETWEEN_DELETION_RETRIES = defaultDeletionWait; + Util.GC_AFTER_FAILED_DELETE = defaultDeletionGC; + unlockFilesForDeletion(); + } + } + + /** Creates multiple directories. */ + private static void mkdirs(File... dirs) { + for( File d : dirs ) { + d.mkdir(); + assertTrue(d.getPath(), d.isDirectory()); + } + } + + /** Creates multiple files, each containing their filename as text content. */ + private static void mkfiles(File... files) throws IOException { + for( File f : files ) + FileUtils.write(f, f.getName()); + } + + /** Means of unlocking all the files we have locked, indexed by {@link File}. */ + private final Map> unlockFileCallables = new HashMap>(); + + /** Prevents a file from being deleted, so we can stress the deletion code's retries. */ + private void lockFileForDeletion(File f) throws IOException, InterruptedException { + assert !unlockFileCallables.containsKey(f) : f + " is already locked." ; + // Limitation: Only works on Windows. On unix we can delete anything we can create. + // On unix, can't use "chmod a-w" on the dir as the code-under-test undoes that. + // On unix, can't use "chattr +i" because that needs root. + // On unix, can't use "chattr +u" because ext fs ignores it. + // On Windows, can't use FileChannel.lock() because that doesn't block deletion + // On Windows, we can't delete files that are open for reading, so we use that. + // NOTE: This is a hack in any case as there is no guarantee that all Windows filesystems + // will enforce blocking deletion on open files... just that the ones we normally + // test with seem to block. + assert Functions.isWindows(); + final InputStream s = new FileInputStream(f); // intentional use of FileInputStream + unlockFileCallables.put(f, new Callable() { + public Void call() throws IOException { s.close(); return null; }; + }); + } + + /** Undoes a call to {@link #lockFileForDeletion(File)}. */ + private void unlockFileForDeletion(File f) throws Exception { + unlockFileCallables.remove(f).call(); + } + + /** Undoes all calls to {@link #lockFileForDeletion(File)}. */ + private void unlockFilesForDeletion() throws Exception { + while( !unlockFileCallables.isEmpty() ) { + unlockFileForDeletion(unlockFileCallables.keySet().iterator().next()); + } + } + + /** Returns all classes in the exception hierarchy. */ + private static Iterable> calcExceptionHierarchy(Throwable t) { + final List> result = Lists.newArrayList(); + for( ; t!=null ; t = t.getCause()) + result.add(t.getClass()); + return result; + } + @Test public void testHtmlEscape() { assertEquals("
", Util.escape("\n")); @@ -473,8 +764,12 @@ public class UtilTest { assertEquals("Non-permission bits should be ignored", PosixFilePermissions.fromString("r-xr-----"), Util.modeToPermissions(0100540)); - expectedException.expectMessage(startsWith("Invalid mode")); - Util.modeToPermissions(01777); + try { + Util.modeToPermissions(01777); + fail("Did not detect invalid mode"); + } catch (IOException e) { + assertThat(e.getMessage(), startsWith("Invalid mode")); + } } @Test diff --git a/core/src/test/java/jenkins/util/io/FileLockerRule.java b/core/src/test/java/jenkins/util/io/FileLockerRule.java deleted file mode 100644 index df0dab652e..0000000000 --- a/core/src/test/java/jenkins/util/io/FileLockerRule.java +++ /dev/null @@ -1,80 +0,0 @@ -/* - * The MIT License - * - * Copyright (c) 2018 CloudBees, Inc. - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN - * THE SOFTWARE. - */ - -package jenkins.util.io; - -import hudson.Functions; -import org.junit.rules.ExternalResource; - -import javax.annotation.Nonnull; -import java.io.Closeable; -import java.io.File; -import java.io.FileInputStream; -import java.io.IOException; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.Iterator; -import java.util.List; -import java.util.Map; - -import static org.hamcrest.Matchers.hasKey; -import static org.hamcrest.Matchers.not; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.assertTrue; - -/** - * Helper class for tracking which files are locked. - * Only useful in Windows environments as POSIX seems to allow locked files to be deleted. - */ -public class FileLockerRule extends ExternalResource { - private final Map locks = new HashMap<>(); - - @Override - protected void after() { - List exceptions = new ArrayList<>(); - Iterator it = locks.values().iterator(); - while (it.hasNext()) { - try (Closeable ignored = it.next()) { - it.remove(); - } catch (IOException e) { - exceptions.add(e); - } - } - if (!exceptions.isEmpty()) - throw new CompositeIOException("Could not unlock all files", exceptions).asUncheckedIOException(); - } - - public synchronized void acquireLock(@Nonnull File file) throws IOException { - assertTrue(Functions.isWindows()); - assertThat(file + " is already locked.", locks, not(hasKey(file))); - Closeable lock = new FileInputStream(file); - locks.put(file, lock); - } - - public synchronized void releaseLock(@Nonnull File file) throws Exception { - assertTrue(Functions.isWindows()); - assertThat(file + " is not locked.", locks, hasKey(file)); - locks.remove(file).close(); - } -} diff --git a/core/src/test/java/jenkins/util/io/PathRemoverTest.java b/core/src/test/java/jenkins/util/io/PathRemoverTest.java deleted file mode 100644 index 55c7970424..0000000000 --- a/core/src/test/java/jenkins/util/io/PathRemoverTest.java +++ /dev/null @@ -1,328 +0,0 @@ -/* - * The MIT License - * - * Copyright (c) 2018 CloudBees, Inc. - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN - * THE SOFTWARE. - */ - -package jenkins.util.io; - -import hudson.Functions; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.ExpectedException; -import org.junit.rules.TemporaryFolder; -import org.junit.rules.Timeout; - -import java.io.File; -import java.io.FileWriter; -import java.io.IOException; -import java.nio.file.FileSystem; -import java.nio.file.FileSystemException; -import java.nio.file.Path; -import java.nio.file.attribute.BasicFileAttributes; -import java.nio.file.spi.FileSystemProvider; -import java.util.ArrayList; -import java.util.List; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicReference; - -import static org.hamcrest.Matchers.allOf; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.hasItem; -import static org.junit.Assert.*; -import static org.junit.Assume.assumeTrue; -import static org.mockito.BDDMockito.given; -import static org.mockito.Mockito.mock; - -public class PathRemoverTest { - - @Rule public TemporaryFolder tmp = new TemporaryFolder(); - @Rule public ExpectedException expectedException = ExpectedException.none(); - @Rule public Timeout timeout = new Timeout(10, TimeUnit.SECONDS); - @Rule public FileLockerRule locker = new FileLockerRule(); - - @Test - public void testForceRemoveFile() throws IOException { - File file = tmp.newFile(); - touchWithFileName(file); - - PathRemover remover = PathRemover.newSimpleRemover(); - remover.forceRemoveFile(file.toPath()); - - assertFalse("Unable to delete file: " + file, file.exists()); - } - - @Test - public void testForceRemoveFile_LockedFile() throws Exception { - String filename = "/var/lock/jenkins.lock"; - File file = mock(File.class); - Path path = mock(Path.class); - FileSystem fs = mock(FileSystem.class); - FileSystemProvider fsProvider = mock(FileSystemProvider.class); - BasicFileAttributes attributes = mock(BasicFileAttributes.class); - - given(file.getPath()).willReturn(filename); - given(file.toPath()).willReturn(path); - given(path.toString()).willReturn(filename); - given(path.toFile()).willReturn(file); - given(path.getFileSystem()).willReturn(fs); - given(fs.provider()).willReturn(fsProvider); - given(fsProvider.deleteIfExists(path)).willThrow(new FileSystemException(filename)); - given(fsProvider.readAttributes(path, BasicFileAttributes.class)).willReturn(attributes); - given(attributes.isDirectory()).willReturn(false); - - PathRemover remover = PathRemover.newSimpleRemover(); - try { - remover.forceRemoveFile(file.toPath()); - fail("Should not have been deleted: " + file); - } catch (IOException e) { - assertThat(calcExceptionHierarchy(e), hasItem(FileSystemException.class)); - assertThat(e.getMessage(), containsString(filename)); - } - - } - - private static List> calcExceptionHierarchy(Throwable t) { - List> hierarchy = new ArrayList<>(); - for (; t != null; t = t.getCause()) { - hierarchy.add(t.getClass()); - // with a composite exception, we might be hiding some other causes - if (t instanceof CompositeIOException) { - CompositeIOException e = (CompositeIOException) t; - e.getExceptions().forEach(ex -> hierarchy.addAll(calcExceptionHierarchy(ex))); - } - } - return hierarchy; - } - - @Test - public void testForceRemoveFile_ReadOnly() throws IOException { - File dir = tmp.newFolder(); - File file = new File(dir, "file.tmp"); - touchWithFileName(file); - assertTrue("Unable to make file read-only: " + file, file.setWritable(false)); - - PathRemover remover = PathRemover.newSimpleRemover(); - remover.forceRemoveFile(file.toPath()); - - assertFalse("Unable to delete file: " + file, file.exists()); - } - - @Test - public void testForceRemoveFile_DoesNotExist() throws IOException { - File dir = tmp.newFolder(); - File file = new File(dir, "invalid.file"); - assertFalse(file.exists()); - - PathRemover remover = PathRemover.newSimpleRemover(); - remover.forceRemoveFile(file.toPath()); - - assertFalse("Unable to delete file: " + file, file.exists()); - } - - @Test - public void testForceRemoveDirectoryContents() throws IOException { - File dir = tmp.newFolder(); - File d1 = new File(dir, "d1"); - File d2 = new File(dir, "d2"); - File f1 = new File(dir, "f1"); - File d1f1 = new File(d1, "d1f1"); - File d2f2 = new File(d2, "d1f2"); - mkdirs(d1, d2); - touchWithFileName(f1, d1f1, d2f2); - - PathRemover remover = PathRemover.newSimpleRemover(); - remover.forceRemoveDirectoryContents(dir.toPath()); - - assertTrue(dir.exists()); - assertFalse(d1.exists()); - assertFalse(d2.exists()); - assertFalse(f1.exists()); - } - - @Test - public void testForceRemoveDirectoryContents_LockedFile() throws Exception { - assumeTrue(Functions.isWindows()); - File dir = tmp.newFolder(); - File d1 = new File(dir, "d1"); - File d2 = new File(dir, "d2"); - File f1 = new File(dir, "f1"); - File d1f1 = new File(d1, "d1f1"); - File d2f2 = new File(d2, "d1f2"); - mkdirs(d1, d2); - touchWithFileName(f1, d1f1, d2f2); - locker.acquireLock(d1f1); - PathRemover remover = PathRemover.newRemoverWithStrategy(retriesAttempted -> retriesAttempted < 1); - expectedException.expectMessage(allOf( - containsString(dir.getPath()), - containsString("Tried 1 time.") - )); - remover.forceRemoveDirectoryContents(dir.toPath()); - assertFalse(d2.exists()); - assertFalse(f1.exists()); - assertFalse(d2f2.exists()); - } - - @Test - public void testForceRemoveRecursive() throws IOException { - File dir = tmp.newFolder(); - File d1 = new File(dir, "d1"); - File d2 = new File(dir, "d2"); - File f1 = new File(dir, "f1"); - File d1f1 = new File(d1, "d1f1"); - File d2f2 = new File(d2, "d1f2"); - mkdirs(d1, d2); - touchWithFileName(f1, d1f1, d2f2); - - PathRemover remover = PathRemover.newSimpleRemover(); - remover.forceRemoveRecursive(dir.toPath()); - - assertFalse(dir.exists()); - } - - @Test - public void testForceRemoveRecursive_DeletesAsMuchAsPossibleWithLockedFiles() throws Exception { - assumeTrue(Functions.isWindows()); - File dir = tmp.newFolder(); - File d1 = new File(dir, "d1"); - File d2 = new File(dir, "d2"); - File f1 = new File(dir, "f1"); - File d1f1 = new File(d1, "d1f1"); - File d2f2 = new File(d2, "d1f2"); - mkdirs(d1, d2); - touchWithFileName(f1, d1f1, d2f2); - - locker.acquireLock(d1f1); - PathRemover remover = PathRemover.newSimpleRemover(); - expectedException.expectMessage(containsString(dir.getPath())); - remover.forceRemoveRecursive(dir.toPath()); - assertTrue(dir.exists()); - assertTrue(d1.exists()); - assertTrue(d1f1.exists()); - assertFalse(d2.exists()); - assertFalse(d2f2.exists()); - assertFalse(f1.exists()); - } - - @Test - public void testForceRemoveRecursive_RetryOnFailure() throws Exception { - assumeTrue(Functions.isWindows()); - File dir = tmp.newFolder(); - File d1 = new File(dir, "d1"); - File d2 = new File(dir, "d2"); - File f1 = new File(dir, "f1"); - File d1f1 = new File(d1, "d1f1"); - File d2f2 = new File(d2, "d1f2"); - mkdirs(d1, d2); - touchWithFileName(f1, d1f1, d2f2); - locker.acquireLock(d2f2); - CountDownLatch unlockLatch = new CountDownLatch(1); - CountDownLatch deleteLatch = new CountDownLatch(1); - AtomicBoolean lockedFileExists = new AtomicBoolean(); - Thread thread = new Thread(() -> { - try { - unlockLatch.await(); - locker.releaseLock(d2f2); - deleteLatch.countDown(); - } catch (Exception ignored) { - } - }); - thread.start(); - PathRemover remover = PathRemover.newRemoverWithStrategy(retriesAttempted -> { - if (retriesAttempted == 0) { - lockedFileExists.set(d2f2.exists()); - unlockLatch.countDown(); - try { - deleteLatch.await(); - return true; - } catch (InterruptedException e) { - return false; - } - } - return false; - }); - remover.forceRemoveRecursive(dir.toPath()); - thread.join(); - assertTrue(lockedFileExists.get()); - assertFalse(dir.exists()); - } - - @Test - public void testForceRemoveRecursive_FailsWhenInterrupted() throws Exception { - assumeTrue(Functions.isWindows()); - File dir = tmp.newFolder(); - File d1 = new File(dir, "d1"); - File d2 = new File(dir, "d2"); - File f1 = new File(dir, "f1"); - File d1f1 = new File(d1, "d1f1"); - File d2f2 = new File(d2, "d1f2"); - mkdirs(d1, d2); - touchWithFileName(f1, d1f1, d2f2); - locker.acquireLock(d1f1); - AtomicReference interrupted = new AtomicReference<>(); - AtomicReference removed = new AtomicReference<>(); - PathRemover remover = PathRemover.newRemoverWithStrategy(retriesAttempted -> { - try { - TimeUnit.SECONDS.sleep(retriesAttempted + 1); - return true; - } catch (InterruptedException e) { - interrupted.set(e); - return false; - } - }); - Thread thread = new Thread(() -> { - try { - remover.forceRemoveRecursive(dir.toPath()); - } catch (IOException e) { - removed.set(e); - } - }); - thread.start(); - TimeUnit.MILLISECONDS.sleep(100); - thread.interrupt(); - thread.join(); - assertFalse(thread.isAlive()); - assertTrue(d1f1.exists()); - IOException ioException = removed.get(); - assertNotNull(ioException); - assertThat(ioException.getMessage(), containsString(dir.getPath())); - assertNotNull(interrupted.get()); - } - - private static void mkdirs(File... dirs) { - for (File dir : dirs) { - assertTrue("Could not mkdir " + dir, dir.mkdir()); - assertTrue(dir.isDirectory()); - } - } - - private static void touchWithFileName(File... files) throws IOException { - for (File file : files) { - try (FileWriter writer = new FileWriter(file)) { - writer.append(file.getName()).append(System.lineSeparator()); - } - assertTrue(file.isFile()); - } - } - -} \ No newline at end of file