use Future+Executor to enforce the timeout

Author: Vladimir Kotal

opengrok-indexer/src/main/java/org/opengrok/indexer/analysis/Ctags.java

@@ -32,12 +32,19 @@
 import java.io.OutputStreamWriter;
 import java.io.StringReader;
 import java.nio.charset.StandardCharsets;
+import java.time.Duration;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Timer;
-import java.util.TimerTask;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+
+import org.opengrok.indexer.configuration.RuntimeEnvironment;
+import org.opengrok.indexer.index.IndexerParallelizer;
 import org.opengrok.indexer.logger.LoggerFactory;
 import org.opengrok.indexer.util.IOUtils;
 import org.opengrok.indexer.util.SourceSplitter;
@@ -59,7 +66,7 @@ public class Ctags implements Resettable {
     private String binary;
     private String CTagsExtraOptionsFile = null;
     private int tabSize;
-    private int timeout = 0; // in seconds
+    private Duration timeout = Duration.ofSeconds(10);
 
     private boolean junit_testing = false;
 
@@ -93,12 +100,12 @@ public void setCTagsExtraOptionsFile(String CTagsExtraOptionsFile) {
         this.CTagsExtraOptionsFile = CTagsExtraOptionsFile;
     }
 
-    public void setTimeout(int timeout) {
-        this.timeout = timeout;
+    public void setTimeout(long timeout) {
+        this.timeout = Duration.ofSeconds(timeout);
     }
 
-    public int getTimeout() {
-        return this.timeout;
+    public long getTimeout() {
+        return this.timeout.getSeconds();
     }
 
     /**
@@ -363,6 +370,13 @@ private void addScalaSupport(List<String> command) {
         command.add("--regex-scala=/^[[:space:]]*package[[:space:]]+([a-zA-Z0-9_.]+)/\\1/p,packages/");
     }
 
+    /**
+     * Run ctags on a file.
+     * @param file file path to process
+     * @return Definitions
+     * @throws IOException I/O exception
+     * @throws InterruptedException interrupted command
+     */
     public Definitions doCtags(String file) throws IOException,
             InterruptedException {
         if (file.length() < 1 || "\n".equals(file)) {
@@ -388,7 +402,7 @@ public Definitions doCtags(String file) throws IOException,
         CtagsReader rdr = new CtagsReader();
         rdr.setSplitterSupplier(() -> trySplitSource(file));
         rdr.setTabSize(tabSize);
-        Definitions ret;
+        Definitions ret = null;
         try {
             ctagsIn.write(file + "\n");
             if (Thread.interrupted()) {
@@ -399,35 +413,35 @@ public Definitions doCtags(String file) throws IOException,
                 throw new InterruptedException("flush()");
             }
 
-            int timeout = this.timeout * 1000;
-            Timer timer = null;
             /*
-             * Setup timer thread to make sure the ctags process completes and the indexer can
+             * Run the ctags reader in a time bound thread to make sure
+             * the ctags process completes so that the indexer can
              * make progress instead of hanging the whole operation.
              */
-            if (timeout != 0) {
-                timer = new Timer();
-                timer.schedule(new TimerTask() {
-                    @Override public void run() {
-                    LOGGER.log(Level.WARNING,
-                            String.format("Terminating ctags process for file '%s' " +
-                                    "due to timeout %d seconds", file, timeout / 1000));
-                    try {
-                        close();
-                    } catch (IOException e) {
-                        LOGGER.log(Level.WARNING, "Failed to terminate overly long ctags command");
-                    }
-                    }
-                }, timeout);
-            }
+            IndexerParallelizer parallelizer = RuntimeEnvironment.getInstance().getIndexerParallelizer();
+            ExecutorService executor = parallelizer.getCtagsWatcherExecutor();
+            Future<Definitions> future = executor.submit(() -> {
+                readTags(rdr);
+                return rdr.getDefinitions();
+            });
 
-            readTags(rdr);
+            try {
+                ret = future.get(getTimeout(), TimeUnit.SECONDS);
+            } catch (ExecutionException ex) {
+                LOGGER.log(Level.WARNING, "execution exception", ex);
+            } catch (TimeoutException ex) {
+                LOGGER.log(Level.WARNING,
+                        String.format("Terminating ctags process for file '%s' " +
+                                "due to timeout %d seconds", file, getTimeout()));
+                try {
+                    close();
+                } catch (IOException e) {
+                    LOGGER.log(Level.WARNING, "Failed to terminate overly long ctags command");
+                }
 
-            if (timer != null) {
-                timer.cancel();
+                // Allow for retry in IndexDatabase.
+                throw new InterruptedException("ctags timeout");
             }
-
-            ret = rdr.getDefinitions();
         } catch (IOException ex) {
             /*
              * In case the ctags process had to be destroyed, possibly pre-empt

opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/Configuration.java

@@ -207,7 +207,7 @@ public final class Configuration {
     private int tabSize;
     private int commandTimeout; // in seconds
     private int interactiveCommandTimeout; // in seconds
-    private int ctagsTimeout; // in seconds
+    private long ctagsTimeout; // in seconds
     private boolean scopesEnabled;
     private boolean projectsEnabled;
     private boolean foldingEnabled;
@@ -377,7 +377,7 @@ public void setInteractiveCommandTimeout(int commandTimeout) throws IllegalArgum
         this.interactiveCommandTimeout = commandTimeout;
     }
 
-    public int getCtagsTimeout() {
+    public long getCtagsTimeout() {
         return ctagsTimeout;
     }
 
@@ -387,7 +387,7 @@ public int getCtagsTimeout() {
      * @param timeout the new value
      * @throws IllegalArgumentException when the timeout is negative
      */
-    public void setCtagsTimeout(int timeout) throws IllegalArgumentException {
+    public void setCtagsTimeout(long timeout) throws IllegalArgumentException {
         if (commandTimeout < 0) {
             throw new IllegalArgumentException(
                     String.format(NEGATIVE_NUMBER_ERROR, "ctagsTimeout", timeout));
@@ -446,7 +446,7 @@ public Configuration() {
         setContextLimit((short) 10);
         //contextSurround is default(short)
         //ctags is default(String)
-        setCtagsTimeout(30);
+        setCtagsTimeout(10);
         setCurrentIndexedCollapseThreshold(27);
         setDataRoot(null);
         setDisplayRepositories(true);

opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/ConfigurationHelp.java

@@ -140,6 +140,8 @@ private static Object getSampleValue(Method setter, Object defaultValue) {
             return "user-specified-value";
         } else if (paramType == int.class) {
             return 1 + (int) defaultValue;
+        } else if (paramType == long.class) {
+            return 1 + (long) defaultValue;
         } else if (paramType == short.class) {
             return (short) (1 + (short) defaultValue);
         } else if (paramType == boolean.class) {

opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/RuntimeEnvironment.java

@@ -302,11 +302,11 @@ public void setInteractiveCommandTimeout(int timeout) {
         setConfigurationValue("interactiveCommandTimeout", timeout);
     }
 
-    public int getCtagsTimeout() {
-        return (int) getConfigurationValue("ctagsTimeout");
+    public long getCtagsTimeout() {
+        return (long) getConfigurationValue("ctagsTimeout");
     }
 
-    public void setCtagsTimeout(int timeout) {
+    public void setCtagsTimeout(long timeout) {
         setConfigurationValue("ctagsTimeout", timeout);
     }
 

opengrok-indexer/src/main/java/org/opengrok/indexer/index/IndexerParallelizer.java

@@ -26,6 +26,7 @@
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ForkJoinPool;
+import java.util.concurrent.ScheduledThreadPoolExecutor;
 import java.util.logging.Logger;
 import org.opengrok.indexer.analysis.Ctags;
 import org.opengrok.indexer.analysis.CtagsValidator;
@@ -68,6 +69,9 @@ public class IndexerParallelizer implements AutoCloseable {
     private LazilyInstantiate<ExecutorService> lzHistoryRenamedExecutor;
     private ExecutorService historyRenamedExecutor;
 
+    private LazilyInstantiate<ExecutorService> lzCtagsWatcherExecutor;
+    private ExecutorService ctagsWatcherExecutor;
+
     /**
      * Initializes a new instance using settings from the specified environment
      * instance.
@@ -89,6 +93,7 @@ public IndexerParallelizer(RuntimeEnvironment env) {
         createLazyFixedExecutor();
         createLazyHistoryExecutor();
         createLazyHistoryRenamedExecutor();
+        createLazyCtagsWatcherExecutor();
     }
 
     /**
@@ -136,6 +141,15 @@ public ExecutorService getHistoryRenamedExecutor() {
         return result;
     }
 
+    /**
+     * @return the Executor used for ctags parallelism
+     */
+    public ExecutorService getCtagsWatcherExecutor() {
+        ExecutorService result = lzCtagsWatcherExecutor.get();
+        ctagsWatcherExecutor = result;
+        return result;
+    }
+
     /**
      * Calls {@link #bounce()}, which prepares for -- but does not start -- new
      * pools.
@@ -195,6 +209,13 @@ public void bounce() {
             createLazyHistoryRenamedExecutor();
             formerFixedExecutor.shutdown();
         }
+
+        ExecutorService formerCtagsWatcherExecutor = ctagsWatcherExecutor;
+        if (formerCtagsWatcherExecutor != null) {
+            ctagsWatcherExecutor = null;
+            createLazyCtagsWatcherExecutor();
+            formerCtagsWatcherExecutor.shutdown();
+        }
     }
 
     private void createLazyForkJoinPool() {
@@ -208,6 +229,15 @@ private void createLazyCtagsPool() {
                         new CtagsValidator(), new CtagsObjectFactory(env)));
     }
 
+    private void createLazyCtagsWatcherExecutor() {
+        lzCtagsWatcherExecutor = LazilyInstantiate.using(() ->
+                new ScheduledThreadPoolExecutor(1, runnable -> {
+                    Thread thread = Executors.defaultThreadFactory().newThread(runnable);
+                    thread.setName("ctags-watcher-" + thread.getId());
+                    return thread;
+                }));
+    }
+
     private void createLazyFixedExecutor() {
         lzFixedExecutor = LazilyInstantiate.using(() ->
                 Executors.newFixedThreadPool(indexingParallelism));