[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.
jenkinsci · Nov 24, 2014 · 534328b · 534328b
1 parent b21827b
commit 534328b
Show file tree

Hide file tree

Showing 5 changed files with 24 additions and 7 deletions.
diff --git a/changelog.html b/changelog.html
@@ -55,7 +55,9 @@
 <!-- Record your changes in the trunk here. -->
 <div id="trunk" style="display:none"><!--=TRUNK-BEGIN=-->
 <ul class=image>
-  <li class=>
+  <li class=bug>
+    Performance problems on large workspaces associated with validating file include patterns.
+    (<a href="https://issues.jenkins-ci.org/browse/JENKINS-25759">issue 25759</a>)
 </ul>
 </div><!--=TRUNK-END=-->
 

diff --git a/core/src/main/java/hudson/FilePath.java b/core/src/main/java/hudson/FilePath.java
@@ -2324,11 +2324,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,
@@ -2338,7 +2345,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
      */
@@ -2446,10 +2453,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();
                     }
                 };
@@ -2512,7 +2524,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) {

diff --git a/core/src/main/java/hudson/tasks/ArtifactArchiver.java b/core/src/main/java/hudson/tasks/ArtifactArchiver.java
@@ -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());
                     }

diff --git a/core/src/main/java/hudson/util/FormFieldValidator.java b/core/src/main/java/hudson/util/FormFieldValidator.java
@@ -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) {

diff --git a/core/src/test/java/hudson/FilePathTest.java b/core/src/test/java/hudson/FilePathTest.java
@@ -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 {