You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by si...@apache.org on 2011/11/30 08:57:30 UTC
svn commit: r1208315 - in /commons/proper/exec/trunk/src:
changes/changes.xml main/java/org/apache/commons/exec/DefaultExecutor.java
main/java/org/apache/commons/exec/ExecuteWatchdog.java
test/java/org/apache/commons/exec/DefaultExecutorTest.java
Author: simonetripodi
Date: Wed Nov 30 07:57:29 2011
New Revision: 1208315
URL: http://svn.apache.org/viewvc?rev=1208315&view=rev
Log:
[EXEC-34] Race condition prevent watchdog working using ExecuteStreamHandler - Patch submitted by Kristian Rosenvold
Modified:
commons/proper/exec/trunk/src/changes/changes.xml
commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/DefaultExecutor.java
commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/ExecuteWatchdog.java
commons/proper/exec/trunk/src/test/java/org/apache/commons/exec/DefaultExecutorTest.java
Modified: commons/proper/exec/trunk/src/changes/changes.xml
URL: http://svn.apache.org/viewvc/commons/proper/exec/trunk/src/changes/changes.xml?rev=1208315&r1=1208314&r2=1208315&view=diff
==============================================================================
--- commons/proper/exec/trunk/src/changes/changes.xml (original)
+++ commons/proper/exec/trunk/src/changes/changes.xml Wed Nov 30 07:57:29 2011
@@ -45,6 +45,10 @@
When encountering a PipedOutputStream we will automatically close it to avoid
the exception.
</action>
+ <action issue="EXEC-34" dev="simonetripodi" type="fix" date="2011-11-30" due-to="Marco Ferrante">
+ Race condition prevent watchdog working using ExecuteStreamHandler.
+ Patch submitted by Kristian Rosenvold.
+ </action>
</release>
<release version="1.1" date="2010-10-08" description="Maintenance Release">
<action dev="sebb" type="fix" date="2010-10-05" >
Modified: commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/DefaultExecutor.java
URL: http://svn.apache.org/viewvc/commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/DefaultExecutor.java?rev=1208315&r1=1208314&r2=1208315&view=diff
==============================================================================
--- commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/DefaultExecutor.java (original)
+++ commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/DefaultExecutor.java Wed Nov 30 07:57:29 2011
@@ -185,6 +185,10 @@ public class DefaultExecutor implements
throw new IOException(workingDirectory + " doesn't exist.");
}
+ if (watchdog != null) {
+ watchdog.setProcessNotStarted();
+ }
+
Runnable runnable = new Runnable()
{
public void run()
Modified: commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/ExecuteWatchdog.java
URL: http://svn.apache.org/viewvc/commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/ExecuteWatchdog.java?rev=1208315&r1=1208314&r2=1208315&view=diff
==============================================================================
--- commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/ExecuteWatchdog.java (original)
+++ commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/ExecuteWatchdog.java Wed Nov 30 07:57:29 2011
@@ -68,6 +68,9 @@ public class ExecuteWatchdog implements
/** Will tell us whether timeout has occurred. */
private final Watchdog watchdog;
+ /** Indicates that the process is verified as started */
+ private volatile boolean processStarted;
+
/**
* Creates a new watchdog with a given timeout.
*
@@ -79,6 +82,7 @@ public class ExecuteWatchdog implements
this.killedProcess = false;
this.watch = false;
this.hasWatchdog = (timeout != INFINITE_TIMEOUT);
+ this.processStarted = false;
if(this.hasWatchdog) {
this.watchdog = new Watchdog(timeout);
this.watchdog.addTimeoutObserver(this);
@@ -108,6 +112,8 @@ public class ExecuteWatchdog implements
this.killedProcess = false;
this.watch = true;
this.process = process;
+ this.processStarted = true;
+ this.notifyAll();
if(this.hasWatchdog) {
watchdog.start();
}
@@ -129,6 +135,7 @@ public class ExecuteWatchdog implements
* Destroys the running process manually.
*/
public synchronized void destroyProcess() {
+ ensureStarted();
this.timeoutOccured(null);
this.stop();
}
@@ -184,6 +191,7 @@ public class ExecuteWatchdog implements
* <tt>false</tt>.
*/
public synchronized boolean isWatching() {
+ ensureStarted();
return watch;
}
@@ -203,5 +211,23 @@ public class ExecuteWatchdog implements
protected synchronized void cleanUp() {
watch = false;
process = null;
- }
+ }
+
+ void setProcessNotStarted(){
+ processStarted = false;
+ }
+
+ /**
+ * Ensures that the process is started, so we do not race with asynch execution.
+ * The caller of this method must be holding the lock on this
+ */
+ private void ensureStarted(){
+ while (!processStarted){
+ try {
+ this.wait();
+ } catch (InterruptedException e) {
+ throw new RuntimeException(e.getMessage());
+ }
+ }
+ }
}
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=1208315&r1=1208314&r2=1208315&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 Wed Nov 30 07:57:29 2011
@@ -669,9 +669,6 @@ public class DefaultExecutorTest extends
DefaultExecuteResultHandler handler = new DefaultExecuteResultHandler();
exec.setWatchdog(watchdog);
exec.execute(cmdLine, handler);
- // if you comment out the next line the test will fail
- Thread.sleep(2000);
- // terminate it
assertTrue(watchdog.isWatching());
watchdog.destroyProcess();
assertTrue("Watchdog should have killed the process",watchdog.killedProcess());