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 2016/01/06 10:37:15 UTC
svn commit: r1723258 - in /commons/proper/exec/trunk/src:
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: sgoeschl
Date: Wed Jan 6 09:37:15 2016
New Revision: 1723258
URL: http://svn.apache.org/viewvc?rev=1723258&view=rev
Log:
[EXEC-71] ExecuteWatchdog hangs when process was not started
Modified:
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/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=1723258&r1=1723257&r2=1723258&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 Jan 6 09:37:15 2016
@@ -16,13 +16,13 @@
*/
package org.apache.commons.exec;
+import org.apache.commons.exec.launcher.CommandLauncher;
+import org.apache.commons.exec.launcher.CommandLauncherFactory;
+
import java.io.File;
import java.io.IOException;
import java.util.Map;
-import org.apache.commons.exec.launcher.CommandLauncher;
-import org.apache.commons.exec.launcher.CommandLauncherFactory;
-
/**
* The default class to start a subprocess. The implementation
* allows to
@@ -331,9 +331,18 @@ public class DefaultExecutor implements
private int executeInternal(final CommandLine command, final Map<String, String> environment,
final File dir, final ExecuteStreamHandler streams) throws IOException {
- setExceptionCaught(null);
+ final Process process;
+ exceptionCaught = null;
- final Process process = this.launch(command, environment, dir);
+ try {
+ process = this.launch(command, environment, dir);
+ }
+ catch(final IOException e) {
+ if(watchdog != null) {
+ watchdog.failedToStart(e);
+ }
+ throw e;
+ }
try {
streams.setProcessInputStream(process.getOutputStream());
@@ -341,6 +350,9 @@ public class DefaultExecutor implements
streams.setProcessErrorStream(process.getErrorStream());
} catch (final IOException e) {
process.destroy();
+ if(watchdog != null) {
+ watchdog.failedToStart(e);
+ }
throw e;
}
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=1723258&r1=1723257&r2=1723258&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 Jan 6 09:37:15 2016
@@ -122,6 +122,18 @@ public class ExecuteWatchdog implements
}
/**
+ * Notification that starting the process failed.
+ *
+ * @param e the offending exception
+ *
+ */
+ public synchronized void failedToStart(Exception e) {
+ this.processStarted = true;
+ this.caught = e;
+ this.notifyAll();
+ }
+
+ /**
* Stops the watcher. It will notify all threads possibly waiting on this
* object.
*/
@@ -220,11 +232,12 @@ public class ExecuteWatchdog implements
}
/**
- * 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
+ * Ensures that the process is started or not already terminated
+ * so we do not race with asynch executionor hang forever. The
+ * caller of this method must be holding the lock on this
*/
private void ensureStarted() {
- while (!processStarted) {
+ while (!processStarted && caught == null) {
try {
this.wait();
} catch (final InterruptedException e) {
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=1723258&r1=1723257&r2=1723258&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 Jan 6 09:37:15 2016
@@ -18,30 +18,14 @@
package org.apache.commons.exec;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
-
-import java.io.BufferedReader;
-import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
-import java.io.File;
-import java.io.FileInputStream;
-import java.io.FileOutputStream;
-import java.io.FileReader;
-import java.io.IOException;
+import org.apache.commons.exec.environment.EnvironmentUtils;
+import org.junit.*;
+
+import java.io.*;
import java.util.HashMap;
import java.util.Map;
-import org.apache.commons.exec.environment.EnvironmentUtils;
-import org.junit.After;
-import org.junit.Before;
-import org.junit.BeforeClass;
-import org.junit.Ignore;
-import org.junit.Test;
+import static org.junit.Assert.*;
/**
* @version $Id$
@@ -377,8 +361,6 @@ public class DefaultExecutorTest {
/**
* Try to start an non-existing application which should result
* in an exception.
- *
- * @throws Exception the test failed
*/
@Test(expected = IOException.class)
public void testExecuteNonExistingApplication() throws Exception {
@@ -389,19 +371,63 @@ public class DefaultExecutorTest {
}
/**
- * Try to start an non-existing application asynchronously which should result
+ * Try to start an non-existing application which should result
* in an exception.
+ */
+ @Test(expected = IOException.class)
+ public void testExecuteNonExistingApplicationWithWatchDog() throws Exception {
+ final CommandLine cl = new CommandLine(nonExistingTestScript);
+ final DefaultExecutor executor = new DefaultExecutor();
+ executor.setWatchdog(new ExecuteWatchdog(ExecuteWatchdog.INFINITE_TIMEOUT));
+
+ executor.execute(cl);
+ }
+
+ /**
+ * Try to start an non-existing application where the exception is caught/processed
+ * by the result handler.
+ */
+ @Test
+ public void testExecuteAsyncNonExistingApplication() throws Exception {
+ final CommandLine cl = new CommandLine(nonExistingTestScript);
+ final DefaultExecuteResultHandler resultHandler = new DefaultExecuteResultHandler();
+ final DefaultExecutor executor = new DefaultExecutor();
+
+ executor.execute(cl, resultHandler);
+ resultHandler.waitFor();
+
+ assertTrue(executor.isFailure(resultHandler.getExitValue()));
+ assertNotNull(resultHandler.getException());
+ }
+
+ /**
+ * Try to start an non-existing application where the exception is caught/processed
+ * by the result handler. The watchdog in notified to avoid waiting for the
+ * process infinitely.
*
- * @throws Exception the test failed
+ * @see <a href="https://issues.apache.org/jira/browse/EXEC-71">EXEC-71</a>
*/
@Test
- public void testExecuteAsyncWithNonExistingApplication() throws Exception {
+ public void testExecuteAsyncNonExistingApplicationWithWatchdog() throws Exception {
final CommandLine cl = new CommandLine(nonExistingTestScript);
- final DefaultExecuteResultHandler handler = new DefaultExecuteResultHandler();
- exec.execute(cl, handler);
- Thread.sleep(2000);
- assertNotNull(handler.getException());
- assertTrue(exec.isFailure(handler.getExitValue()));
+ final DefaultExecuteResultHandler resultHandler = new DefaultExecuteResultHandler() {
+ @Override
+ public void onProcessFailed(ExecuteException e) {
+ System.out.println("Process did not stop gracefully, had exception '" + e.getMessage() + "' while executing process");
+ super.onProcessFailed(e);
+ }
+ };
+ final DefaultExecutor executor = new DefaultExecutor();
+ executor.setWatchdog(new ExecuteWatchdog(ExecuteWatchdog.INFINITE_TIMEOUT));
+
+ executor.execute(cl, resultHandler);
+ resultHandler.waitFor();
+
+ assertTrue(executor.isFailure(resultHandler.getExitValue()));
+ assertNotNull(resultHandler.getException());
+ assertFalse(executor.getWatchdog().isWatching());
+ assertFalse(executor.getWatchdog().killedProcess());
+ executor.getWatchdog().destroyProcess();
}
/**