commons-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From simonetrip...@apache.org
Subject 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
Date Wed, 30 Nov 2011 07:57:30 GMT
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());



Mime
View raw message