diff --git a/src/main/java/org/codelibs/fess/thumbnail/impl/CommandGenerator.java b/src/main/java/org/codelibs/fess/thumbnail/impl/CommandGenerator.java index 1fd6825f9..dffe3e765 100644 --- a/src/main/java/org/codelibs/fess/thumbnail/impl/CommandGenerator.java +++ b/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 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 commandList; + private final InputStreamThread ist; + + private final AtomicBoolean executed = new AtomicBoolean(false); private final long timeout; - protected ProcessDestroyer(final Process p, final List 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 commandList) { diff --git a/src/main/java/org/codelibs/fess/util/InputStreamThread.java b/src/main/java/org/codelibs/fess/util/InputStreamThread.java index 1262f54ac..74f995663 100644 --- a/src/main/java/org/codelibs/fess/util/InputStreamThread.java +++ b/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 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 outputCallback) { + public InputStreamThread(final InputStream is, final Charset charset, final int bufferSize, final Consumer 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); } diff --git a/src/main/java/org/codelibs/fess/util/JobProcess.java b/src/main/java/org/codelibs/fess/util/JobProcess.java index 8f5b56f91..9f513a894 100644 --- a/src/main/java/org/codelibs/fess/util/JobProcess.java +++ b/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 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() {