From 534328b264f9338e48418d2bcc0d28daaf48b3a0 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 24 Nov 2014 10:37:58 -0500 Subject: [PATCH] [FIXED JENKINS-25759] Avoid consuming too much memory while running validateAntFileMask. 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. --- changelog.html | 4 +++- core/src/main/java/hudson/FilePath.java | 18 +++++++++++++++--- .../java/hudson/tasks/ArtifactArchiver.java | 2 +- .../java/hudson/util/FormFieldValidator.java | 2 +- core/src/test/java/hudson/FilePathTest.java | 5 ++++- 5 files changed, 24 insertions(+), 7 deletions(-) diff --git a/changelog.html b/changelog.html index f5b4dca23c..ee8e562b3e 100644 --- a/changelog.html +++ b/changelog.html @@ -55,7 +55,9 @@ Upcoming changes diff --git a/core/src/main/java/hudson/FilePath.java b/core/src/main/java/hudson/FilePath.java index 33997c363a..74760a4288 100644 --- a/core/src/main/java/hudson/FilePath.java +++ b/core/src/main/java/hudson/FilePath.java @@ -2324,11 +2324,18 @@ public final class FilePath implements Serializable { * 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. *

Whereas the unbounded overload is appropriate for calling from cancelable, long-running tasks such as build steps, @@ -2338,7 +2345,7 @@ public final class FilePath implements Serializable { * 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 */ @@ -2446,10 +2453,15 @@ public final class FilePath implements Serializable { 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(); } }; @@ -2512,7 +2524,7 @@ public final class FilePath implements Serializable { 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) { diff --git a/core/src/main/java/hudson/tasks/ArtifactArchiver.java b/core/src/main/java/hudson/tasks/ArtifactArchiver.java index ade511433d..f681642b50 100644 --- a/core/src/main/java/hudson/tasks/ArtifactArchiver.java +++ b/core/src/main/java/hudson/tasks/ArtifactArchiver.java @@ -227,7 +227,7 @@ public class ArtifactArchiver extends Recorder implements SimpleBuildStep { 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()); } diff --git a/core/src/main/java/hudson/util/FormFieldValidator.java b/core/src/main/java/hudson/util/FormFieldValidator.java index 9558090186..d344dd43d5 100644 --- a/core/src/main/java/hudson/util/FormFieldValidator.java +++ b/core/src/main/java/hudson/util/FormFieldValidator.java @@ -378,7 +378,7 @@ public abstract class FormFieldValidator { 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) { diff --git a/core/src/test/java/hudson/FilePathTest.java b/core/src/test/java/hudson/FilePathTest.java index 529fcd937e..3fe2788f20 100644 --- a/core/src/test/java/hudson/FilePathTest.java +++ b/core/src/test/java/hudson/FilePathTest.java @@ -56,6 +56,7 @@ import java.util.concurrent.Executors; 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 class FilePathTest extends ChannelTestCase { } } + @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 { -- GitLab