Skip to content
Permalink
Browse files

[FIXED JENKINS-25759] Avoid consuming too much memory while running v…

…alidateAntFileMask.

Not fully solved, since the scannedDirs field can still grow to be large, but at least clearing files/dirsNotIncluded.
Also imposing a 5s timeout on the scan regardless of file count, and defining a user-customizable bound.
(cherry picked from commit 534328b)

Conflicts:
	changelog.html
  • Loading branch information
jglick authored and olivergondza committed Dec 21, 2014
1 parent bd0453f commit 23e0d6ad1964e0efdd1e0f1c90360321acc8a073
@@ -2236,11 +2236,18 @@ public Boolean call() throws IOException {
* null if no error was found. Otherwise returns a human readable error message.
* @since 1.90
* @see #validateFileMask(FilePath, String)
* @deprecated use {@link #validateAntFileMask(String, int)} instead
*/
public String validateAntFileMask(final String fileMasks) throws IOException, InterruptedException {
return validateAntFileMask(fileMasks, Integer.MAX_VALUE);
}

/**
* Default bound for {@link #validateAntFileMask(String, int)}.
* @since 1.592
*/
public static int VALIDATE_ANT_FILE_MASK_BOUND = Integer.getInteger(FilePath.class.getName() + ".VALIDATE_ANT_FILE_MASK_BOUND", 10000);

/**
* Like {@link #validateAntFileMask(String)} but performing only a bounded number of operations.
* <p>Whereas the unbounded overload is appropriate for calling from cancelable, long-running tasks such as build steps,
@@ -2250,7 +2257,7 @@ public String validateAntFileMask(final String fileMasks) throws IOException, In
* A message is returned in case the file pattern can definitely be determined to not match anything in the directory within the alloted time.
* If the time runs out without finding a match but without ruling out the possibility that there might be one, {@link InterruptedException} is thrown,
* in which case the calling code should give the user the benefit of the doubt and use {@link hudson.util.FormValidation.Kind#OK} (with or without a message).
* @param bound a maximum number of negative operations (deliberately left vague) to perform before giving up on a precise answer; 10_000 is a reasonable pick
* @param bound a maximum number of negative operations (deliberately left vague) to perform before giving up on a precise answer; try {@link #VALIDATE_ANT_FILE_MASK_BOUND}
* @throws InterruptedException not only in case of a channel failure, but also if too many operations were performed without finding any matches
* @since 1.484
*/
@@ -2358,10 +2365,15 @@ private boolean hasMatch(File dir, String pattern) throws InterruptedException {
class Cancel extends RuntimeException {}
DirectoryScanner ds = bound == Integer.MAX_VALUE ? new DirectoryScanner() : new DirectoryScanner() {
int ticks;
long start = System.currentTimeMillis();
@Override public synchronized boolean isCaseSensitive() {
if (!filesIncluded.isEmpty() || !dirsIncluded.isEmpty() || ticks++ > bound) {
if (!filesIncluded.isEmpty() || !dirsIncluded.isEmpty() || ticks++ > bound || System.currentTimeMillis() - start > 5000) {
throw new Cancel();
}
filesNotIncluded.clear();
dirsNotIncluded.clear();
// notFollowedSymlinks might be large, but probably unusual
// scannedDirs will typically be largish, but seems to be needed
return super.isCaseSensitive();
}
};
@@ -2424,7 +2436,7 @@ public FormValidation validateFileMask(String value, boolean errorIfNotExist) th
if(!exists()) // no workspace. can't check
return FormValidation.ok();

String msg = validateAntFileMask(value, 10000);
String msg = validateAntFileMask(value, VALIDATE_ANT_FILE_MASK_BOUND);
if(errorIfNotExist) return FormValidation.error(msg);
else return FormValidation.warning(msg);
} catch (InterruptedException e) {
@@ -227,7 +227,7 @@ public void perform(Run<?,?> build, FilePath ws, Launcher launcher, TaskListener
listenerWarnOrError(listener, Messages.ArtifactArchiver_NoMatchFound(artifacts));
String msg = null;
try {
msg = ws.validateAntFileMask(artifacts);
msg = ws.validateAntFileMask(artifacts, FilePath.VALIDATE_ANT_FILE_MASK_BOUND);
} catch (Exception e) {
listenerWarnOrError(listener, e.getMessage());
}
@@ -378,7 +378,7 @@ protected void check() throws IOException, ServletException {
return;
}

String msg = ws.validateAntFileMask(value, 10000);
String msg = ws.validateAntFileMask(value, FilePath.VALIDATE_ANT_FILE_MASK_BOUND);
if(errorIfNotExist) error(msg);
else warning(msg);
} catch (InterruptedException e) {
@@ -56,6 +56,7 @@
import java.util.concurrent.Future;
import java.util.zip.ZipEntry;
import java.util.zip.ZipOutputStream;
import org.jvnet.hudson.test.Issue;

import static org.mockito.Mockito.*;

@@ -496,11 +497,13 @@ public void testValidateAntFileMask() throws Exception {
}
}

@SuppressWarnings("deprecation")
private static void assertValidateAntFileMask(String expected, FilePath d, String fileMasks) throws Exception {
assertEquals(expected, d.validateAntFileMask(fileMasks));
}

@Bug(7214)
@Issue("JENKINS-7214")
@SuppressWarnings("deprecation")
public void testValidateAntFileMaskBounded() throws Exception {
File tmp = Util.createTempDir();
try {

0 comments on commit 23e0d6a

Please sign in to comment.
You can’t perform that action at this time.