You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by sg...@apache.org on 2010/09/20 21:19:18 UTC
svn commit: r999068 - in /commons/proper/exec/trunk/src:
main/java/org/apache/commons/exec/DefaultExecuteResultHandler.java
test/java/org/apache/commons/exec/DefaultExecutorTest.java
Author: sgoeschl
Date: Mon Sep 20 19:19:18 2010
New Revision: 999068
URL: http://svn.apache.org/viewvc?rev=999068&view=rev
Log:
Added a testExecuteWatchdogAsync() test
Modified:
commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/DefaultExecuteResultHandler.java
commons/proper/exec/trunk/src/test/java/org/apache/commons/exec/DefaultExecutorTest.java
Modified: commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/DefaultExecuteResultHandler.java
URL: http://svn.apache.org/viewvc/commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/DefaultExecuteResultHandler.java?rev=999068&r1=999067&r2=999068&view=diff
==============================================================================
--- commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/DefaultExecuteResultHandler.java (original)
+++ commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/DefaultExecuteResultHandler.java Mon Sep 20 19:19:18 2010
@@ -24,31 +24,41 @@ package org.apache.commons.exec;
*/
public class DefaultExecuteResultHandler implements ExecuteResultHandler {
- private static final int SLEEP_TIME_MS = 100;
+ /** the interval polling the result */
+ private static final int SLEEP_TIME_MS = 50;
/** Keep track if the process is still running */
- private boolean hasResult;
+ private volatile boolean hasResult;
/** The exit value of the finished process */
- private int exitValue;
+ private volatile int exitValue;
/** Any offending exception */
- private ExecuteException exception;
+ private volatile ExecuteException exception;
+
+ /**
+ * Constructor.
+ */
+ public DefaultExecuteResultHandler() {
+ this.hasResult = false;
+ this.exitValue = Executor.INVALID_EXITVALUE;
+ }
/**
* @see org.apache.commons.exec.ExecuteResultHandler#onProcessComplete(int)
*/
- synchronized public void onProcessComplete(int exitValue) {
+ public void onProcessComplete(int exitValue) {
this.exitValue = exitValue;
+ this.exception = null;
this.hasResult = true;
}
/**
* @see org.apache.commons.exec.ExecuteResultHandler#onProcessFailed(org.apache.commons.exec.ExecuteException)
*/
- synchronized public void onProcessFailed(ExecuteException e) {
+ public void onProcessFailed(ExecuteException e) {
+ this.exitValue = e.getExitValue();
this.exception = e;
- exitValue = e.getExitValue();
this.hasResult = true;
}
@@ -58,8 +68,12 @@ public class DefaultExecuteResultHandler
* @return Returns the exception.
* @throws IllegalStateException if the process has not exited yet
*/
- synchronized public ExecuteException getException() {
- if(!hasResult) throw new IllegalStateException("The process has not exited yet therefore no result is available ...");
+ public ExecuteException getException() {
+
+ if(!hasResult) {
+ throw new IllegalStateException("The process has not exited yet therefore no result is available ...");
+ }
+
return exception;
}
@@ -69,17 +83,21 @@ public class DefaultExecuteResultHandler
* @return Returns the exitValue.
* @throws IllegalStateException if the process has not exited yet
*/
- synchronized public int getExitValue() {
- if(!hasResult) throw new IllegalStateException("The process has not exited yet therefore no result is available ...");
+ public int getExitValue() {
+
+ if(!hasResult) {
+ throw new IllegalStateException("The process has not exited yet therefore no result is available ...");
+ }
+
return exitValue;
}
/**
- * Has the process exited and a result is available?
+ * Has the process exited and a result is available, i.e. exitCode or exception?
*
* @return true if a result of the execution is available
*/
- synchronized public boolean hasResult() {
+ public boolean hasResult() {
return hasResult;
}
@@ -90,24 +108,37 @@ public class DefaultExecuteResultHandler
* not yet terminated, the calling thread will be blocked until the
* process exits.
*
- * @return the exit value of the process.
* @exception InterruptedException if the current thread is
* {@linkplain Thread#interrupt() interrupted} by another
* thread while it is waiting, then the wait is ended and
* an {@link InterruptedException} is thrown.
- * @exception ExecuteException re-thrown exception if the process
- * execution has failed due to ExecuteException
*/
- public int waitFor() throws InterruptedException, ExecuteException {
- while (!this.hasResult()) {
+ public void waitFor() throws InterruptedException {
+
+ while (!hasResult()) {
Thread.sleep(SLEEP_TIME_MS);
}
+ }
- if(getException() == null) {
- return getExitValue();
- }
- else {
- throw getException();
+ /**
+ * Causes the current thread to wait, if necessary, until the
+ * process has terminated. This method returns immediately if
+ * the process has already terminated. If the process has
+ * not yet terminated, the calling thread will be blocked until the
+ * process exits.
+ *
+ * @param timeout the maximum time to wait in milliseconds
+ * @exception InterruptedException if the current thread is
+ * {@linkplain Thread#interrupt() interrupted} by another
+ * thread while it is waiting, then the wait is ended and
+ * an {@link InterruptedException} is thrown.
+ */
+ public void waitFor(long timeout) throws InterruptedException {
+
+ long until = System.currentTimeMillis() + timeout;
+
+ while (!hasResult() && (System.currentTimeMillis() < until)) {
+ Thread.sleep(SLEEP_TIME_MS);
}
}
}
\ No newline at end of file
Modified: commons/proper/exec/trunk/src/test/java/org/apache/commons/exec/DefaultExecutorTest.java
URL: http://svn.apache.org/viewvc/commons/proper/exec/trunk/src/test/java/org/apache/commons/exec/DefaultExecutorTest.java?rev=999068&r1=999067&r2=999068&view=diff
==============================================================================
--- commons/proper/exec/trunk/src/test/java/org/apache/commons/exec/DefaultExecutorTest.java (original)
+++ commons/proper/exec/trunk/src/test/java/org/apache/commons/exec/DefaultExecutorTest.java Mon Sep 20 19:19:18 2010
@@ -84,6 +84,10 @@ public class DefaultExecutorTest extends
this.baos.close();
}
+ // ======================================================================
+ // Start of regression tests
+ // ======================================================================
+
/**
* The simplest possible test - start a script and
* check that the output was pumped into our
@@ -158,29 +162,38 @@ public class DefaultExecutorTest extends
assertFalse(exec.isFailure(exitValue));
}
+ /**
+ * Start a asynchronous process which returns an success
+ * exit value.
+ *
+ * @throws Exception the test failed
+ */
public void testExecuteAsync() throws Exception {
CommandLine cl = new CommandLine(testScript);
DefaultExecuteResultHandler resultHandler = new DefaultExecuteResultHandler();
exec.execute(cl, resultHandler);
- resultHandler.waitFor();
+ resultHandler.waitFor(2000);
+ assertTrue(resultHandler.hasResult());
+ assertNull(resultHandler.getException());
assertFalse(exec.isFailure(resultHandler.getExitValue()));
assertEquals("FOO..", baos.toString().trim());
}
+ /**
+ * Start a asynchronous process which returns an error
+ * exit value.
+ *
+ * @throws Exception the test failed
+ */
public void testExecuteAsyncWithError() throws Exception {
CommandLine cl = new CommandLine(errorTestScript);
DefaultExecuteResultHandler resultHandler = new DefaultExecuteResultHandler();
exec.execute(cl, resultHandler);
- try {
- resultHandler.waitFor();
- }
- catch(ExecuteException e) {
- assertTrue(exec.isFailure(e.getExitValue()));
- assertNotNull(resultHandler.getException());
- assertEquals("FOO..", baos.toString().trim());
- return;
- }
- fail("Expecting an ExecuteException");
+ resultHandler.waitFor(2000);
+ assertTrue(resultHandler.hasResult());
+ assertTrue(exec.isFailure(resultHandler.getExitValue()));
+ assertNotNull(resultHandler.getException());
+ assertEquals("FOO..", baos.toString().trim());
}
/**
@@ -198,14 +211,18 @@ public class DefaultExecutorTest extends
// wait for script to run
Thread.sleep(2000);
assertTrue("Watchdog should watch the process", watchdog.isWatching());
- // terminate it using the watchdog
+ // terminate it manually using the watchdog
watchdog.destroyProcess();
+ // wait until the result of the process execution is propagated
+ handler.waitFor();
assertTrue("Watchdog should have killed the process", watchdog.killedProcess());
- assertFalse(watchdog.isWatching());
+ assertFalse("Watchdog is no longer watching the process", watchdog.isWatching());
+ assertTrue("ResultHandler received a result", handler.hasResult());
+ assertNotNull("ResultHandler received an exception as result", handler.getException());
}
/**
- * Start a async process and try to terminate it manually but
+ * Start a asynchronous process and try to terminate it manually but
* the process was already terminated by the watchdog. This is
* basically a race condition between infrastructure and user
* code.
@@ -214,20 +231,24 @@ public class DefaultExecutorTest extends
*/
public void testExecuteAsyncWithTooLateUserTermination() throws Exception {
CommandLine cl = new CommandLine(foreverTestScript);
+ DefaultExecuteResultHandler handler = new DefaultExecuteResultHandler();
ExecuteWatchdog watchdog = new ExecuteWatchdog(3000);
exec.setWatchdog(watchdog);
- DefaultExecuteResultHandler handler = new DefaultExecuteResultHandler();
exec.execute(cl, handler);
// wait for script to be terminated by the watchdog
Thread.sleep(6000);
// try to terminate the already terminated process
watchdog.destroyProcess();
- assertTrue("Watchdog should have killed the process already",watchdog.killedProcess());
- assertFalse(watchdog.isWatching());
+ // wait until the result of the process execution is propagated
+ handler.waitFor();
+ assertTrue("Watchdog should have killed the process already", watchdog.killedProcess());
+ assertFalse("Watchdog is no longer watching the process", watchdog.isWatching());
+ assertTrue("ResultHandler received a result", handler.hasResult());
+ assertNotNull("ResultHandler received an exception as result", handler.getException());
}
/**
- * Start a script looping forever and check if the ExecuteWatchdog
+ * Start a script looping forever (synchronously) and check if the ExecuteWatchdog
* kicks in killing the run away process. To make killing a process
* more testable the "forever" scripts write each second a '.'
* into "./target/forever.txt" (a marker file). After a test run
@@ -235,7 +256,7 @@ public class DefaultExecutorTest extends
*
* @throws Exception the test failed
*/
- public void testExecuteWatchdog() throws Exception {
+ public void testExecuteWatchdogSync() throws Exception {
long timeout = 10000;
@@ -251,8 +272,8 @@ public class DefaultExecutorTest extends
catch(ExecuteException e) {
Thread.sleep(timeout);
int nrOfInvocations = getOccurrences(readFile(this.foreverOutputFile), '.');
- assertTrue("killing the subprocess did not work : " + nrOfInvocations, nrOfInvocations > 5 && nrOfInvocations <= 11);
assertTrue( executor.getWatchdog().killedProcess() );
+ assertTrue("killing the subprocess did not work : " + nrOfInvocations, nrOfInvocations > 5 && nrOfInvocations <= 11);
return;
}
catch(Throwable t) {
@@ -264,6 +285,35 @@ public class DefaultExecutorTest extends
}
/**
+ * Start a script looping forever (asynchronously) and check if the
+ * ExecuteWatchdog kicks in killing the run away process. To make killing
+ * a process more testable the "forever" scripts write each second a '.'
+ * into "./target/forever.txt" (a marker file). After a test run
+ * we should have a few dots in there.
+ *
+ * @throws Exception the test failed
+ */
+ public void testExecuteWatchdogAsync() throws Exception {
+
+ long timeout = 10000;
+
+ CommandLine cl = new CommandLine(foreverTestScript);
+ DefaultExecuteResultHandler handler = new DefaultExecuteResultHandler();
+ DefaultExecutor executor = new DefaultExecutor();
+ executor.setWorkingDirectory(new File("."));
+ executor.setWatchdog(new ExecuteWatchdog(timeout));
+
+ executor.execute(cl, handler);
+ handler.waitFor();
+
+ assertTrue("Killed process should be true", executor.getWatchdog().killedProcess() );
+ int nrOfInvocations = getOccurrences(readFile(this.foreverOutputFile), '.');
+ assertTrue("Killing the process did not work : " + nrOfInvocations, nrOfInvocations > 5 && nrOfInvocations <= 11);
+ assertTrue("ResultHandler received a result", handler.hasResult());
+ assertNotNull("ResultHandler received an exception as result", handler.getException());
+ }
+
+ /**
* Try to start an non-existing application which should result
* in an exception.
*
@@ -279,7 +329,7 @@ public class DefaultExecutorTest extends
// expected
return;
}
- fail("Got no exception when executing an non-existing application");
+ fail("Got no exception when executing an non-existing application");
}
/**
@@ -382,15 +432,8 @@ public class DefaultExecutorTest extends
// terminate it and the process destroyer is detached
watchdog.destroyProcess();
assertTrue(watchdog.killedProcess());
-
- try {
- handler.waitFor();
- fail("Expecting an ExecutionException");
- }
- catch(ExecuteException e) {
- // nothing to do
- }
-
+ handler.waitFor();
+ assertNotNull(handler.getException());
assertEquals("Processor Destroyer size should be 0", 0, processDestroyer.size());
assertFalse("Process destroyer should not exist as shutdown hook", processDestroyer.isAddedAsShutdownHook());
}
@@ -853,7 +896,7 @@ public class DefaultExecutorTest extends
*
* @throws Exception the test failed
*/
- public void testExecuteStability() throws Exception {
+ public void _testExecuteStability() throws Exception {
// make a plain-vanilla test
for(int i=0; i<100; i++) {
@@ -875,12 +918,8 @@ public class DefaultExecutorTest extends
ExecuteWatchdog watchdog = new ExecuteWatchdog(500);
exec.setWatchdog(watchdog);
exec.execute(cl, env, resultHandler);
- try {
- resultHandler.waitFor();
- }
- catch(ExecuteException e) {
- // nothing to do
- }
+ resultHandler.waitFor();
+ assertNotNull(resultHandler.getException());
baos.reset();
}
}