Skip to content

Commit

Permalink
Make ExecUtil more robust (openhab#1700)
Browse files Browse the repository at this point in the history 
Signed-off-by: Connor Petty <[email protected]>
  • Loading branch information
@cpmeister
cpmeister authored Oct 8, 2020
1 parent c4b76a0 commit 0dfda1e
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 73 deletions.
127 changes: 74 additions & 53 deletions bundles/org.openhab.core.io.net/src/main/java/org/openhab/core/io/net/exec/ExecUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,19 @@

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.util.Arrays;
import java.util.List;
import java.io.StringWriter;
import java.lang.ProcessBuilder.Redirect;
import java.time.Duration;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.core.common.ThreadPoolManager;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -28,87 +35,101 @@
*
* @author Pauli Anttila - Initial contribution
* @author Kai Kreuzer - added exception logging
* @author Connor Petty - replaced delimiter usage with argument array
*/
@NonNullByDefault
public class ExecUtil {

private static Logger logger = LoggerFactory.getLogger(ExecUtil.class);

/**
* Use this to separate between command and parameter, and also between parameters.
*/
public static final String CMD_LINE_DELIMITER = "@@";
private static ExecutorService executor = ThreadPoolManager.getPool("ExecUtil");

/**
* <p>
* Executes <code>commandLine</code>. Sometimes (especially observed on MacOS) the commandLine isn't executed
* properly. In that cases another exec-method is to be used. To accomplish this please use the special delimiter
* '<code>@@</code>'. If <code>commandLine</code> contains this delimiter it is split into a String list and the
* special exec-method is used.
* Executes <code>commandLine</code>.
*
* <p>
* A possible {@link IOException} gets logged but no further processing is done.
*
* @param commandLine the command line to execute
*/
public static void executeCommandLine(String commandLine) {
internalExecute(commandLine);
public static void executeCommandLine(String... commandLine) {
try {
new ProcessBuilder(commandLine).redirectError(Redirect.DISCARD).redirectOutput(Redirect.DISCARD).start();
} catch (IOException e) {
logger.warn("Error occurred when executing commandLine '{}'", commandLine, e);
}
}

/**
* <p>
* Executes <code>commandLine</code> and return its result. Sometimes (especially observed on MacOS) the commandLine
* isn't executed properly. In that cases another exec-method is to be used. To accomplish this please use the
* special delimiter '<code>@@</code>'. If <code>commandLine</code> contains this delimiter it is split into a
* String list and the special exec-method is used.
* Executes <code>commandLine</code> and return its result.
*
* <p>
* A possible {@link IOException} gets logged but no further processing is done.
*
* @param timeout the max time to wait for a process to finish, null to wait indefinitely
* @param commandLine the command line to execute
* @param timeout timeout for execution in milliseconds
* @return response data from executed command line or <code>null</code> if an error occurred
* @return response data from executed command line or <code>null</code> if a timeout or error occurred
*/
public static String executeCommandLineAndWaitResponse(String commandLine, int timeout) {
final Process process = internalExecute(commandLine);
if (process != null) {
try {
process.waitFor(timeout, TimeUnit.MILLISECONDS);
int exitCode = process.exitValue();
final StringBuilder result = new StringBuilder();
try (final BufferedReader reader = new BufferedReader(
new InputStreamReader(process.getInputStream()))) {
String line = "";
while ((line = reader.readLine()) != null) {
result.append(line).append("\n");
}
public static @Nullable String executeCommandLineAndWaitResponse(@Nullable Duration timeout,
String... commandLine) {

Process processTemp = null;
Future<String> outputFuture = null;
Future<String> errorFuture = null;
cleanup: try {
Process process = processTemp = new ProcessBuilder(commandLine).start();

outputFuture = executor.submit(() -> {
try (InputStream inputStream = process.getInputStream();
BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream))) {
StringWriter output = new StringWriter();
reader.transferTo(output);
return output.toString();
}
logger.debug("exit code '{}', result '{}'", exitCode, result);
return result.toString();
} catch (IOException e) {
logger.debug("I/O exception occurred when executing commandLine '{}'", commandLine, e);
} catch (InterruptedException e) {
});

errorFuture = executor.submit(() -> {
try (InputStream inputStream = process.getErrorStream();
BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream))) {
StringWriter output = new StringWriter();
reader.transferTo(output);
return output.toString();
}
});
int exitCode;
if (timeout == null) {
exitCode = process.waitFor();
} else if (process.waitFor(timeout.toMillis(), TimeUnit.MILLISECONDS)) {
exitCode = process.exitValue();
} else {
logger.warn("Timeout occurred when executing commandLine '{}'", commandLine);
logger.debug("{}", e.getMessage(), e);
break cleanup;
}
}
return null;
}

private static @Nullable Process internalExecute(String commandLine) {
final Process process;
try {
if (commandLine.contains(CMD_LINE_DELIMITER)) {
final List<String> cmdArray = Arrays.asList(commandLine.split(CMD_LINE_DELIMITER));
process = new ProcessBuilder(cmdArray).start();
logger.debug("executed commandLine '{}'", cmdArray);
if (exitCode == 0) {
return outputFuture.get();
} else {
process = new ProcessBuilder(commandLine).start();
logger.debug("executed commandLine '{}'", commandLine);
logger.debug("exit code '{}', result '{}', errors '{}'", exitCode, outputFuture.get(),
errorFuture.get());
return null;
}
} catch (ExecutionException e) {
logger.warn("Error occurred when executing commandLine '{}'", commandLine, e.getCause());
} catch (InterruptedException e) {
logger.debug("commandLine '{}' was interrupted", commandLine, e);
} catch (IOException e) {
logger.debug("couldn't execute commandLine '{}'", commandLine, e);
return null;
logger.warn("Error occurred when executing commandLine '{}'", commandLine, e);
}
return process;
if (processTemp != null && processTemp.isAlive()) {
processTemp.destroyForcibly();
}
if (outputFuture != null) {
outputFuture.cancel(true);
}
if (errorFuture != null) {
errorFuture.cancel(true);
}
return null;
}
}
12 changes: 7 additions & 5 deletions bundles/org.openhab.core.io.net/src/test/java/org/openhab/core/io/net/exec/ExecUtilTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

import static org.junit.jupiter.api.Assertions.*;

import java.time.Duration;

import org.junit.jupiter.api.Test;

/**
Expand All @@ -26,7 +28,7 @@ public class ExecUtilTest {
@Test
public void testBasicExecuteCommandLine() {
if (isWindowsSystem()) {
ExecUtil.executeCommandLine("cmd@@/c@@dir");
ExecUtil.executeCommandLine("cmd", "/c", "dir");
} else {
ExecUtil.executeCommandLine("ls");
}
Expand All @@ -36,9 +38,9 @@ public void testBasicExecuteCommandLine() {
public void testBasicExecuteCommandLineAndWaitResponse() {
final String result;
if (isWindowsSystem()) {
result = ExecUtil.executeCommandLineAndWaitResponse("cmd@@/c@@dir", 1000);
result = ExecUtil.executeCommandLineAndWaitResponse(Duration.ofSeconds(1), "cmd", "/c", "dir");
} else {
result = ExecUtil.executeCommandLineAndWaitResponse("ls", 1000);
result = ExecUtil.executeCommandLineAndWaitResponse(Duration.ofSeconds(1), "ls");
}
assertNotNull(result);
assertNotEquals("", result);
Expand All @@ -48,9 +50,9 @@ public void testBasicExecuteCommandLineAndWaitResponse() {
public void testExecuteCommandLineAndWaitResponseWithArguments() {
final String result;
if (isWindowsSystem()) {
result = ExecUtil.executeCommandLineAndWaitResponse("cmd@@/c@@echo@@test", 1000);
result = ExecUtil.executeCommandLineAndWaitResponse(Duration.ofSeconds(1), "cmd", "/c", "echo", "test");
} else {
result = ExecUtil.executeCommandLineAndWaitResponse("echo@@'test'", 1000);
result = ExecUtil.executeCommandLineAndWaitResponse(Duration.ofSeconds(1), "echo", "'test'");
}
assertNotNull(result);
assertNotEquals("test", result);
Expand Down
25 changes: 10 additions & 15 deletions bundles/org.openhab.core.model.script/src/org/openhab/core/model/script/actions/Exec.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
package org.openhab.core.model.script.actions;

import java.io.IOException;
import java.time.Duration;

import org.openhab.core.io.net.exec.ExecUtil;

Expand All @@ -26,40 +27,34 @@ public class Exec {

/**
* <p>
* Executes <code>commandLine</code>. Sometimes (especially observed on MacOS) the commandLine isn't executed
* properly. In that cases another exec-method is to be used. To accomplish this please use the special delimiter '
* <code>@@</code>'. If <code>commandLine</code> contains this delimiter it is split into a String[] array and the
* special exec-method is used.
*
* Executes <code>commandLine</code>.
*
* <p>
* A possible {@link IOException} gets logged but no further processing is done.
*
* @param commandLine
* the command line to execute
* @see http://www.peterfriese.de/running-applescript-from-java/
*/
static public void executeCommandLine(String commandLine) {
public static void executeCommandLine(String... commandLine) {
ExecUtil.executeCommandLine(commandLine);
}

/**
* <p>
* Executes <code>commandLine</code>. Sometimes (especially observed on MacOS) the commandLine isn't executed
* properly. In that cases another exec-method is to be used. To accomplish this please use the special delimiter '
* <code>@@</code>'. If <code>commandLine</code> contains this delimiter it is split into a String[] array and the
* special exec-method is used.
*
* Executes <code>commandLine</code>.
*
* <p>
* A possible {@link IOException} gets logged but no further processing is done.
*
* @param timeout
* timeout for execution, if null will wait indefinitely
* @param commandLine
* the command line to execute
* @param timeout
* timeout for execution in milliseconds
* @return response data from executed command line
*/
static public String executeCommandLine(String commandLine, int timeout) {
return ExecUtil.executeCommandLineAndWaitResponse(commandLine, timeout);
public static String executeCommandLine(Duration timeout, String... commandLine) {
return ExecUtil.executeCommandLineAndWaitResponse(timeout, commandLine);
}

}

0 comments on commit 0dfda1e

Please sign in to comment.