commons-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sgoes...@apache.org
Subject svn commit: r1180579 - in /commons/proper/exec/trunk/src: changes/changes.xml main/java/org/apache/commons/exec/Watchdog.java test/java/org/apache/commons/exec/DefaultExecutorTest.java
Date Sun, 09 Oct 2011 10:40:56 GMT
Author: sgoeschl
Date: Sun Oct  9 10:40:55 2011
New Revision: 1180579

URL: http://svn.apache.org/viewvc?rev=1180579&view=rev
Log:
[EXEX-60] - Fixed dead lock by calling the timeout observers outside of the synchronized block

Modified:
    commons/proper/exec/trunk/src/changes/changes.xml
    commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/Watchdog.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=1180579&r1=1180578&r2=1180579&view=diff
==============================================================================
--- commons/proper/exec/trunk/src/changes/changes.xml (original)
+++ commons/proper/exec/trunk/src/changes/changes.xml Sun Oct  9 10:40:55 2011
@@ -24,6 +24,11 @@
     </properties>
     <body>
         <release version="1.1.1-SNAPSHOT" date="TBA" description="Bugfixing Release">
+            <action issue="EXEC-60" dev="sgoeschl" type="fix" date="2011-10-09"  due-to="Peter
Kofler">
+                Fixed dead lock by calling the timeout observers outside of the synchronized
block thereby
+                removing on pre-requisite of a deadlock. Also added a test case to demonstrate
that this
+                problem is fixed (which of course can not guarantee the absence of a dead
lock).
+            </action>
             <action issue="EXEC-55" dev="sgoeschl" type="add" date="2011-02-21"  due-to="Dominik
Stadler">
                 Set names for started threads.
             </action>

Modified: commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/Watchdog.java
URL: http://svn.apache.org/viewvc/commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/Watchdog.java?rev=1180579&r1=1180578&r2=1180579&view=diff
==============================================================================
--- commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/Watchdog.java (original)
+++ commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/Watchdog.java Sun Oct
 9 10:40:55 2011
@@ -68,17 +68,26 @@ public class Watchdog implements Runnabl
         notifyAll();
     }
 
-    public synchronized void run() {
-        final long until = System.currentTimeMillis() + timeout;
-        long now;
-        while (!stopped && until > (now = System.currentTimeMillis())) {
-            try {
-                wait(until - now);
-            } catch (InterruptedException e) {
-            }
-        }
-        if (!stopped) {
-            fireTimeoutOccured();
-        }
-    }
+    public void run() {
+       final long until = System.currentTimeMillis() + timeout;
+       boolean isWaiting;
+       synchronized (this) {
+          long now = System.currentTimeMillis();
+          isWaiting = until > now;
+          while (!stopped && isWaiting) {
+              try {
+                  wait(until - now);
+              } catch (InterruptedException e) {
+              }
+              now = System.currentTimeMillis();
+              isWaiting = until > now;
+          }
+       }
+
+       // notify the listeners outside of the synchronized block (see EXEC-60)
+       if (!isWaiting) {
+           fireTimeoutOccured();
+       }
+   }
+    
 }

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=1180579&r1=1180578&r2=1180579&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
Sun Oct  9 10:40:55 2011
@@ -956,10 +956,10 @@ public class DefaultExecutorTest extends
 
             // start an asynchronous process to enable the main thread
             System.out.println("Preparing to execute process - commandLine=" + cl.toString());
-            DefaultExecuteResultHandler handler = new DefaultExecuteResultHandler();    
      
+            DefaultExecuteResultHandler handler = new DefaultExecuteResultHandler();
             exec.execute(cl, handler);
             System.out.println("Process spun off successfully - process=" + cl.getExecutable());
-    
+
             int x;
             PipedInputStream pis = new PipedInputStream(pipedOutputStream);
             while ((x = pis.read()) >= 0) {
@@ -1010,8 +1010,46 @@ public class DefaultExecutorTest extends
             }
             pis.close();
 
-            handler.waitFor();            
+            handler.waitFor();
+        }
+    }
+
+    /**
+     * Test EXEC-60 (https://issues.apache.org/jira/browse/EXEC-60).
+     *
+     * Possible deadlock when a process is terminating at the same time its timing out. Please
+     * note that a successful test is no proof that the issues was indeed fixed.
+     *
+     * @throws Exception the test failed
+     */
+    public void testExec_60() throws IOException {
+
+        int start = 0;
+        final int seconds = 1;
+        int processTerminatedCounter = 0;
+        int watchdogKilledProcessCounter = 0;
+        CommandLine cmdLine = new CommandLine(pingScript);
+        cmdLine.addArgument(Integer.toString(seconds + 1)); // need to add "1" to wait the
requested number of seconds
+
+        for (int offset = start; offset <= 20; offset += 1) {
+            ExecuteWatchdog watchdog = new ExecuteWatchdog(seconds * 1000 + offset);
+            exec.setWatchdog(watchdog);
+            try {
+                exec.execute(cmdLine);
+                processTerminatedCounter++;
+                System.out.println(offset + ": process has terminated: " + watchdog.killedProcess());
+                if(processTerminatedCounter > 5) {
+                    break;
+                }
+            } catch (ExecuteException ex) {
+                System.out.println(offset + ": process was killed: " + watchdog.killedProcess());
+                assertTrue("Watchdog killed the process", watchdog.killedProcess());
+                watchdogKilledProcessCounter++;;
+            }
         }
+
+        assertTrue("Not a single process terminated on its own", processTerminatedCounter
> 0);
+        assertTrue("Not a single process was killed by the watch dog", watchdogKilledProcessCounter
> 0);
     }
 
     // ======================================================================
@@ -1062,7 +1100,7 @@ public class DefaultExecutorTest extends
 
         String text;
         StringBuffer contents = new StringBuffer();
-        BufferedReader reader = new BufferedReader(new FileReader(file));        
+        BufferedReader reader = new BufferedReader(new FileReader(file));
 
         while ((text = reader.readLine()) != null)
         {



Mime
View raw message