Complete refactoring of Util.delete methods

This removes duplicated tests from UtilTest that were added in PathRemoverTest. This also ports the sleep/gc strategy for use by default to preserve backward compatibility.

Signed-off-by: Matt Sicker <boards@gmail.com>
This commit is contained in:
Matt Sicker committed 2018-12-14 10:33:18 -06:00
1 parent 3ee6a38484
commit 2e8197a2c7
4 files changed
+107 -535

No files matched your search

+8 -226
View File
@@ -23,14 +23,13 @@
*/
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;
@@ -67,7 +66,6 @@ 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;
@@ -247,16 +245,7 @@ public class Util {
* if the operation fails.
*/
public static void deleteContentsRecursive(@Nonnull File file) throws IOException {
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);
}
}
newPathRemover().forceRemoveDirectoryContents(fileToPath(file));
}
/**
@@ -267,88 +256,7 @@ public class Util {
* @throws IOException if it exists but could not be successfully deleted
*/
public static void deleteFile(@Nonnull File f) throws IOException {
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<PosixFilePermission> 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);
newPathRemover().forceRemoveFile(fileToPath(f));
}
/**
@@ -360,137 +268,7 @@ public class Util {
* if the operation fails.
*/
public static void deleteRecursive(@Nonnull File dir) throws IOException {
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 <em>stop</em> 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 <em>stop</em>
* 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.
* <p>
* See {@link #WAIT_BETWEEN_DELETION_RETRIES} for details of
* the pause duration.<br/>
* 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();
newPathRemover().forceRemoveRecursive(fileToPath(dir));
}
/*
@@ -1797,6 +1575,10 @@ 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.