fix #2619 improve a process handling

src/main/java/org/codelibs/fess/thumbnail/impl/CommandGenerator.java

@@ -15,24 +15,25 @@
  */
 package org.codelibs.fess.thumbnail.impl;
 
-import java.io.BufferedReader;
 import java.io.File;
-import java.io.InputStreamReader;
 import java.nio.charset.Charset;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Timer;
 import java.util.TimerTask;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 import javax.annotation.PostConstruct;
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.codelibs.core.concurrent.CommonPoolUtil;
 import org.codelibs.core.io.CloseableUtil;
 import org.codelibs.core.io.CopyUtil;
 import org.codelibs.core.lang.StringUtil;
 import org.codelibs.fess.util.ComponentUtil;
+import org.codelibs.fess.util.InputStreamThread;
 
 public class CommandGenerator extends BaseThumbnailGenerator {
     private static final Logger logger = LogManager.getLogger(CommandGenerator.class);
@@ -45,7 +46,7 @@ public class CommandGenerator extends BaseThumbnailGenerator {
 
     protected File baseDir;
 
-    private volatile Timer destoryTimer;
+    private Timer destoryTimer;
 
     @PostConstruct
     public void init() {
@@ -127,48 +128,48 @@ public class CommandGenerator extends BaseThumbnailGenerator {
     }
 
     protected void executeCommand(final String thumbnailId, final List<String> cmdList) {
-        ProcessBuilder pb = null;
-        Process p = null;
-
-        if (logger.isDebugEnabled()) {
-            logger.debug("Thumbnail Command: {}", cmdList);
-        }
-
-        TimerTask task = null;
+        ProcessDestroyer task = null;
         try {
-            pb = new ProcessBuilder(cmdList);
+            ProcessBuilder pb = new ProcessBuilder(cmdList);
             pb.directory(baseDir);
             pb.redirectErrorStream(true);
 
-            p = pb.start();
+            if (logger.isDebugEnabled()) {
+                logger.debug("Thumbnail Command: {}", cmdList);
+            }
 
-            task = new ProcessDestroyer(p, cmdList, commandTimeout);
-            try {
-                destoryTimer.schedule(task, commandTimeout);
+            Process p = pb.start();
 
-                String line;
-                BufferedReader br = null;
-                try {
-                    br = new BufferedReader(new InputStreamReader(p.getInputStream(), Charset.defaultCharset()));
-                    while ((line = br.readLine()) != null) {
-                        if (logger.isDebugEnabled()) {
-                            logger.debug(line);
-                        }
-                    }
-                } finally {
-                    CloseableUtil.closeQuietly(br);
+            final InputStreamThread ist = new InputStreamThread(p.getInputStream(), Charset.defaultCharset(), 0, s -> {
+                if (logger.isDebugEnabled()) {
+                    logger.debug(s);
                 }
+            });
+            task = new ProcessDestroyer(p, ist, commandDestroyTimeout);
+            destoryTimer.schedule(task, commandTimeout);
+            ist.start();
 
-                p.waitFor();
-            } catch (final Exception e) {
+            if (logger.isDebugEnabled()) {
+                logger.debug("Waiting for {}.", getName());
+            }
+            if (!p.waitFor(commandTimeout + commandDestroyTimeout, TimeUnit.MILLISECONDS)) {
+                logger.warn("Destroying {} because of the process timeout.", getName());
                 p.destroy();
             }
+
+            final int exitValue = p.exitValue();
+            if (exitValue != 0) {
+                logger.warn("{} is failed (exit code:{}, timeout:{}): {}", getName(), exitValue, task.isExecuted(), commandList);
+            }
+
+            if (logger.isDebugEnabled()) {
+                logger.debug("{} is finished with exit code {}.", getName(), exitValue);
+            }
         } catch (final Exception e) {
-            logger.warn("Failed to generate a thumbnail of {}", thumbnailId, e);
+            logger.warn("Failed to generate a thumbnail of {}: {}", thumbnailId, commandList, e);
         }
-        if (task != null) {
+        if (task != null && !task.isExecuted()) {
             task.cancel();
-            task = null;
         }
     }
 
@@ -176,25 +177,70 @@ public class CommandGenerator extends BaseThumbnailGenerator {
 
         private final Process p;
 
-        private final List<String> commandList;
+        private final InputStreamThread ist;
+
+        private final AtomicBoolean executed = new AtomicBoolean(false);
 
         private final long timeout;
 
-        protected ProcessDestroyer(final Process p, final List<String> commandList, final long timeout) {
+        public ProcessDestroyer(final Process p, final InputStreamThread ist, final long timeout) {
             this.p = p;
-            this.commandList = commandList;
+            this.ist = ist;
             this.timeout = timeout;
         }
 
         @Override
         public void run() {
-            logger.warn("CommandGenerator is timed out: {}", commandList);
+            if (!p.isAlive()) {
+                return;
+            }
+
+            executed.set(true);
+
+            if (logger.isDebugEnabled()) {
+                logger.debug("Interrupting a stream thread.");
+            }
+            ist.interrupt();
+
+            CommonPoolUtil.execute(() -> {
+                try {
+                    CloseableUtil.closeQuietly(p.getInputStream());
+                } catch (final Exception e) {
+                    logger.warn("Could not close a process input stream.", e);
+                }
+            });
+            CommonPoolUtil.execute(() -> {
+                try {
+                    CloseableUtil.closeQuietly(p.getErrorStream());
+                } catch (final Exception e) {
+                    logger.warn("Could not close a process error stream.", e);
+                }
+            });
+            CommonPoolUtil.execute(() -> {
+                try {
+                    CloseableUtil.closeQuietly(p.getOutputStream());
+                } catch (final Exception e) {
+                    logger.warn("Could not close a process output stream.", e);
+                }
+            });
+
+            if (logger.isDebugEnabled()) {
+                logger.debug("Terminating process {}.", p);
+            }
             try {
-                p.destroyForcibly().waitFor(timeout, TimeUnit.MILLISECONDS);
+                if (!p.destroyForcibly().waitFor(timeout, TimeUnit.MILLISECONDS)) {
+                    logger.warn("Terminating process {} is timed out.", p);
+                } else if (logger.isDebugEnabled()) {
+                    logger.debug("Terminated process {}.", p);
+                }
             } catch (final Exception e) {
                 logger.warn("Failed to stop destroyer.", e);
             }
         }
+
+        public boolean isExecuted() {
+            return executed.get();
+        }
     }
 
     public void setCommandList(final List<String> commandList) {

src/main/java/org/codelibs/fess/util/InputStreamThread.java

@@ -18,19 +18,18 @@ package org.codelibs.fess.util;
 import java.io.BufferedReader;
 import java.io.InputStream;
 import java.io.InputStreamReader;
-import java.io.UnsupportedEncodingException;
+import java.nio.charset.Charset;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.function.Consumer;
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
-import org.codelibs.fess.exception.FessSystemException;
 
 public class InputStreamThread extends Thread {
     private static final Logger logger = LogManager.getLogger(InputStreamThread.class);
 
-    private BufferedReader br;
+    private final BufferedReader br;
 
     public static final int MAX_BUFFER_SIZE = 1000;
 
@@ -40,20 +39,17 @@ public class InputStreamThread extends Thread {
 
     private final Consumer<String> outputCallback;
 
+    @Deprecated
     public InputStreamThread(final InputStream is, final String charset) {
-        this(is, charset, MAX_BUFFER_SIZE, null);
+        this(is, Charset.forName(charset), MAX_BUFFER_SIZE, null);
     }
 
-    public InputStreamThread(final InputStream is, final String charset, final int bufferSize, final Consumer<String> outputCallback) {
+    public InputStreamThread(final InputStream is, final Charset charset, final int bufferSize, final Consumer<String> outputCallback) {
         super("InputStreamThread");
         this.bufferSize = bufferSize;
         this.outputCallback = outputCallback;
 
-        try {
-            br = new BufferedReader(new InputStreamReader(is, charset));
-        } catch (final UnsupportedEncodingException e) {
-            throw new FessSystemException(e);
-        }
+        br = new BufferedReader(new InputStreamReader(is, charset));
     }
 
     @Override
@@ -68,7 +64,9 @@ public class InputStreamThread extends Thread {
                     if (logger.isDebugEnabled()) {
                         logger.debug(line);
                     }
-                    list.add(line);
+                    if (bufferSize > 0) {
+                        list.add(line);
+                    }
                     if (outputCallback != null) {
                         outputCallback.accept(line);
                     }

src/main/java/org/codelibs/fess/util/JobProcess.java

@@ -30,7 +30,7 @@ public class JobProcess {
 
     public JobProcess(final Process process, final int bufferSize, final Consumer<String> outputCallback) {
         this.process = process;
-        inputStreamThread = new InputStreamThread(process.getInputStream(), Constants.UTF_8, bufferSize, outputCallback);
+        inputStreamThread = new InputStreamThread(process.getInputStream(), Constants.CHARSET_UTF_8, bufferSize, outputCallback);
     }
 
     public Process getProcess() {