[FIXED JENKINS-24903] Aborts too long running regular expressions. Ad…

…ded a configuration field for the timeout.
jenkinsci · Nov 22, 2015 · c4aff2b · c4aff2b
1 parent 4012e4c
commit c4aff2b
Show file tree

Hide file tree

Showing 6 changed files with 152 additions and 5 deletions.
diff --git a/src/main/java/com/chikli/hudson/plugin/naginator/NaginatorPublisher.java b/src/main/java/com/chikli/hudson/plugin/naginator/NaginatorPublisher.java
@@ -28,6 +28,8 @@
  * @author Nayan Hajratwala <nayan@chikli.com>
  */
 public class NaginatorPublisher extends Notifier {
+    public final static long DEFAULT_REGEXP_TIMEOUT_MS = 30000;
+
     private final String regexpForRerun;
     private final boolean rerunIfUnstable;
     private final boolean rerunMatrixPart;
@@ -176,9 +178,39 @@ public DescriptorImpl getDescriptor() {
      */
     @Extension
     public static final class DescriptorImpl extends BuildStepDescriptor<Publisher> {
+        private long regexpTimeoutMs;
 
         public DescriptorImpl() {
-            super(NaginatorPublisher.class);
+            // default value
+            regexpTimeoutMs = DEFAULT_REGEXP_TIMEOUT_MS;
+            load();
+        }
+
+        /**
+         * @return timeout for regular expressions.
+         * @since 1.16.1
+         */
+        public long getRegexpTimeoutMs() {
+            return regexpTimeoutMs;
+        }
+
+        /**
+         * @param regexpTimeoutMs timeout for regular expressions.
+         * @since 1.16.1
+         */
+        public void setRegexpTimeoutMs(long regexpTimeoutMs) {
+            this.regexpTimeoutMs = regexpTimeoutMs;
+        }
+
+        /**
+         * @see hudson.model.Descriptor#configure(org.kohsuke.stapler.StaplerRequest, net.sf.json.JSONObject)
+         */
+        @Override
+        public boolean configure(StaplerRequest req, JSONObject json) throws hudson.model.Descriptor.FormException {
+            setRegexpTimeoutMs(json.getLong("regexpTimeoutMs"));
+            boolean result = super.configure(req, json);
+            save();
+            return result;
         }
 
         /**

diff --git a/src/main/java/com/chikli/hudson/plugin/naginator/NaginatorPublisherScheduleAction.java b/src/main/java/com/chikli/hudson/plugin/naginator/NaginatorPublisherScheduleAction.java
@@ -10,6 +10,11 @@
 import java.io.File;
 import java.io.FileReader;
 import java.io.IOException;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.FutureTask;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.regex.Matcher;
@@ -18,6 +23,8 @@
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
 
+import jenkins.model.Jenkins;
+
 /**
  * Used from {@link NaginatorPublisher} to mark a build to be reshceduled.
  * 
@@ -118,7 +125,89 @@ private boolean testRegexp(@Nonnull Run<?, ?> run, TaskListener listener) {
         return true;
     }
 
-    private boolean parseLog(File logFile, @Nonnull String regexp) throws IOException {
+    private long getRegexpTimeoutMs() {
+        Jenkins j = Jenkins.getInstance();
+        if (j == null) {
+            return NaginatorPublisher.DEFAULT_REGEXP_TIMEOUT_MS;
+        }
+        NaginatorPublisher.DescriptorImpl d = (NaginatorPublisher.DescriptorImpl)j.getDescriptor(NaginatorPublisher.class);
+        if (d == null) {
+            return NaginatorPublisher.DEFAULT_REGEXP_TIMEOUT_MS;
+        }
+        return d.getRegexpTimeoutMs();
+    }
+
+    private boolean parseLog(final File logFile, @Nonnull final String regexp) throws IOException {
+        // TODO annotate `logFile` with `@Nonnull`
+        // after upgrading the target Jenkins to 1.568 or later.
+
+        long timeout = getRegexpTimeoutMs();
+
+        FutureTask<Boolean> task = new FutureTask<Boolean>(new Callable<Boolean>() {
+            public Boolean call() throws Exception {
+                return parseLogImpl(logFile, regexp);
+            }
+        });
+
+        Thread t = new Thread(task);
+        t.start();
+
+        try {
+            // never null
+            return task.get(timeout, TimeUnit.MILLISECONDS);
+        } catch (TimeoutException e) {
+            LOGGER.log(
+                    Level.WARNING,
+                    String.format("Aborted regexp '%s' for too long execution time ( > %d ms).", regexp, timeout)
+            );
+        } catch (InterruptedException e) {
+            LOGGER.log(
+                    Level.SEVERE,
+                    String.format("Aborted regexp '%s'", regexp),
+                    e
+            );
+        } catch (ExecutionException e) {
+            LOGGER.log(
+                    Level.SEVERE,
+                    String.format("Aborted regexp '%s'", regexp),
+                    e
+            );
+        }
+        if (t.isAlive()) {
+            t.interrupt();
+        }
+        try {
+            t.join();
+        } catch (InterruptedException e) {
+            // ok
+        }
+        return false;
+    }
+
+    private class InterruptibleCharSequence implements CharSequence {
+        private final CharSequence wrapped;
+
+        public InterruptibleCharSequence(CharSequence wrapped) {
+            this.wrapped = wrapped;
+        }
+
+        public int length() {
+            return wrapped.length();
+        }
+
+        public char charAt(int index) {
+            if (Thread.currentThread().isInterrupted()) {
+                throw new RuntimeException(new InterruptedException());
+            }
+            return wrapped.charAt(index);
+        }
+
+        public CharSequence subSequence(int start, int end) {
+            return wrapped.subSequence(start, end);
+        }
+    }
+
+    private boolean parseLogImpl(final File logFile, @Nonnull final String regexp) throws IOException {
         // TODO annotate `logFile` with `@Nonnull`
         // after upgrading the target Jenkins to 1.568 or later.
 
@@ -129,7 +218,7 @@ private boolean parseLog(File logFile, @Nonnull String regexp) throws IOExceptio
         try {
             reader = new BufferedReader(new FileReader(logFile));
             while ((line = reader.readLine()) != null) {
-                Matcher matcher = pattern.matcher(line);
+                Matcher matcher = pattern.matcher(new InterruptibleCharSequence(line));
                 if (matcher.find()) {
                     return true;
                 }

diff --git a/src/main/resources/com/chikli/hudson/plugin/naginator/NaginatorPublisher/global.jelly b/src/main/resources/com/chikli/hudson/plugin/naginator/NaginatorPublisher/global.jelly
@@ -1,3 +1,7 @@
 <j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form">
-  <!-- nothing to configure -->
-</j:jelly>
+<f:section title="${descriptor.displayName}">
+  <f:entry field="regexpTimeoutMs" title="${%Timeout for regular expressions (ms)}">
+    <f:textbox />
+  </f:entry>
+</f:section>
+</j:jelly>
diff --git a/...resources/com/chikli/hudson/plugin/naginator/NaginatorPublisher/help-regexpTimeoutMs.html b/...resources/com/chikli/hudson/plugin/naginator/NaginatorPublisher/help-regexpTimeoutMs.html
@@ -0,0 +1,4 @@
+<div>
+Milliseconds to abort regular expressions.
+Some regular expressions can cause long execution time and exhaust CPUs.
+</div>
diff --git a/src/test/java/com/chikli/hudson/plugin/naginator/NaginatorListenerTest.java b/src/test/java/com/chikli/hudson/plugin/naginator/NaginatorListenerTest.java
@@ -249,6 +249,9 @@ public void testCatastorophicRegularExpression() throws Exception {
                 new FixedDelay(0)       // delay
         ));
 
+        ((NaginatorPublisher.DescriptorImpl)jenkins.getDescriptor(NaginatorPublisher.class))
+            .setRegexpTimeoutMs(1000);
+
         p.scheduleBuild2(0);
         waitUntilNoActivityUpTo(10 * 1000);
     }

diff --git a/src/test/java/com/chikli/hudson/plugin/naginator/NaginatorPublisherTest.java b/src/test/java/com/chikli/hudson/plugin/naginator/NaginatorPublisherTest.java
@@ -328,4 +328,19 @@ public void testConfigurationForRegexpOnFreeStyleProject() throws Exception {
         assertFalse(p.getPublishersList().get(NaginatorPublisher.class).isRegexpForMatrixParent());
         j.assertEqualDataBoundBeans(expected, p.getPublishersList().get(NaginatorPublisher.class));
     }
+
+    @Test
+    public void testRegexpTimeoutMsInSystemConfiguration() throws Exception {
+        NaginatorPublisher.DescriptorImpl d = (NaginatorPublisher.DescriptorImpl)j.jenkins.getDescriptor(NaginatorPublisher.class);
+        d.setRegexpTimeoutMs(NaginatorPublisher.DEFAULT_REGEXP_TIMEOUT_MS);
+        d.save();       // ensure the default value is saved to the file.
+
+        // set to the memory, but not saved to the file.
+        d.setRegexpTimeoutMs(1000);
+
+        j.configRoundtrip();
+
+        d.load();
+        assertEquals(1000, d.getRegexpTimeoutMs());
+    }
 }