commons-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sgoes...@apache.org
Subject 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
Date Wed, 06 Jan 2016 09:37:15 GMT
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();
     }
 
     /**



Mime
View raw message