Skip to content

Commit

Permalink
[FIXED JENKINS-25759] Avoid consuming too much memory while running v…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
jglick committed Nov 24, 2014
1 parent b21827b commit 534328b
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 7 deletions.
4 changes: 3 additions & 1 deletion changelog.html
Original file line number Diff line number Diff line change
Expand Up @@ -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=-->

Expand Down
18 changes: 15 additions & 3 deletions core/src/main/java/hudson/FilePath.java
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
*/
Expand Down Expand Up @@ -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();
}
};
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/hudson/tasks/ArtifactArchiver.java
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/hudson/util/FormFieldValidator.java
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
5 changes: 4 additions & 1 deletion core/src/test/java/hudson/FilePathTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.*;

Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 534328b

Please sign in to comment.