From 3b077539f1b5224a89d7e8d2b03d041ef6045f43 Mon Sep 17 00:00:00 2001
From: ribeaudc <ribeaudc>
Date: Thu, 22 May 2008 10:51:33 +0000
Subject: [PATCH] change: - 'ProcessResult' should no longer access the
Process.
SVN: 6244
---
.../process/ProcessExecutionHelper.java | 63 ++++++++---
.../cisd/common/process/ProcessResult.java | 103 +++++++++---------
2 files changed, 97 insertions(+), 69 deletions(-)
diff --git a/common/source/java/ch/systemsx/cisd/common/process/ProcessExecutionHelper.java b/common/source/java/ch/systemsx/cisd/common/process/ProcessExecutionHelper.java
index 86d6d774ce8..851d68e3bfd 100644
--- a/common/source/java/ch/systemsx/cisd/common/process/ProcessExecutionHelper.java
+++ b/common/source/java/ch/systemsx/cisd/common/process/ProcessExecutionHelper.java
@@ -22,6 +22,7 @@ import java.io.IOException;
import java.io.InputStreamReader;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collections;
import java.util.List;
import org.apache.commons.io.IOUtils;
@@ -202,45 +203,58 @@ public final class ProcessExecutionHelper
{
assert millisoWaitForCompletion > 0L : "Unspecified time out.";
final ProcessWatchdog processWatchdog = new ProcessWatchdog(millisoWaitForCompletion);
+ Process process = null;
try
{
- final Process process = launchProcess(commandLine);
+ process = launchProcess(commandLine);
processWatchdog.start(process);
try
{
process.waitFor();
processWatchdog.stop();
+ return createResult(commandLine, process, processWatchdog.isProcessKilled(),
+ readProcessOutputLines(process, machineLog));
} catch (final InterruptedException e)
{
- process.destroy();
operationLog.warn(String.format("Execution of %s interrupted after timeout.",
commandLine));
+ return createResult(commandLine, process, processWatchdog.isProcessKilled(),
+ Collections.<String> emptyList());
}
- return createResult(commandLine, process, processWatchdog.isProcessKilled());
} catch (final IOException ex)
{
return createNotStartedResult(commandLine, ex);
+ } finally
+ {
+ closeStreams(process);
+ process.destroy();
}
}
private final ProcessResult runWithoutWatchdog(final List<String> commandLine)
{
+ Process process = null;
try
{
- final Process process = launchProcess(commandLine);
+ process = launchProcess(commandLine);
try
{
process.waitFor();
+ return createResult(commandLine, process, false, readProcessOutputLines(process,
+ machineLog));
} catch (final InterruptedException e)
{
- process.destroy();
operationLog.warn(String.format("Execution of %s interrupted after timeout.",
commandLine));
+ return createResult(commandLine, process, true, Collections.<String> emptyList());
}
- return createResult(commandLine, process, false);
} catch (final IOException ex)
{
return createNotStartedResult(commandLine, ex);
+ } finally
+ {
+ closeStreams(process);
+ process.destroy();
}
}
@@ -252,11 +266,7 @@ public final class ProcessExecutionHelper
{
operationLog.debug("Executing command: " + commandLine);
}
- // NOTE 2008-02-04, Tomasz Pylak: This operation can get blocked. I've observed it when ln
- // was executed on NAS
- // file system mounted locally.
- final Process process = processBuilder.start();
- return process;
+ return processBuilder.start();
}
private final ProcessResult createNotStartedResult(final List<String> commandLine,
@@ -267,20 +277,29 @@ public final class ProcessExecutionHelper
}
private final ProcessResult createResult(final List<String> commandLine,
- final Process processOrNull, final boolean isInterrupted)
+ final Process processOrNull, final boolean isInterrupted, final List<String> outputLines)
{
if (processOrNull == null)
{
return ProcessResult.createNotStarted(commandLine, operationLog, machineLog);
} else
{
+ final List<String> lines;
+ if (outputLines == null)
+ {
+ lines = readProcessOutputLines(processOrNull, machineLog);
+ } else
+ {
+ lines = outputLines;
+ }
if (isInterrupted)
{
return ProcessResult.createWaitingInterrupted(processOrNull, commandLine,
- operationLog, machineLog);
+ operationLog, machineLog, lines);
} else
{
- return ProcessResult.create(processOrNull, commandLine, operationLog, machineLog);
+ return ProcessResult.create(processOrNull, commandLine, operationLog, machineLog,
+ lines);
}
}
}
@@ -296,7 +315,6 @@ public final class ProcessExecutionHelper
result = runWithoutWatchdog(cmd);
}
result.log();
- result.destroyProcess();
return result.isOK();
}
@@ -352,4 +370,19 @@ public final class ProcessExecutionHelper
}
}
}
+
+ /**
+ * Close the streams belonging to given <var>Process</var>.
+ */
+ private final static void closeStreams(final Process processOrNull)
+ {
+ if (processOrNull == null)
+ {
+ return;
+ }
+ IOUtils.closeQuietly(processOrNull.getInputStream());
+ IOUtils.closeQuietly(processOrNull.getOutputStream());
+ IOUtils.closeQuietly(processOrNull.getErrorStream());
+ }
+
}
diff --git a/common/source/java/ch/systemsx/cisd/common/process/ProcessResult.java b/common/source/java/ch/systemsx/cisd/common/process/ProcessResult.java
index e52b3a66ab6..830bd4e5a2d 100644
--- a/common/source/java/ch/systemsx/cisd/common/process/ProcessResult.java
+++ b/common/source/java/ch/systemsx/cisd/common/process/ProcessResult.java
@@ -17,7 +17,7 @@
package ch.systemsx.cisd.common.process;
import java.io.File;
-import java.io.IOException;
+import java.util.Collections;
import java.util.List;
import org.apache.log4j.Level;
@@ -28,11 +28,10 @@ import org.apache.log4j.Logger;
* <p>
* Since the process output can only ever be read once from a process, it need to be kept around if
* it is needed more than once. This is what this class is good for.
+ * </p>
*/
public final class ProcessResult
{
- private final Process processOrNull; // null if not started
-
private final boolean hasBlocked;
private final List<String> commandLine;
@@ -43,63 +42,69 @@ public final class ProcessResult
private final Logger machineLog;
- private List<String> outputLines;
+ private final int exitValue;
- // process finished or was terminated after timeout
- public static ProcessResult create(final Process process, final List<String> commandLine,
- final Logger operationLog, final Logger machineLog)
+ private final List<String> outputLines;
+
+ private final boolean run;
+
+ /**
+ * Creates a <code>ProcessResult</code> for a process which normally terminates.
+ */
+ public final static ProcessResult create(final Process process, final List<String> commandLine,
+ final Logger operationLog, final Logger machineLog, final List<String> outputLines)
{
- return new ProcessResult(process, false, commandLine, operationLog, machineLog);
+ return new ProcessResult(process, false, commandLine, operationLog, machineLog, outputLines);
}
- // process could not start at all
+ /**
+ * Creates a <code>ProcessResult</code> for a process which did not start at all.
+ */
public static ProcessResult createNotStarted(final List<String> commandLine,
final Logger operationLog, final Logger machineLog)
{
- return new ProcessResult(null, false, commandLine, operationLog, machineLog);
+ return new ProcessResult(null, false, commandLine, operationLog, machineLog, Collections
+ .<String> emptyList());
}
- // process started, but was blocked and could not be terminated, so we stopped waiting for it
+ /**
+ * Creates a <code>ProcessResult</code> for a process which blocked and could not be
+ * terminated. So we stopped waiting for it.
+ */
public static ProcessResult createWaitingInterrupted(final Process process,
- final List<String> commandLine, final Logger operationLog, final Logger machineLog)
+ final List<String> commandLine, final Logger operationLog, final Logger machineLog,
+ final List<String> outputLines)
{
- return new ProcessResult(process, true, commandLine, operationLog, machineLog);
+ return new ProcessResult(process, true, commandLine, operationLog, machineLog, outputLines);
}
private ProcessResult(final Process processOrNull, final boolean hasBlocked,
- final List<String> commandLine, final Logger operationLog, final Logger machineLog)
+ final List<String> commandLine, final Logger operationLog, final Logger machineLog,
+ final List<String> outputLines)
{
this.commandLine = commandLine;
this.commandName = new File(commandLine.get(0)).getName();
- this.processOrNull = processOrNull;
this.hasBlocked = hasBlocked;
this.operationLog = operationLog;
this.machineLog = machineLog;
- this.outputLines = null;
+ this.exitValue = getExitValue(processOrNull, hasBlocked);
+ this.outputLines = outputLines;
+ this.run = processOrNull != null;
}
- /**
- * Calls the {@link Process#destroy()} method which explicitly closes some file handles.
- * <p>
- * <i>Note that one must not call {#link {@link #getProcessOutput()} for the first time after
- * this method has been called.</i>
- * <p>
- * Whether it is necessary to call this method depends on the JRE. For some JREs there occur
- * {@link IOException}s with code 24 ("Too many open files") when running processes with high
- * frequency without calling this method.
- */
- public void destroyProcess()
+ private final static int getExitValue(final Process processOrNull, final boolean hasBlocked)
{
- if (processOrNull != null)
+ if (processOrNull != null && hasBlocked == false)
{
- processOrNull.destroy();
+ return processOrNull.exitValue();
}
+ return ProcessExecutionHelper.NO_EXIT_VALUE;
}
/**
* Returns the command line that belongs to this process.
*/
- public List<String> getCommandLine()
+ public final List<String> getCommandLine()
{
return commandLine;
}
@@ -107,7 +112,7 @@ public final class ProcessResult
/**
* Returns the name of the command that belongs to this process.
*/
- public String getCommandName()
+ public final String getCommandName()
{
return commandName;
}
@@ -115,27 +120,17 @@ public final class ProcessResult
/**
* Returns the lines of the process output.
*/
- public List<String> getProcessOutput()
+ public final List<String> getProcessOutput()
{
- if (outputLines == null)
- {
- outputLines = ProcessExecutionHelper.readProcessOutputLines(processOrNull, machineLog);
- }
return outputLines;
}
- public int exitValue()
+ public final int exitValue()
{
- if (hasBlocked == false && processOrNull != null)
- {
- return processOrNull.exitValue();
- } else
- {
- return ProcessExecutionHelper.NO_EXIT_VALUE;
- }
+ return exitValue;
}
- public boolean isOK()
+ public final boolean isOK()
{
return exitValue() == ProcessExecutionHelper.EXIT_VALUE_OK;
}
@@ -143,31 +138,31 @@ public final class ProcessResult
/**
* Returns <code>true</code> if the process has been run at all.
*/
- public boolean isRun()
+ public final boolean isRun()
{
- return processOrNull != null;
+ return run;
}
/**
* Returns <code>true</code> if the process could not been terminated after the timeout and we
* stopped waiting for it.
*/
- public boolean hasBlocked()
+ public final boolean hasBlocked()
{
return hasBlocked;
}
/**
- * Returns <code>true</code> if the process has been terminated on the Operating System level.
+ * Returns <code>true</code> if the process has been terminated on the <i>Operating System</i>
+ * level.
*/
- public boolean isTerminated()
+ public final boolean isTerminated()
{
return ProcessExecutionHelper.isProcessTerminated(exitValue());
}
- public void log()
+ public final void log()
{
-
if (isOK() == false)
{
logProcessExitValue(Level.WARN);
@@ -179,7 +174,7 @@ public final class ProcessResult
}
}
- private void logProcessExitValue(final Level logLevel)
+ private final void logProcessExitValue(final Level logLevel)
{
if (isRun() == false)
{
@@ -199,7 +194,7 @@ public final class ProcessResult
}
}
- private void logProcessOutput(final Level logLevel)
+ private final void logProcessOutput(final Level logLevel)
{
assert logLevel != null;
--
GitLab