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)
{
