This is an automated email from the ASF dual-hosted git repository. ggregory pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-exec.git
commit ef031c2f293322ac9c611f97034df2333ecffe3d Author: Gary Gregory <[email protected]> AuthorDate: Sat Dec 30 09:54:45 2023 -0500 [EXEC-70] Delegate thread creation to java.util.concurrent.ThreadFactory - Add builders to avoid constructor creep: - Add DefaultExecutor.Builder - Add DaemonExecutor.Builder - Add ExecuteWatchdog.Builder - Add Watchdog.Builder - No need for date for a change.xml action --- src/changes/changes.xml | 7 +- .../org/apache/commons/exec/DaemonExecutor.java | 52 ++++- .../org/apache/commons/exec/DefaultExecutor.java | 234 ++++++++++++++------- .../org/apache/commons/exec/ExecuteWatchdog.java | 94 +++++++-- .../java/org/apache/commons/exec/Executor.java | 8 +- .../org/apache/commons/exec/PumpStreamHandler.java | 131 +++++++----- .../java/org/apache/commons/exec/ThreadUtil.java | 46 ++++ .../java/org/apache/commons/exec/Watchdog.java | 78 ++++++- .../environment/DefaultProcessingEnvironment.java | 2 +- src/site/apt/tutorial.apt | 12 +- .../apache/commons/exec/DefaultExecutorTest.java | 32 ++- .../apache/commons/exec/LogOutputStreamTest.java | 4 +- .../org/apache/commons/exec/StandAloneTest.java | 41 +++- .../java/org/apache/commons/exec/TutorialTest.java | 11 +- .../org/apache/commons/exec/issues/Exec33Test.java | 4 +- .../org/apache/commons/exec/issues/Exec34Test.java | 2 +- .../org/apache/commons/exec/issues/Exec36Test.java | 2 +- .../org/apache/commons/exec/issues/Exec41Test.java | 4 +- .../org/apache/commons/exec/issues/Exec44Test.java | 2 +- .../org/apache/commons/exec/issues/Exec49Test.java | 2 +- .../org/apache/commons/exec/issues/Exec57Test.java | 4 +- .../org/apache/commons/exec/issues/Exec60Test.java | 2 +- .../org/apache/commons/exec/issues/Exec62Test.java | 2 +- .../org/apache/commons/exec/issues/Exec65Test.java | 6 +- 24 files changed, 564 insertions(+), 218 deletions(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 6542041d..fb478e99 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -32,8 +32,12 @@ <action dev="ggregory" type="add" due-to="Gary Gregory">Add Watchdog.Watchdog(Duration).</action> <action dev="ggregory" type="add" due-to="Gary Gregory">Add ExecuteWatchdog.ExecuteWatchdog(Duration).</action> <action dev="ggregory" type="add" due-to="Gary Gregory">Add PumpStreamHandler.setStopTimeout(Duration) and deprecate PumpStreamHandler.setStopTimeout(long).</action> + <action dev="ggregory" type="add" due-to="Gary Gregory">Add DefaultExecutor.Builder.</action> + <action dev="ggregory" type="add" due-to="Gary Gregory">Add DaemonExecutor.Builder.</action> + <action dev="ggregory" type="add" due-to="Gary Gregory">Add ExecuteWatchdog.Builder.</action> + <action dev="ggregory" type="add" due-to="Gary Gregory">Add Watchdog.Builder.</action> <!-- FIX --> - <action issue="EXEC-105" type="fix" date="2023-07-16" due-to="Dimitrios Efthymiou">Fix code snippet in tutorial page.</action> + <action issue="EXEC-105" type="fix" due-to="Dimitrios Efthymiou">Fix code snippet in tutorial page.</action> <action issue="EXEC-100" dev="sgoeschl" type="fix" date="2016-01-11">Sync org.apache.commons.exec.OS with the newest Ant source file.</action> <action issue="EXEC-64" dev="sebb" type="fix" due-to="Michael Vorburger">DefaultExecutor swallows IOException cause instead of propagating it (work-round for Java 1.5).</action> <action dev="ggregory" type="fix" due-to="nullptr">Java-style Array declaration and remove empty finally block #26.</action> @@ -45,6 +49,7 @@ <action dev="ggregory" type="fix" due-to="Gary Gregory">Propagate exception in DebugUtils.handleException(String, Exception).</action> <action dev="ggregory" type="fix" due-to="Gary Gregory">Deprecate StringUtils.toString(String[], String) in favor of String.join(CharSequence, CharSequence...).</action> <action issue="EXEC-78" dev="ggregory" type="fix">No need to use System.class.getMethod("getenv",...) any more.</action> + <action issue="EXEC-70" dev="ggregory" type="fix">Delegate thread creation to java.util.concurrent.ThreadFactory.</action> <!-- REMOVE --> <action dev="ggregory" type="remove" due-to="Gary Gregory">Deprecate DefaultExecuteResultHandler.waitFor(long).</action> <action dev="ggregory" type="remove" due-to="Gary Gregory">Deprecate ExecuteWatchdog.ExecuteWatchdog(long).</action> diff --git a/src/main/java/org/apache/commons/exec/DaemonExecutor.java b/src/main/java/org/apache/commons/exec/DaemonExecutor.java index a5217f06..6d3780ef 100644 --- a/src/main/java/org/apache/commons/exec/DaemonExecutor.java +++ b/src/main/java/org/apache/commons/exec/DaemonExecutor.java @@ -16,6 +16,9 @@ */ package org.apache.commons.exec; +import java.io.File; +import java.util.concurrent.ThreadFactory; + /** * Runs daemon processes asynchronously. Callers are expected to register a {@link ProcessDestroyer} before executing any processes. * @@ -23,6 +26,49 @@ package org.apache.commons.exec; */ public class DaemonExecutor extends DefaultExecutor { + /** + * Constructs a new builder. + * + * @since 1.4.0 + */ + public static class Builder extends DefaultExecutor.Builder<Builder> { + + /** + * Creates a new configured DaemonExecutor. + * + * @return a new configured DaemonExecutor. + */ + @Override + public DefaultExecutor get() { + return new DaemonExecutor(getThreadFactory(), getExecuteStreamHandler(), getWorkingDirectory()); + } + + } + + /** + * Constructs a new instance. + * + * @deprecated Use {@link Builder#get()}. + */ + @Deprecated + public DaemonExecutor() { + // super + } + + private DaemonExecutor(final ThreadFactory threadFactory, final ExecuteStreamHandler executeStreamHandler, final File workingDirectory) { + super(threadFactory, executeStreamHandler, workingDirectory); + } + + /** + * Creates a new builder. + * + * @return a new builder. + * @since 1.4.0 + */ + public static Builder builder() { + return new Builder(); + } + /** * Factory method to create a thread waiting for the result of an asynchronous execution. * @@ -32,8 +78,8 @@ public class DaemonExecutor extends DefaultExecutor { */ @Override protected Thread createThread(final Runnable runnable, final String name) { - final Thread t = super.createThread(runnable, name); - t.setDaemon(true); - return t; + final Thread thread = super.createThread(runnable, name); + thread.setDaemon(true); + return thread; } } diff --git a/src/main/java/org/apache/commons/exec/DefaultExecutor.java b/src/main/java/org/apache/commons/exec/DefaultExecutor.java index 5bfb47b9..910c019e 100644 --- a/src/main/java/org/apache/commons/exec/DefaultExecutor.java +++ b/src/main/java/org/apache/commons/exec/DefaultExecutor.java @@ -16,12 +16,16 @@ */ package org.apache.commons.exec; -import org.apache.commons.exec.launcher.CommandLauncher; -import org.apache.commons.exec.launcher.CommandLauncherFactory; - +import java.io.Closeable; import java.io.File; import java.io.IOException; import java.util.Map; +import java.util.concurrent.Executors; +import java.util.concurrent.ThreadFactory; +import java.util.function.Supplier; + +import org.apache.commons.exec.launcher.CommandLauncher; +import org.apache.commons.exec.launcher.CommandLauncherFactory; /** * The default class to start a subprocess. The implementation allows to @@ -37,15 +41,99 @@ import java.util.Map; * The following example shows the basic usage: * * <pre> - * Executor exec = new DefaultExecutor(); + * Executor exec = DefaultExecutor.builder().get(); * CommandLine cl = new CommandLine("ls -l"); * int exitvalue = exec.execute(cl); * </pre> */ public class DefaultExecutor implements Executor { + /** + * Constructs a new builder. + * + * @param <T> The builder type. + * @since 1.4.0 + */ + public static class Builder<T extends Builder<T>> implements Supplier<DefaultExecutor> { + + private ThreadFactory threadFactory; + private ExecuteStreamHandler executeStreamHandler; + private File workingDirectory; + + @SuppressWarnings("unchecked") + T asThis() { + return (T) this; + } + + /** + * Creates a new configured DefaultExecutor. + * + * @return a new configured DefaultExecutor. + */ + @Override + public DefaultExecutor get() { + return new DefaultExecutor(threadFactory, executeStreamHandler, workingDirectory); + } + + ExecuteStreamHandler getExecuteStreamHandler() { + return executeStreamHandler; + } + + ThreadFactory getThreadFactory() { + return threadFactory; + } + + File getWorkingDirectory() { + return workingDirectory; + } + + /** + * Sets the PumpStreamHandler. + * + * @param executeStreamHandler the ExecuteStreamHandler, null resets to the default. + * @return this. + */ + public T setExecuteStreamHandler(final ExecuteStreamHandler executeStreamHandler) { + this.executeStreamHandler = executeStreamHandler; + return asThis(); + } + + /** + * Sets the ThreadFactory. + * + * @param threadFactory the ThreadFactory, null resets to the default. + * @return this. + */ + public T setThreadFactory(final ThreadFactory threadFactory) { + this.threadFactory = threadFactory; + return asThis(); + } + + /** + * Sets the working directory. + * + * @param workingDirectory the working directory., null resets to the default. + * @return this. + */ + public T setWorkingDirectory(final File workingDirectory) { + this.workingDirectory = workingDirectory; + return asThis(); + } + + } + + /** + * Creates a new builder. + * + * @return a new builder. + * @since 1.4.0 + */ + public static Builder<?> builder() { + return new Builder<>(); + } + /** Taking care of output and error stream. */ - private ExecuteStreamHandler streamHandler; + private ExecuteStreamHandler executeStreamHandler; /** The working directory of the process. */ private File workingDirectory; @@ -68,44 +156,64 @@ public class DefaultExecutor implements Executor { /** The first exception being caught to be thrown to the caller. */ private IOException exceptionCaught; + /** + * The thread factory. + */ + private final ThreadFactory threadFactory; + /** * Constructs a default {@code PumpStreamHandler} and sets the working directory of the subprocess to the current working directory. * * The {@code PumpStreamHandler} pumps the output of the subprocess into our {@code System.out} and {@code System.err} to avoid into our {@code System.out} * and {@code System.err} to avoid a blocked or deadlocked subprocess (see {@link Process Process}). + * @deprecated Use {@link Builder#get()}. */ + @Deprecated public DefaultExecutor() { - this.streamHandler = new PumpStreamHandler(); + this(Executors.defaultThreadFactory(), new PumpStreamHandler(), new File(".")); + } + + DefaultExecutor(final ThreadFactory threadFactory, final ExecuteStreamHandler executeStreamHandler, final File workingDirectory) { + this.threadFactory = threadFactory != null ? threadFactory : Executors.defaultThreadFactory(); + this.executeStreamHandler = executeStreamHandler != null ? executeStreamHandler : new PumpStreamHandler(); + this.workingDirectory = workingDirectory != null ? workingDirectory : new File("."); this.launcher = CommandLauncherFactory.createVMLauncher(); this.exitValues = new int[0]; - this.workingDirectory = new File("."); - this.exceptionCaught = null; } - /** - * Closes the streams belonging to the given Process. - * - * @param process the {@link Process}. - */ - private void closeProcessStreams(final Process process) { + private void checkWorkingDirectory() throws IOException { + checkWorkingDirectory(workingDirectory); + } - try { - process.getInputStream().close(); - } catch (final IOException e) { - setExceptionCaught(e); + private void checkWorkingDirectory(final File directory) throws IOException { + if (directory != null && !directory.exists()) { + throw new IOException(directory + " doesn't exist."); } + } + /** + * Closes the Closeable, remembering any exception. + * + * @param closeable the {@link Closeable} to close. + */ + private void closeCatch(final Closeable closeable) { try { - process.getOutputStream().close(); + closeable.close(); } catch (final IOException e) { setExceptionCaught(e); } + } - try { - process.getErrorStream().close(); - } catch (final IOException e) { - setExceptionCaught(e); - } + /** + * Closes the streams belonging to the given Process. + * + * @param process the {@link Process}. + */ + @SuppressWarnings("resource") + private void closeProcessStreams(final Process process) { + closeCatch(process.getInputStream()); + closeCatch(process.getOutputStream()); + closeCatch(process.getErrorStream()); } /** @@ -116,7 +224,7 @@ public class DefaultExecutor implements Executor { * @return the thread */ protected Thread createThread(final Runnable runnable, final String name) { - return new Thread(runnable, name); + return ThreadUtil.newThread(threadFactory, runnable, name, false); } /** @@ -140,13 +248,8 @@ public class DefaultExecutor implements Executor { */ @Override public int execute(final CommandLine command, final Map<String, String> environment) throws ExecuteException, IOException { - - if (workingDirectory != null && !workingDirectory.exists()) { - throw new IOException(workingDirectory + " doesn't exist."); - } - - return executeInternal(command, environment, workingDirectory, streamHandler); - + checkWorkingDirectory(); + return executeInternal(command, environment, workingDirectory, executeStreamHandler); } /** @@ -155,26 +258,21 @@ public class DefaultExecutor implements Executor { @Override public void execute(final CommandLine command, final Map<String, String> environment, final ExecuteResultHandler handler) throws ExecuteException, IOException { - - if (workingDirectory != null && !workingDirectory.exists()) { - throw new IOException(workingDirectory + " doesn't exist."); - } - + checkWorkingDirectory(); if (watchdog != null) { watchdog.setProcessNotStarted(); } - executorThread = createThread(() -> { int exitValue = Executor.INVALID_EXITVALUE; try { - exitValue = executeInternal(command, environment, workingDirectory, streamHandler); + exitValue = executeInternal(command, environment, workingDirectory, executeStreamHandler); handler.onProcessComplete(exitValue); } catch (final ExecuteException e) { handler.onProcessFailed(e); } catch (final Exception e) { handler.onProcessFailed(new ExecuteException("Execution failed", exitValue, e)); } - }, "Exec Default Executor"); + }, "CommonsExecDefaultExecutor"); getExecutorThread().start(); } @@ -190,10 +288,8 @@ public class DefaultExecutor implements Executor { */ private int executeInternal(final CommandLine command, final Map<String, String> environment, final File workingDirectory, final ExecuteStreamHandler streams) throws IOException { - final Process process; exceptionCaught = null; - try { process = launch(command, environment, workingDirectory); } catch (final IOException e) { @@ -202,7 +298,6 @@ public class DefaultExecutor implements Executor { } throw e; } - try { setStreams(streams, process); } catch (final IOException e) { @@ -212,23 +307,17 @@ public class DefaultExecutor implements Executor { } throw e; } - streams.start(); - try { - // add the process to the list of those to destroy if the VM exits if (getProcessDestroyer() != null) { getProcessDestroyer().add(process); } - // associate the watchdog with the newly created process if (watchdog != null) { watchdog.start(process); } - int exitValue = Executor.INVALID_EXITVALUE; - try { exitValue = process.waitFor(); } catch (final InterruptedException e) { @@ -240,23 +329,18 @@ public class DefaultExecutor implements Executor { // but we have to do that manually Thread.interrupted(); } - if (watchdog != null) { watchdog.stop(); } - try { streams.stop(); } catch (final IOException e) { setExceptionCaught(e); } - closeProcessStreams(process); - if (getExceptionCaught() != null) { throw getExceptionCaught(); } - if (watchdog != null) { try { watchdog.checkException(); @@ -266,11 +350,9 @@ public class DefaultExecutor implements Executor { throw new IOException(e); } } - if (isFailure(exitValue)) { throw new ExecuteException("Process exited with an error: " + exitValue, exitValue); } - return exitValue; } finally { // remove the process to the list of those to destroy if the VM exits @@ -280,13 +362,6 @@ public class DefaultExecutor implements Executor { } } - @SuppressWarnings("resource") - private void setStreams(final ExecuteStreamHandler streams, final Process process) throws IOException { - streams.setProcessInputStream(process.getOutputStream()); - streams.setProcessOutputStream(process.getInputStream()); - streams.setProcessErrorStream(process.getErrorStream()); - } - /** * Gets the first IOException being thrown. * @@ -318,7 +393,16 @@ public class DefaultExecutor implements Executor { */ @Override public ExecuteStreamHandler getStreamHandler() { - return streamHandler; + return executeStreamHandler; + } + + /** + * Gets the thread factory. Z + * + * @return the thread factory. + */ + ThreadFactory getThreadFactory() { + return threadFactory; } /** @@ -340,7 +424,6 @@ public class DefaultExecutor implements Executor { /** @see org.apache.commons.exec.Executor#isFailure(int) */ @Override public boolean isFailure(final int exitValue) { - if (exitValues == null) { return false; } @@ -365,19 +448,15 @@ public class DefaultExecutor implements Executor { * @throws IOException forwarded from the particular launcher used. */ protected Process launch(final CommandLine command, final Map<String, String> env, final File workingDirectory) throws IOException { - if (launcher == null) { throw new IllegalStateException("CommandLauncher can not be null"); } - - if (workingDirectory != null && !workingDirectory.exists()) { - throw new IOException(workingDirectory + " doesn't exist."); - } + checkWorkingDirectory(workingDirectory); return launcher.exec(command, env, workingDirectory); } /** - * Keep track of the first IOException being thrown. + * Sets the first IOException thrown. * * @param e the IOException. */ @@ -412,7 +491,14 @@ public class DefaultExecutor implements Executor { */ @Override public void setStreamHandler(final ExecuteStreamHandler streamHandler) { - this.streamHandler = streamHandler; + this.executeStreamHandler = streamHandler; + } + + @SuppressWarnings("resource") + private void setStreams(final ExecuteStreamHandler streams, final Process process) throws IOException { + streams.setProcessInputStream(process.getOutputStream()); + streams.setProcessOutputStream(process.getInputStream()); + streams.setProcessErrorStream(process.getErrorStream()); } /** @@ -424,8 +510,12 @@ public class DefaultExecutor implements Executor { } /** + * Sets the working directory. + * * @see org.apache.commons.exec.Executor#setWorkingDirectory(java.io.File) + * @deprecated Use {@link Builder#setWorkingDirectory(File)}. */ + @Deprecated @Override public void setWorkingDirectory(final File workingDirectory) { this.workingDirectory = workingDirectory; diff --git a/src/main/java/org/apache/commons/exec/ExecuteWatchdog.java b/src/main/java/org/apache/commons/exec/ExecuteWatchdog.java index 96590bdb..f21b99c3 100644 --- a/src/main/java/org/apache/commons/exec/ExecuteWatchdog.java +++ b/src/main/java/org/apache/commons/exec/ExecuteWatchdog.java @@ -19,6 +19,9 @@ package org.apache.commons.exec; import java.time.Duration; import java.util.Objects; +import java.util.concurrent.Executors; +import java.util.concurrent.ThreadFactory; +import java.util.function.Supplier; import org.apache.commons.exec.util.DebugUtils; @@ -26,27 +29,72 @@ import org.apache.commons.exec.util.DebugUtils; * Destroys a process running for too long. For example: * * <pre> - * ExecuteWatchdog watchdog = new ExecuteWatchdog(30000); - * Executor executor = new DefaultExecutor(); - * executor.setStreamHandler(new PumpStreamHandler()); + * ExecuteWatchdog watchdog = ExecuteWatchdog.builder().setTimeout(Duration.ofSeconds(30)).get(); + * Executor executor = DefaultExecutor.builder().setExecuteStreamHandler(new PumpStreamHandler()).get(); * executor.setWatchdog(watchdog); * int exitValue = executor.execute(myCommandLine); * if (executor.isFailure(exitValue) && watchdog.killedProcess()) { * // it was killed on purpose by the watchdog * } * </pre> - * + * <p> * When starting an asynchronous process than 'ExecuteWatchdog' is the keeper of the process handle. In some cases it is useful not to define a timeout (and - * pass 'INFINITE_TIMEOUT') and to kill the process explicitly using 'destroyProcess()'. + * pass {@link #INFINITE_TIMEOUT_DURATION}) and to kill the process explicitly using {@link #destroyProcess()}. + * </p> * <p> - * Please note that ExecuteWatchdog is processed asynchronously, e.g. it might be still attached to a process even after the DefaultExecutor.execute has - * returned. + * Please note that ExecuteWatchdog is processed asynchronously, e.g. it might be still attached to a process even after the + * {@link DefaultExecutor#execute(CommandLine)} or a variation has returned. + * </p> * - * @see org.apache.commons.exec.Executor - * @see org.apache.commons.exec.Watchdog + * @see Executor + * @see Watchdog */ public class ExecuteWatchdog implements TimeoutObserver { + /** + * Builds ExecuteWatchdog instances. + * + * @since 1.4.0 + */ + public static final class Builder implements Supplier<ExecuteWatchdog> { + + private ThreadFactory threadFactory; + private Duration timeout; + + /** + * Creates a new configured ExecuteWatchdog. + * + * @return a new configured ExecuteWatchdog. + */ + @Override + public ExecuteWatchdog get() { + return new ExecuteWatchdog(threadFactory, timeout); + } + + /** + * Sets the thread factory. + * + * @param threadFactory the thread factory. + * @return this. + */ + public Builder setThreadFactory(final ThreadFactory threadFactory) { + this.threadFactory = threadFactory; + return this; + } + + /** + * Sets the timeout duration. + * + * @param timeout the timeout duration. + * @return this. + */ + public Builder setTimeout(final Duration timeout) { + this.timeout = timeout; + return this; + } + + } + /** The marker for an infinite timeout. */ public static final long INFINITE_TIMEOUT = -1; @@ -75,18 +123,34 @@ public class ExecuteWatchdog implements TimeoutObserver { private volatile boolean processStarted; /** - * Creates a new watchdog with a given timeout. + * The thread factory. + */ + private final ThreadFactory threadFactory; + + /** + * Creates a new builder. * - * @param timeout the timeout Duration for the process. It must be greater than 0 or {@code INFINITE_TIMEOUT_DURATION}. + * @return a new builder. * @since 1.4.0 */ - public ExecuteWatchdog(final Duration timeout) { + public static Builder builder() { + return new Builder(); + } + + /** + * Creates a new watchdog with a given timeout. + * + * @param threadFactory the thread factory. + * @param timeout the timeout Duration for the process. It must be greater than 0 or {@code INFINITE_TIMEOUT_DURATION}. + */ + private ExecuteWatchdog(final ThreadFactory threadFactory, final Duration timeout) { this.killedProcess = false; this.watch = false; this.hasWatchdog = !INFINITE_TIMEOUT_DURATION.equals(timeout); this.processStarted = false; + this.threadFactory = threadFactory != null ? threadFactory : Executors.defaultThreadFactory(); if (this.hasWatchdog) { - this.watchdog = new Watchdog(timeout); + this.watchdog = Watchdog.builder().setThreadFactory(this.threadFactory).setTimeout(timeout).get(); this.watchdog.addTimeoutObserver(this); } else { this.watchdog = null; @@ -97,11 +161,11 @@ public class ExecuteWatchdog implements TimeoutObserver { * Creates a new watchdog with a given timeout. * * @param timeoutMillis the timeout for the process in milliseconds. It must be greater than 0 or {@code INFINITE_TIMEOUT}. - * @deprecated Use {@link #ExecuteWatchdog(Duration)}. + * @deprecated Use {@link Builder#get()}. */ @Deprecated public ExecuteWatchdog(final long timeoutMillis) { - this(Duration.ofMillis(timeoutMillis)); + this(Executors.defaultThreadFactory(), Duration.ofMillis(timeoutMillis)); } /** diff --git a/src/main/java/org/apache/commons/exec/Executor.java b/src/main/java/org/apache/commons/exec/Executor.java index c84c3ddd..3c4e1736 100644 --- a/src/main/java/org/apache/commons/exec/Executor.java +++ b/src/main/java/org/apache/commons/exec/Executor.java @@ -24,7 +24,7 @@ import java.util.Map; /** * The main abstraction to start an external process. * - * The interface allows to + * The interface allows to: * <ul> * <li>set a current working directory for the subprocess</li> * <li>provide a set of environment variables passed to the subprocess</li> @@ -33,11 +33,11 @@ import java.util.Map; * <li>define a set of expected exit values</li> * <li>terminate any started processes when the main process is terminating using a ProcessDestroyer</li> * </ul> - * + * <p> * The following example shows the basic usage: - * + * </p> * <pre> - * Executor exec = new DefaultExecutor(); + * Executor exec = DefaultExecutor.builder().get(); * CommandLine cl = new CommandLine("ls -l"); * int exitvalue = exec.execute(cl); * </pre> diff --git a/src/main/java/org/apache/commons/exec/PumpStreamHandler.java b/src/main/java/org/apache/commons/exec/PumpStreamHandler.java index 78b248df..80f0d450 100644 --- a/src/main/java/org/apache/commons/exec/PumpStreamHandler.java +++ b/src/main/java/org/apache/commons/exec/PumpStreamHandler.java @@ -23,6 +23,8 @@ import java.io.OutputStream; import java.io.PipedOutputStream; import java.time.Duration; import java.time.Instant; +import java.util.concurrent.Executors; +import java.util.concurrent.ThreadFactory; import org.apache.commons.exec.util.DebugUtils; @@ -40,11 +42,11 @@ public class PumpStreamHandler implements ExecuteStreamHandler { private Thread inputThread; - private final OutputStream out; + private final OutputStream outputStream; - private final OutputStream err; + private final OutputStream errorOutputStream; - private final InputStream input; + private final InputStream inputStream; private InputStreamPumper inputStreamPumper; @@ -54,6 +56,11 @@ public class PumpStreamHandler implements ExecuteStreamHandler { /** The last exception being caught. */ private IOException caught; + /** + * The thread factory. + */ + private final ThreadFactory threadFactory; + /** * Constructs a new {@link PumpStreamHandler}. */ @@ -64,33 +71,46 @@ public class PumpStreamHandler implements ExecuteStreamHandler { /** * Constructs a new {@link PumpStreamHandler}. * - * @param outAndErr the output/error {@link OutputStream}. + * @param allOutputStream the output/error {@link OutputStream}. + */ + public PumpStreamHandler(final OutputStream allOutputStream) { + this(allOutputStream, allOutputStream); + } + + /** + * Constructs a new {@link PumpStreamHandler}. + * + * @param outputStream the output {@link OutputStream}. + * @param errorOutputStream the error {@link OutputStream}. */ - public PumpStreamHandler(final OutputStream outAndErr) { - this(outAndErr, outAndErr); + public PumpStreamHandler(final OutputStream outputStream, final OutputStream errorOutputStream) { + this(outputStream, errorOutputStream, null); } /** * Constructs a new {@link PumpStreamHandler}. * - * @param out the output {@link OutputStream}. - * @param err the error {@link OutputStream}. + * @param outputStream the output {@link OutputStream}. + * @param errorOutputStream the error {@link OutputStream}. + * @param inputStream the input {@link InputStream}. */ - public PumpStreamHandler(final OutputStream out, final OutputStream err) { - this(out, err, null); + public PumpStreamHandler(final OutputStream outputStream, final OutputStream errorOutputStream, final InputStream inputStream) { + this(Executors.defaultThreadFactory(), outputStream, errorOutputStream, inputStream); } /** * Constructs a new {@link PumpStreamHandler}. * - * @param out the output {@link OutputStream}. - * @param err the error {@link OutputStream}. - * @param input the input {@link InputStream}. + * @param outputStream the output {@link OutputStream}. + * @param errorOutputStream the error {@link OutputStream}. + * @param inputStream the input {@link InputStream}. */ - public PumpStreamHandler(final OutputStream out, final OutputStream err, final InputStream input) { - this.out = out; - this.err = err; - this.input = input; + private PumpStreamHandler(final ThreadFactory threadFactory, final OutputStream outputStream, final OutputStream errorOutputStream, + final InputStream inputStream) { + this.threadFactory = threadFactory; + this.outputStream = outputStream; + this.errorOutputStream = errorOutputStream; + this.inputStream = inputStream; } /** @@ -134,9 +154,7 @@ public class PumpStreamHandler implements ExecuteStreamHandler { * @return the stream pumper thread. */ protected Thread createPump(final InputStream is, final OutputStream os, final boolean closeWhenExhausted) { - final Thread result = new Thread(new StreamPumper(is, os, closeWhenExhausted), "Exec Stream Pumper"); - result.setDaemon(true); - return result; + return ThreadUtil.newThread(threadFactory, new StreamPumper(is, os, closeWhenExhausted), "CommonsExecStreamPumper-", true); } /** @@ -148,9 +166,7 @@ public class PumpStreamHandler implements ExecuteStreamHandler { */ private Thread createSystemInPump(final InputStream is, final OutputStream os) { inputStreamPumper = new InputStreamPumper(is, os); - final Thread result = new Thread(inputStreamPumper, "Exec Input Stream Pumper"); - result.setDaemon(true); - return result; + return ThreadUtil.newThread(threadFactory, inputStreamPumper, "CommonsExecStreamPumper-", true); } /** @@ -159,7 +175,7 @@ public class PumpStreamHandler implements ExecuteStreamHandler { * @return {@link OutputStream}. */ protected OutputStream getErr() { - return err; + return errorOutputStream; } /** @@ -168,7 +184,7 @@ public class PumpStreamHandler implements ExecuteStreamHandler { * @return {@link OutputStream}. */ protected OutputStream getOut() { - return out; + return outputStream; } Duration getStopTimeout() { @@ -182,8 +198,8 @@ public class PumpStreamHandler implements ExecuteStreamHandler { */ @Override public void setProcessErrorStream(final InputStream is) { - if (err != null) { - createProcessErrorPump(is, err); + if (errorOutputStream != null) { + createProcessErrorPump(is, errorOutputStream); } } @@ -194,11 +210,11 @@ public class PumpStreamHandler implements ExecuteStreamHandler { */ @Override public void setProcessInputStream(final OutputStream os) { - if (input != null) { - if (input == System.in) { - inputThread = createSystemInPump(input, os); + if (inputStream != null) { + if (inputStream == System.in) { + inputThread = createSystemInPump(inputStream, os); } else { - inputThread = createPump(input, os, true); + inputThread = createPump(inputStream, os, true); } } else { try { @@ -217,8 +233,8 @@ public class PumpStreamHandler implements ExecuteStreamHandler { */ @Override public void setProcessOutputStream(final InputStream is) { - if (out != null) { - createProcessOutputPump(is, out); + if (outputStream != null) { + createProcessOutputPump(is, outputStream); } } @@ -248,14 +264,17 @@ public class PumpStreamHandler implements ExecuteStreamHandler { */ @Override public void start() { - if (outputThread != null) { - outputThread.start(); - } - if (errorThread != null) { - errorThread.start(); - } - if (inputThread != null) { - inputThread.start(); + start(outputThread); + start(errorThread); + start(inputThread); + } + + /** + * Starts the given {@link Thread}. + */ + private void start(final Thread thread) { + if (thread != null) { + thread.start(); } } @@ -264,27 +283,25 @@ public class PumpStreamHandler implements ExecuteStreamHandler { */ @Override public void stop() throws IOException { - if (inputStreamPumper != null) { inputStreamPumper.stopProcessing(); } + stop(outputThread, stopTimeout); + stop(errorThread, stopTimeout); + stop(inputThread, stopTimeout); - stopThread(outputThread, stopTimeout); - stopThread(errorThread, stopTimeout); - stopThread(inputThread, stopTimeout); - - if (err != null && err != out) { + if (errorOutputStream != null && errorOutputStream != outputStream) { try { - err.flush(); + errorOutputStream.flush(); } catch (final IOException e) { final String msg = "Got exception while flushing the error stream : " + e.getMessage(); DebugUtils.handleException(msg, e); } } - if (out != null) { + if (outputStream != null) { try { - out.flush(); + outputStream.flush(); } catch (final IOException e) { final String msg = "Got exception while flushing the output stream"; DebugUtils.handleException(msg, e); @@ -297,13 +314,13 @@ public class PumpStreamHandler implements ExecuteStreamHandler { } /** - * Stops a pumper thread. The implementation actually waits longer than specified in 'timeout' to detect if the timeout was indeed exceeded. If the - * timeout was exceeded an IOException is created to be thrown to the caller. + * Stops a pumper thread. The implementation actually waits longer than specified in 'timeout' to detect if the timeout was indeed exceeded. If the timeout + * was exceeded an IOException is created to be thrown to the caller. * - * @param thread the thread to be stopped. + * @param thread the thread to be stopped. * @param timeout the time in ms to wait to join. */ - private void stopThread(final Thread thread, final Duration timeout) { + private void stop(final Thread thread, final Duration timeout) { if (thread != null) { try { if (timeout.equals(Duration.ZERO)) { @@ -323,13 +340,13 @@ public class PumpStreamHandler implements ExecuteStreamHandler { } /** - * Stops a pumper thread. The implementation actually waits longer than specified in 'timeout' to detect if the timeout was indeed exceeded. If the - * timeout was exceeded an IOException is created to be thrown to the caller. + * Stops a pumper thread. The implementation actually waits longer than specified in 'timeout' to detect if the timeout was indeed exceeded. If the timeout + * was exceeded an IOException is created to be thrown to the caller. * * @param thread the thread to be stopped. * @param timeoutMillis the time in ms to wait to join. */ protected void stopThread(final Thread thread, final long timeoutMillis) { - stopThread(thread, Duration.ofMillis(timeoutMillis)); + stop(thread, Duration.ofMillis(timeoutMillis)); } } diff --git a/src/main/java/org/apache/commons/exec/ThreadUtil.java b/src/main/java/org/apache/commons/exec/ThreadUtil.java new file mode 100644 index 00000000..f524f1df --- /dev/null +++ b/src/main/java/org/apache/commons/exec/ThreadUtil.java @@ -0,0 +1,46 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.commons.exec; + +import java.util.concurrent.ThreadFactory; + +/** + * Internal thread helper. + */ +final class ThreadUtil { + + /** + * Creates a new Thread from the given factory and prefixes it's name with a prefix and sets the daemon flag. + * + * @param threadFactory the thread factory. + * @param runnable The runnable to thread. + * @param prefix the thread name prefix + * @param daemon marks this thread as a daemon thread + * @return constructed thread, or {@code null} if the request to create a thread is rejected + */ + static Thread newThread(final ThreadFactory threadFactory, final Runnable runnable, final String prefix, boolean daemon) { + final Thread thread = threadFactory.newThread(runnable); + if (thread == null) { + throw new IllegalStateException(String.format("The ThreadFactory %s cound not construct a thread for '%s'", threadFactory, prefix)); + } + thread.setName(prefix + thread.getName()); + thread.setDaemon(daemon); + return thread; + } + +} diff --git a/src/main/java/org/apache/commons/exec/Watchdog.java b/src/main/java/org/apache/commons/exec/Watchdog.java index f296cbca..add829aa 100644 --- a/src/main/java/org/apache/commons/exec/Watchdog.java +++ b/src/main/java/org/apache/commons/exec/Watchdog.java @@ -19,6 +19,8 @@ package org.apache.commons.exec; import java.time.Duration; import java.util.Vector; +import java.util.concurrent.ThreadFactory; +import java.util.function.Supplier; /** * Generalization of {@code ExecuteWatchdog}. @@ -27,37 +29,93 @@ import java.util.Vector; */ public class Watchdog implements Runnable { + /** + * Builds ExecuteWatchdog instances. + * + * @since 1.4.0 + */ + public static final class Builder implements Supplier<Watchdog> { + + private ThreadFactory threadFactory; + private Duration timeout; + + /** + * Creates a new configured ExecuteWatchdog. + * + * @return a new configured ExecuteWatchdog. + */ + @Override + public Watchdog get() { + return new Watchdog(threadFactory, timeout); + } + + /** + * Sets the thread factory. + * + * @param threadFactory the thread factory. + * @return this. + */ + public Builder setThreadFactory(final ThreadFactory threadFactory) { + this.threadFactory = threadFactory; + return this; + } + + /** + * Sets the timeout duration. + * + * @param timeout the timeout duration. + * @return this. + */ + public Builder setTimeout(final Duration timeout) { + this.timeout = timeout; + return this; + } + + } + + /** + * Creates a new builder. + * + * @return a new builder. + * @since 1.4.0 + */ + public static Builder builder() { + return new Builder(); + } + private final Vector<TimeoutObserver> observers = new Vector<>(1); private final long timeoutMillis; private boolean stopped; + /** + * The thread factory. + */ + private final ThreadFactory threadFactory; + /** * Constructs a new instance. - * + * @param threadFactory the thread factory. * @param timeout the timeout duration. - * @since 1.4.0 */ - public Watchdog(final Duration timeout) { + private Watchdog(final ThreadFactory threadFactory, final Duration timeout) { if (timeout.isNegative() || Duration.ZERO.equals(timeout)) { throw new IllegalArgumentException("timeout must not be less than 1."); } this.timeoutMillis = timeout.toMillis(); + this.threadFactory = threadFactory; } /** * Constructs a new instance. * * @param timeoutMillis the timeout duration. - * @deprecated Use {@link #Watchdog(Duration)}. + * @deprecated Use {@link #Watchdog(ThreadFactory, Duration)}. */ @Deprecated public Watchdog(final long timeoutMillis) { - if (timeoutMillis < 1) { - throw new IllegalArgumentException("timeout must not be less than 1."); - } - this.timeoutMillis = timeoutMillis; + this(null, Duration.ofMillis(timeoutMillis)); } public void addTimeoutObserver(final TimeoutObserver to) { @@ -98,9 +156,7 @@ public class Watchdog implements Runnable { public synchronized void start() { stopped = false; - final Thread t = new Thread(this, "WATCHDOG"); - t.setDaemon(true); - t.start(); + ThreadUtil.newThread(threadFactory, this, "CommonsExecWatchdog-", true).start(); } public synchronized void stop() { diff --git a/src/main/java/org/apache/commons/exec/environment/DefaultProcessingEnvironment.java b/src/main/java/org/apache/commons/exec/environment/DefaultProcessingEnvironment.java index 807b31ab..0fb8d846 100644 --- a/src/main/java/org/apache/commons/exec/environment/DefaultProcessingEnvironment.java +++ b/src/main/java/org/apache/commons/exec/environment/DefaultProcessingEnvironment.java @@ -204,7 +204,7 @@ public class DefaultProcessingEnvironment { @Deprecated protected BufferedReader runProcEnvCommand() throws IOException { // final ByteArrayOutputStream out = new ByteArrayOutputStream(); -// final Executor exe = new DefaultExecutor(); +// final Executor exe = DefaultExecutor.builder().get(); // exe.setStreamHandler(new PumpStreamHandler(out)); // // ignore the exit value - Just try to use what we got // exe.execute(getProcEnvCommand()); diff --git a/src/site/apt/tutorial.apt b/src/site/apt/tutorial.apt index 1eb36fdb..3437492f 100644 --- a/src/site/apt/tutorial.apt +++ b/src/site/apt/tutorial.apt @@ -48,7 +48,7 @@ Apache Commons Exec +---------------------------------------------------------------------------- String line = "AcroRd32.exe /p /h " + file.getAbsolutePath(); CommandLine cmdLine = CommandLine.parse(line); -DefaultExecutor executor = new DefaultExecutor(); +DefaultExecutor executor = DefaultExecutor.builder().get(); int exitValue = executor.execute(cmdLine); +---------------------------------------------------------------------------- @@ -60,7 +60,7 @@ int exitValue = executor.execute(cmdLine); +---------------------------------------------------------------------------- String line = "AcroRd32.exe /p /h " + file.getAbsolutePath(); CommandLine cmdLine = CommandLine.parse(line); -DefaultExecutor executor = new DefaultExecutor(); +DefaultExecutor executor = DefaultExecutor.builder().get(); executor.setExitValue(1); int exitValue = executor.execute(cmdLine); +---------------------------------------------------------------------------- @@ -76,7 +76,7 @@ int exitValue = executor.execute(cmdLine); +---------------------------------------------------------------------------- String line = "AcroRd32.exe /p /h " + file.getAbsolutePath(); CommandLine cmdLine = CommandLine.parse(line); -DefaultExecutor executor = new DefaultExecutor(); +DefaultExecutor executor = DefaultExecutor.builder().get(); executor.setExitValue(1); ExecuteWatchdog watchdog = new ExecuteWatchdog(60000); executor.setWatchdog(watchdog); @@ -102,7 +102,7 @@ int exitValue = executor.execute(cmdLine); +---------------------------------------------------------------------------- String line = "AcroRd32.exe /p /h \"" + file.getAbsolutePath() + "\""; CommandLine cmdLine = CommandLine.parse(line); -DefaultExecutor executor = new DefaultExecutor(); +DefaultExecutor executor = DefaultExecutor.builder().get(); executor.setExitValue(1); ExecuteWatchdog watchdog = new ExecuteWatchdog(60000); executor.setWatchdog(watchdog); @@ -127,7 +127,7 @@ cmdLine.addArgument("/p"); cmdLine.addArgument("/h"); cmdLine.addArgument("${file}"); cmdLine.setSubstitutionMap(map); -DefaultExecutor executor = new DefaultExecutor(); +DefaultExecutor executor = DefaultExecutor.builder().get(); executor.setExitValue(1); ExecuteWatchdog watchdog = new ExecuteWatchdog(60000); executor.setWatchdog(watchdog); @@ -162,7 +162,7 @@ cmdLine.setSubstitutionMap(map); DefaultExecuteResultHandler resultHandler = new DefaultExecuteResultHandler(); ExecuteWatchdog watchdog = new ExecuteWatchdog(60*1000); -Executor executor = new DefaultExecutor(); +Executor executor = DefaultExecutor.builder().get(); executor.setExitValue(1); executor.setWatchdog(watchdog); executor.execute(cmdLine, resultHandler); diff --git a/src/test/java/org/apache/commons/exec/DefaultExecutorTest.java b/src/test/java/org/apache/commons/exec/DefaultExecutorTest.java index 0f12ce6d..53b0b48b 100644 --- a/src/test/java/org/apache/commons/exec/DefaultExecutorTest.java +++ b/src/test/java/org/apache/commons/exec/DefaultExecutorTest.java @@ -69,7 +69,7 @@ public class DefaultExecutorTest { ERROR_STATUS = statuses[1]; } - private final Executor exec = new DefaultExecutor(); + private final Executor exec = DefaultExecutor.builder().get(); private final File testDir = new File("src/test/scripts"); private final File foreverOutputFile = new File("./target/forever.txt"); @@ -136,10 +136,6 @@ public class DefaultExecutorTest { return result; } - // ====================================================================== - // Start of regression tests - // ====================================================================== - private String readFile(final File file) throws Exception { String text; @@ -251,7 +247,7 @@ public class DefaultExecutorTest { public void testExecuteAsyncNonExistingApplication() throws Exception { final CommandLine cl = new CommandLine(nonExistingTestScript); final DefaultExecuteResultHandler resultHandler = new DefaultExecuteResultHandler(); - final DefaultExecutor executor = new DefaultExecutor(); + final DefaultExecutor executor = DefaultExecutor.builder().get(); executor.execute(cl, resultHandler); resultHandler.waitFor(); @@ -276,7 +272,7 @@ public class DefaultExecutorTest { super.onProcessFailed(e); } }; - final DefaultExecutor executor = new DefaultExecutor(); + final DefaultExecutor executor = DefaultExecutor.builder().get(); executor.setWatchdog(new ExecuteWatchdog(ExecuteWatchdog.INFINITE_TIMEOUT)); executor.execute(cl, resultHandler); @@ -402,7 +398,7 @@ public class DefaultExecutorTest { @Test public void testExecuteNonExistingApplication() throws Exception { final CommandLine cl = new CommandLine(nonExistingTestScript); - final DefaultExecutor executor = new DefaultExecutor(); + final DefaultExecutor executor = DefaultExecutor.builder().get(); assertThrows(IOException.class, () -> executor.execute(cl)); } @@ -413,7 +409,7 @@ public class DefaultExecutorTest { @Test public void testExecuteNonExistingApplicationWithWatchDog() throws Exception { final CommandLine cl = new CommandLine(nonExistingTestScript); - final DefaultExecutor executor = new DefaultExecutor(); + final DefaultExecutor executor = DefaultExecutor.builder().get(); executor.setWatchdog(new ExecuteWatchdog(ExecuteWatchdog.INFINITE_TIMEOUT)); assertThrows(IOException.class, () -> executor.execute(cl)); @@ -432,7 +428,7 @@ public class DefaultExecutorTest { final CommandLine cl = new CommandLine(foreverTestScript); final DefaultExecuteResultHandler handler = new DefaultExecuteResultHandler(); - final DefaultExecutor executor = new DefaultExecutor(); + final DefaultExecutor executor = DefaultExecutor.builder().get(); executor.setWorkingDirectory(new File(".")); executor.setWatchdog(new ExecuteWatchdog(timeout)); @@ -464,7 +460,7 @@ public class DefaultExecutorTest { final long timeout = 10000; final CommandLine cl = new CommandLine(foreverTestScript); - final DefaultExecutor executor = new DefaultExecutor(); + final DefaultExecutor executor = DefaultExecutor.builder().get(); executor.setWorkingDirectory(new File(".")); final ExecuteWatchdog watchdog = new ExecuteWatchdog(timeout); executor.setWatchdog(watchdog); @@ -496,7 +492,7 @@ public class DefaultExecutorTest { final long timeout = Long.MAX_VALUE; final CommandLine cl = new CommandLine(testScript); - final DefaultExecutor executor = new DefaultExecutor(); + final DefaultExecutor executor = DefaultExecutor.builder().get(); executor.setWorkingDirectory(new File(".")); final ExecuteWatchdog watchdog = new ExecuteWatchdog(timeout); executor.setWatchdog(watchdog); @@ -530,7 +526,7 @@ public class DefaultExecutorTest { final CommandLine cl = new CommandLine(printArgsScript); cl.addArgument("gdal_translate"); cl.addArgument("HDF5:\"/home/kk/grass/data/4404.he5\"://HDFEOS/GRIDS/OMI_Column_Amount_O3/Data_Fields/ColumnAmountO3/home/kk/4.tif", false); - final DefaultExecutor executor = new DefaultExecutor(); + final DefaultExecutor executor = DefaultExecutor.builder().get(); final int exitValue = executor.execute(cl); assertFalse(exec.isFailure(exitValue)); } @@ -608,7 +604,7 @@ public class DefaultExecutorTest { public void testExecuteWithNullOutErr() throws Exception { final CommandLine cl = new CommandLine(testScript); final PumpStreamHandler pumpStreamHandler = new PumpStreamHandler(null, null); - final DefaultExecutor executor = new DefaultExecutor(); + final DefaultExecutor executor = DefaultExecutor.builder().get(); executor.setStreamHandler(pumpStreamHandler); final int exitValue = executor.execute(cl); assertFalse(exec.isFailure(exitValue)); @@ -649,7 +645,7 @@ public class DefaultExecutorTest { final FileInputStream fis = new FileInputStream("./NOTICE.txt"); final CommandLine cl = new CommandLine(redirectScript); final PumpStreamHandler pumpStreamHandler = new PumpStreamHandler(baos, baos, fis); - final DefaultExecutor executor = new DefaultExecutor(); + final DefaultExecutor executor = DefaultExecutor.builder().get(); executor.setWorkingDirectory(new File(".")); executor.setStreamHandler(pumpStreamHandler); final int exitValue = executor.execute(cl); @@ -676,7 +672,7 @@ public class DefaultExecutorTest { final CommandLine cl = new CommandLine(testScript); try (OutputStream outAndErr = Files.newOutputStream(outFile)) { final PumpStreamHandler pumpStreamHandler = new PumpStreamHandler(outAndErr); - final DefaultExecutor executor = new DefaultExecutor(); + final DefaultExecutor executor = DefaultExecutor.builder().get(); executor.setStreamHandler(pumpStreamHandler); final int exitValue = executor.execute(cl); assertFalse(exec.isFailure(exitValue)); @@ -715,7 +711,7 @@ public class DefaultExecutorTest { public void testExecuteWithStdOutErr() throws Exception { final CommandLine cl = new CommandLine(testScript); final PumpStreamHandler pumpStreamHandler = new PumpStreamHandler(System.out, System.err); - final DefaultExecutor executor = new DefaultExecutor(); + final DefaultExecutor executor = DefaultExecutor.builder().get(); executor.setStreamHandler(pumpStreamHandler); final int exitValue = executor.execute(cl); assertFalse(exec.isFailure(exitValue)); @@ -749,7 +745,7 @@ public class DefaultExecutorTest { final CommandLine cl = new CommandLine(this.stdinSript); final PumpStreamHandler pumpStreamHandler = new PumpStreamHandler(this.baos, System.err, bais); final DefaultExecuteResultHandler resultHandler = new DefaultExecuteResultHandler(); - final Executor executor = new DefaultExecutor(); + final Executor executor = DefaultExecutor.builder().get(); executor.setStreamHandler(pumpStreamHandler); executor.execute(cl, resultHandler); diff --git a/src/test/java/org/apache/commons/exec/LogOutputStreamTest.java b/src/test/java/org/apache/commons/exec/LogOutputStreamTest.java index 1f084c88..5eecb201 100644 --- a/src/test/java/org/apache/commons/exec/LogOutputStreamTest.java +++ b/src/test/java/org/apache/commons/exec/LogOutputStreamTest.java @@ -25,8 +25,6 @@ import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junitpioneer.jupiter.SetSystemProperty; @@ -62,7 +60,7 @@ public class LogOutputStreamTest { } } - private final Executor exec = new DefaultExecutor(); + private final Executor exec = DefaultExecutor.builder().get(); private final File testDir = new File("src/test/scripts"); private OutputStream systemOut; diff --git a/src/test/java/org/apache/commons/exec/StandAloneTest.java b/src/test/java/org/apache/commons/exec/StandAloneTest.java index add634a7..ea103112 100644 --- a/src/test/java/org/apache/commons/exec/StandAloneTest.java +++ b/src/test/java/org/apache/commons/exec/StandAloneTest.java @@ -20,26 +20,53 @@ package org.apache.commons.exec; import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.File; +import java.util.concurrent.Executors; -import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import org.junitpioneer.jupiter.SetSystemProperty; /** * Placeholder for mailing list question - provided a minimal test case to answer the question as sel-contained regression test. */ +@SetSystemProperty(key = "org.apache.commons.exec.lenient", value = "false") +@SetSystemProperty(key = "org.apache.commons.exec.debug", value = "true") public class StandAloneTest { - @BeforeAll - public static void classSetUp() { - System.setProperty("org.apache.commons.exec.lenient", "false"); - System.setProperty("org.apache.commons.exec.debug", "true"); + @Test + public void testDefaultExecutor() throws Exception { + if (OS.isFamilyUnix()) { + final File testScript = TestUtil.resolveScriptForOS("./src/test/scripts/standalone"); + final Executor exec = new DefaultExecutor(); + exec.setStreamHandler(new PumpStreamHandler()); + final CommandLine cl = new CommandLine(testScript); + exec.execute(cl); + assertTrue(new File("./target/mybackup.gz").exists()); + } } @Test - public void testMe() throws Exception { + public void testDefaultExecutorDefaultBuilder() throws Exception { if (OS.isFamilyUnix()) { final File testScript = TestUtil.resolveScriptForOS("./src/test/scripts/standalone"); - final Executor exec = new DefaultExecutor(); + final Executor exec = DefaultExecutor.builder().get(); + exec.setStreamHandler(new PumpStreamHandler()); + final CommandLine cl = new CommandLine(testScript); + exec.execute(cl); + assertTrue(new File("./target/mybackup.gz").exists()); + } + } + + @Test + public void testDefaultExecutorBuilder() throws Exception { + if (OS.isFamilyUnix()) { + final File testScript = TestUtil.resolveScriptForOS("./src/test/scripts/standalone"); + // @formatter:off + final Executor exec = DefaultExecutor.builder() + .setThreadFactory(Executors.defaultThreadFactory()) + .setExecuteStreamHandler(new PumpStreamHandler()) + .setWorkingDirectory(new File(".")) + .get(); + // @formatter:on exec.setStreamHandler(new PumpStreamHandler()); final CommandLine cl = new CommandLine(testScript); exec.execute(cl); diff --git a/src/test/java/org/apache/commons/exec/TutorialTest.java b/src/test/java/org/apache/commons/exec/TutorialTest.java index a21fc122..87a49123 100644 --- a/src/test/java/org/apache/commons/exec/TutorialTest.java +++ b/src/test/java/org/apache/commons/exec/TutorialTest.java @@ -21,6 +21,7 @@ import static org.junit.jupiter.api.Assertions.fail; import java.io.File; import java.io.IOException; +import java.time.Duration; import java.util.HashMap; import java.util.Map; @@ -80,7 +81,7 @@ public class TutorialTest { * @return a print result handler (implementing a future) * @throws IOException the test failed */ - public PrintResultHandler print(final File file, final long printJobTimeout, final boolean printInBackground) throws IOException { + public PrintResultHandler print(final File file, final Duration printJobTimeout, final boolean printInBackground) throws IOException { int exitValue; ExecuteWatchdog watchdog = null; @@ -96,12 +97,12 @@ public class TutorialTest { commandLine.setSubstitutionMap(map); // create the executor and consider the exitValue '1' as success - final Executor executor = new DefaultExecutor(); + final Executor executor = DefaultExecutor.builder().get(); executor.setExitValue(1); // create a watchdog if requested - if (printJobTimeout > 0) { - watchdog = new ExecuteWatchdog(printJobTimeout); + if (printJobTimeout.toMillis() > 0) { + watchdog = ExecuteWatchdog.builder().setTimeout(printJobTimeout).get(); executor.setWatchdog(watchdog); } @@ -122,7 +123,7 @@ public class TutorialTest { @Test public void testTutorialExample() throws Exception { - final long printJobTimeout = 15000; + final Duration printJobTimeout = Duration.ofSeconds(15); final boolean printInBackground = false; final File pdfFile = new File("/Documents and Settings/foo.pdf"); diff --git a/src/test/java/org/apache/commons/exec/issues/Exec33Test.java b/src/test/java/org/apache/commons/exec/issues/Exec33Test.java index 16a24b2f..446cf8e6 100644 --- a/src/test/java/org/apache/commons/exec/issues/Exec33Test.java +++ b/src/test/java/org/apache/commons/exec/issues/Exec33Test.java @@ -35,7 +35,7 @@ import org.junit.jupiter.api.Test; */ public class Exec33Test { - private final Executor exec = new DefaultExecutor(); + private final Executor exec = DefaultExecutor.builder().get(); private final File testDir = new File("src/test/scripts"); private final File testScript = TestUtil.resolveScriptForOS(testDir + "/test"); @@ -43,7 +43,7 @@ public class Exec33Test { public void testExec33() throws Exception { final CommandLine cl = new CommandLine(testScript); final PumpStreamHandler pumpStreamHandler = new PumpStreamHandler(System.out, System.err, System.in); - final DefaultExecutor executor = new DefaultExecutor(); + final DefaultExecutor executor = DefaultExecutor.builder().get(); executor.setStreamHandler(pumpStreamHandler); final int exitValue = executor.execute(cl); assertFalse(exec.isFailure(exitValue)); diff --git a/src/test/java/org/apache/commons/exec/issues/Exec34Test.java b/src/test/java/org/apache/commons/exec/issues/Exec34Test.java index 657d55ab..2f7e8302 100644 --- a/src/test/java/org/apache/commons/exec/issues/Exec34Test.java +++ b/src/test/java/org/apache/commons/exec/issues/Exec34Test.java @@ -36,7 +36,7 @@ import org.junit.jupiter.api.Test; */ public class Exec34Test { - private final Executor exec = new DefaultExecutor(); + private final Executor exec = DefaultExecutor.builder().get(); private final File testDir = new File("src/test/scripts"); private final File pingScript = TestUtil.resolveScriptForOS(testDir + "/ping"); diff --git a/src/test/java/org/apache/commons/exec/issues/Exec36Test.java b/src/test/java/org/apache/commons/exec/issues/Exec36Test.java index 8aaed1f2..1a43100e 100644 --- a/src/test/java/org/apache/commons/exec/issues/Exec36Test.java +++ b/src/test/java/org/apache/commons/exec/issues/Exec36Test.java @@ -41,7 +41,7 @@ import org.junit.jupiter.api.Test; */ public class Exec36Test { - private final Executor exec = new DefaultExecutor(); + private final Executor exec = DefaultExecutor.builder().get(); private final File testDir = new File("src/test/scripts"); private final File printArgsScript = TestUtil.resolveScriptForOS(testDir + "/printargs"); diff --git a/src/test/java/org/apache/commons/exec/issues/Exec41Test.java b/src/test/java/org/apache/commons/exec/issues/Exec41Test.java index 95a3e188..1fa4ab78 100644 --- a/src/test/java/org/apache/commons/exec/issues/Exec41Test.java +++ b/src/test/java/org/apache/commons/exec/issues/Exec41Test.java @@ -52,7 +52,7 @@ public class Exec41Test { final CommandLine cmdLine = new CommandLine(pingScript); cmdLine.addArgument("10"); // sleep 10 seconds - final DefaultExecutor executor = new DefaultExecutor(); + final DefaultExecutor executor = DefaultExecutor.builder().get(); final ExecuteWatchdog watchdog = new ExecuteWatchdog(2 * 1000); // allow process no more than 2 seconds // create a custom "PumpStreamHandler" doing no pumping at all @@ -106,7 +106,7 @@ public class Exec41Test { return; } - final DefaultExecutor executor = new DefaultExecutor(); + final DefaultExecutor executor = DefaultExecutor.builder().get(); final ExecuteWatchdog watchdog = new ExecuteWatchdog(2 * 1000); // allow process no more than 2 seconds final PumpStreamHandler pumpStreamHandler = new PumpStreamHandler(System.out, System.err); // this method was part of the patch I reverted diff --git a/src/test/java/org/apache/commons/exec/issues/Exec44Test.java b/src/test/java/org/apache/commons/exec/issues/Exec44Test.java index add83c82..d4c19a24 100644 --- a/src/test/java/org/apache/commons/exec/issues/Exec44Test.java +++ b/src/test/java/org/apache/commons/exec/issues/Exec44Test.java @@ -35,7 +35,7 @@ import org.junit.jupiter.api.Test; */ public class Exec44Test { - private final Executor exec = new DefaultExecutor(); + private final Executor exec = DefaultExecutor.builder().get(); private final File testDir = new File("src/test/scripts"); private final File foreverTestScript = TestUtil.resolveScriptForOS(testDir + "/forever"); diff --git a/src/test/java/org/apache/commons/exec/issues/Exec49Test.java b/src/test/java/org/apache/commons/exec/issues/Exec49Test.java index 544afa9b..aa26e85a 100644 --- a/src/test/java/org/apache/commons/exec/issues/Exec49Test.java +++ b/src/test/java/org/apache/commons/exec/issues/Exec49Test.java @@ -36,7 +36,7 @@ import org.junit.jupiter.api.Test; public class Exec49Test { private static final Duration WAIT = Duration.ofSeconds(10); - private final Executor exec = new DefaultExecutor(); + private final Executor exec = DefaultExecutor.builder().get(); /** * The issue was detected when trying to capture stdout/stderr with a PipedOutputStream and then pass that to a PipedInputStream. The following code will diff --git a/src/test/java/org/apache/commons/exec/issues/Exec57Test.java b/src/test/java/org/apache/commons/exec/issues/Exec57Test.java index 5931ecf3..5d796bf5 100644 --- a/src/test/java/org/apache/commons/exec/issues/Exec57Test.java +++ b/src/test/java/org/apache/commons/exec/issues/Exec57Test.java @@ -48,7 +48,7 @@ public class Exec57Test extends AbstractExecTest { public void testExecutionOfBackgroundProcess() throws IOException { final CommandLine cmdLine = new CommandLine("sh").addArgument("-c").addArgument("./src/test/scripts/issues/exec-57-nohup.sh", false); - final DefaultExecutor executor = new DefaultExecutor(); + final DefaultExecutor executor = DefaultExecutor.builder().get(); final PumpStreamHandler pumpStreamHandler = new PumpStreamHandler(System.out, System.err); executor.setStreamHandler(pumpStreamHandler); @@ -71,7 +71,7 @@ public class Exec57Test extends AbstractExecTest { } final CommandLine cmdLine = new CommandLine("sh").addArgument("-c").addArgument("./src/test/scripts/issues/exec-57-detached.sh", false); - final DefaultExecutor executor = new DefaultExecutor(); + final DefaultExecutor executor = DefaultExecutor.builder().get(); final PumpStreamHandler pumpStreamHandler = new PumpStreamHandler(System.out, System.err); executor.setStreamHandler(pumpStreamHandler); diff --git a/src/test/java/org/apache/commons/exec/issues/Exec60Test.java b/src/test/java/org/apache/commons/exec/issues/Exec60Test.java index fd6ec048..5c4f2a47 100644 --- a/src/test/java/org/apache/commons/exec/issues/Exec60Test.java +++ b/src/test/java/org/apache/commons/exec/issues/Exec60Test.java @@ -35,7 +35,7 @@ import org.junit.jupiter.api.Test; */ public class Exec60Test extends AbstractExecTest { - private final Executor exec = new DefaultExecutor(); + private final Executor exec = DefaultExecutor.builder().get(); private final File pingScript = resolveTestScript("ping"); /** diff --git a/src/test/java/org/apache/commons/exec/issues/Exec62Test.java b/src/test/java/org/apache/commons/exec/issues/Exec62Test.java index 6d46d83d..b21cdd30 100644 --- a/src/test/java/org/apache/commons/exec/issues/Exec62Test.java +++ b/src/test/java/org/apache/commons/exec/issues/Exec62Test.java @@ -49,7 +49,7 @@ public class Exec62Test { commandLine.addArgument(testScript.getAbsolutePath()); - final DefaultExecutor executor = new DefaultExecutor(); + final DefaultExecutor executor = DefaultExecutor.builder().get(); executor.setExitValues(null); // ignore exit values executor.setWatchdog(watchdog); diff --git a/src/test/java/org/apache/commons/exec/issues/Exec65Test.java b/src/test/java/org/apache/commons/exec/issues/Exec65Test.java index 04ecad7b..829c158f 100644 --- a/src/test/java/org/apache/commons/exec/issues/Exec65Test.java +++ b/src/test/java/org/apache/commons/exec/issues/Exec65Test.java @@ -55,7 +55,7 @@ public class Exec65Test extends AbstractExecTest { @Timeout(value = TEST_TIMEOUT, unit = TimeUnit.MILLISECONDS) public void testExec65WithSleepUsingShellScript() throws Exception { assumeTrue(OS.isFamilyMac()); - final DefaultExecutor executor = new DefaultExecutor(); + final DefaultExecutor executor = DefaultExecutor.builder().get(); executor.setStreamHandler(new PumpStreamHandler(System.out, System.err)); executor.setWatchdog(new ExecuteWatchdog(WATCHDOG_TIMEOUT)); final CommandLine command = new CommandLine(resolveTestScript("sleep")); @@ -94,7 +94,7 @@ public class Exec65Test extends AbstractExecTest { if (!OS.isFamilyUnix()) { throw new ExecuteException(testNotSupportedForCurrentOperatingSystem(), 0); } - final DefaultExecutor executor = new DefaultExecutor(); + final DefaultExecutor executor = DefaultExecutor.builder().get(); executor.setStreamHandler(new PumpStreamHandler(System.out, System.err, System.in)); executor.setWatchdog(new ExecuteWatchdog(WATCHDOG_TIMEOUT)); final CommandLine command = new CommandLine(resolveTestScript("issues", "exec-65")); @@ -110,7 +110,7 @@ public class Exec65Test extends AbstractExecTest { throw new ExecuteException(testNotSupportedForCurrentOperatingSystem(), 0); } final ExecuteWatchdog watchdog = new ExecuteWatchdog(WATCHDOG_TIMEOUT); - final DefaultExecutor executor = new DefaultExecutor(); + final DefaultExecutor executor = DefaultExecutor.builder().get(); final CommandLine command = new CommandLine("sleep"); command.addArgument("60"); executor.setStreamHandler(new PumpStreamHandler(System.out, System.err));
