ExecuteWatchdog does not really terminate the "hung" process?

[Mishkin, Ernest]

Hello,

I have a simple test case which involves starting a .bat file which
tries to simulate a process "stuck for N seconds" by executing
ping -n %1 -w 1000 127.0.0.1

I invoke this from java using DefaultExcutor, passing "10" as an
argument to .bat file (so that it will run 10 approx. 1 sec pings
resulting in close to 10 sec wait).
I'm also setting ExecuteWatchdog with a 2 sec timeout.
Problem is, my execute call does not return until ~ 10 seconds later, as
measured by System.currentTimeMillis().
The watchdog confirms that it "killed" the process however the expected
behavior is that execute() call gets interrupted after 2 secs and it
wasn't.

The complete source code is attached.

Please help me understand: am I missing something fundamental, have I
found an issue, or something else?


If developer(s) are lurking on this list, I'd like to mention that it
seems like when ExecuteWatchdog detects a timeout it should interrupt
the "primary" thread (the one which runs code from
DefaultExecutor::executeInternal, in particular the lines below:
            try {
                exitValue = process.waitFor();
            } catch (InterruptedException e) {
                process.destroy();
            }
process.waitFor() blocks, and it seems that when process.destroy() is
called from the watchdog thread the other thread which is stuck in
process.waitFor() is not released.
That's why having DefaultWatchdog maintain a reference to the executor's
thread and interrupt()'ing it might be helpful.



Sincerely,
Ernest Mishkin
 
--------------------------------------------------------

The information contained in this message is intended only for the recipient, and may be a confidential attorney-client communication or may otherwise be privileged and confidential and protected from disclosure. If the reader of this message is not the intended recipient, or an employee or agent responsible for delivering this message to the intended recipient, please be aware that any dissemination or copying of this communication is strictly prohibited. If you have received this communication in error, please immediately notify us by replying to the message and deleting it from your computer. The McGraw-Hill Companies, Inc. reserves the right, subject to applicable local law, to monitor and review the content of any electronic message or information sent to or from McGraw-Hill employee e-mail addresses without informing the sender or recipient of the message.
--------------------------------------------------------



---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@...
For additional commands, e-mail: user-help@...

[Mishkin, Ernest]

Looks like my attachements got chopped somewhere along the way, so I'll
just paste the two files here since they're really short:


ExecTest.java
//////////////////////////////////
import java.io.File;

import org.apache.commons.exec.CommandLine;
import org.apache.commons.exec.DefaultExecutor;
import org.apache.commons.exec.ExecuteException;
import org.apache.commons.exec.ExecuteWatchdog;
import org.apache.commons.exec.PumpStreamHandler;
import org.apache.commons.io.output.ByteArrayOutputStream;

public class ExecTest {

        public static void main(String[] args) throws Exception {
                File scriptFile = new File("sleep.bat");
                CommandLine cmdLine = new CommandLine(scriptFile);
                cmdLine.addArgument("10"); // sleep 10 secs
                System.out.println("Cmd line: " + cmdLine);
                DefaultExecutor executor = new DefaultExecutor();
                ExecuteWatchdog watchdog = new ExecuteWatchdog(2 *
1000); // allow process no more than 2 secs
                executor.setWatchdog(watchdog);
                ByteArrayOutputStream baos = new
ByteArrayOutputStream();
                executor.setStreamHandler(new PumpStreamHandler(baos));

                long startTime = System.currentTimeMillis();
                try {
                        executor.execute(cmdLine);
                } catch (ExecuteException e) {
                        System.out.println(e);
                }
                System.out.println("Process completed in " +
(System.currentTimeMillis() - startTime) +" millis; below is its
output");
                System.out.println(baos);
                if (watchdog.killedProcess()) {
                        System.out.println("Process timed out and was
killed.");
                }
        }

}
//////////////////////////////////


sleep.bat

ping -n %1 -w 1000 127.0.0.1
 
--------------------------------------------------------

The information contained in this message is intended only for the recipient, and may be a confidential attorney-client communication or may otherwise be privileged and confidential and protected from disclosure. If the reader of this message is not the intended recipient, or an employee or agent responsible for delivering this message to the intended recipient, please be aware that any dissemination or copying of this communication is strictly prohibited. If you have received this communication in error, please immediately notify us by replying to the message and deleting it from your computer. The McGraw-Hill Companies, Inc. reserves the right, subject to applicable local law, to monitor and review the content of any electronic message or information sent to or from McGraw-Hill employee e-mail addresses without informing the sender or recipient of the message.
--------------------------------------------------------

---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@...
For additional commands, e-mail: user-help@...

[Siegfried Goeschl]

Hi Ernest,

this is a lurking developer .... :-)

+) killing your top-level process has not the semantics you would expect
(see http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4770092) - so
under Windows it would never kill your "ping" process

+) it seems that the new ProcessBuilder does not help either (see
http://stackoverflow.com/questions/801609/java-processbuilder-process-destroy-not-killing-child-processes-in-winxp)

+) we have a test case
"DefaultExecutorTest#testExecuteAsyncWithTooLateUserTermination" which
kills a process using Watchdog - please note that we carefully avoided
to create s subprocess here since we rely on built-in shell commands -
we always try to prove that our code works ... :-)

+) which leaves the question why your thread waits for the whole 10
seconds - need to investigate that

Cheers,

Siegfried Goeschl



Mishkin, Ernest wrote:

> Hello,
>
> I have a simple test case which involves starting a .bat file which
> tries to simulate a process "stuck for N seconds" by executing
> ping -n %1 -w 1000 127.0.0.1
>
> I invoke this from java using DefaultExcutor, passing "10" as an
> argument to .bat file (so that it will run 10 approx. 1 sec pings
> resulting in close to 10 sec wait).
> I'm also setting ExecuteWatchdog with a 2 sec timeout.
> Problem is, my execute call does not return until ~ 10 seconds later, as
> measured by System.currentTimeMillis().
> The watchdog confirms that it "killed" the process however the expected
> behavior is that execute() call gets interrupted after 2 secs and it
> wasn't.
>
> The complete source code is attached.
>
> Please help me understand: am I missing something fundamental, have I
> found an issue, or something else?
>
>
> If developer(s) are lurking on this list, I'd like to mention that it
> seems like when ExecuteWatchdog detects a timeout it should interrupt
> the "primary" thread (the one which runs code from
> DefaultExecutor::executeInternal, in particular the lines below:
>             try {
>                 exitValue = process.waitFor();
>             } catch (InterruptedException e) {
>                 process.destroy();
>             }
> process.waitFor() blocks, and it seems that when process.destroy() is
> called from the watchdog thread the other thread which is stuck in
> process.waitFor() is not released.
> That's why having DefaultWatchdog maintain a reference to the executor's
> thread and interrupt()'ing it might be helpful.
>
>
>
> Sincerely,
> Ernest Mishkin
>  
> --------------------------------------------------------
>
> The information contained in this message is intended only for the recipient, and may be a confidential attorney-client communication or may otherwise be privileged and confidential and protected from disclosure. If the reader of this message is not the intended recipient, or an employee or agent responsible for delivering this message to the intended recipient, please be aware that any dissemination or copying of this communication is strictly prohibited. If you have received this communication in error, please immediately notify us by replying to the message and deleting it from your computer. The McGraw-Hill Companies, Inc. reserves the right, subject to applicable local law, to monitor and review the content of any electronic message or information sent to or from McGraw-Hill employee e-mail addresses without informing the sender or recipient of the message.
> --------------------------------------------------------
>
>  
> ------------------------------------------------------------------------
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: user-unsubscribe@...
> For additional commands, e-mail: user-help@...

---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@...
For additional commands, e-mail: user-help@...

[Mishkin, Ernest]

Siegfried,

Thanks for looking into this.

I found that my "main" thread was stuck trying to join with stream
handling threads.
I modified PumpStreamHandler::stop - added boolean interrupt parameter,
where if interrupt == true I call [out|error|input]Thread.interrupt()
instead of .join().
Then I modified DefaultExecutor::executeInternal() to pass true to
PumpStreamHandler::stop when watchdog != null &&
watchdog.killedProcess().
I also had to avoid calling closeStreams() in case of forced
termination.

The net effect is that now when timeout occurs, my execute() call is
interrupted, and java code can continue (with the process's output up to
the point of interruption captured), leaving the
supposed-to-die-but-not-yet-dead native process (ping in my case)
running (until it dies on its own).
This is pretty much the behavior I need / was looking for.

I also happened to fix NPE in setExitValues().

If you have any interest in patches, let me know.


Additionally, there's an open-source library hosted at [1] which
provides a JNI wrapper around windows native process management so that
one can truly kill a process.
This can be integrated with commons-exec as an optional component,
perhaps.

Regards,
Ernest

[1] https://winp.dev.java.net/




-----Original Message-----
From: Siegfried Goeschl [mailto:siegfried.goeschl@...]
Sent: Monday, June 22, 2009 1:19 PM
To: Commons Users List
Subject: Re: ExecuteWatchdog does not really terminate the "hung"
process?

Hi Ernest,

this is a lurking developer .... :-)

+) killing your top-level process has not the semantics you would expect
(see http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4770092) - so
under Windows it would never kill your "ping" process

+) it seems that the new ProcessBuilder does not help either (see
http://stackoverflow.com/questions/801609/java-processbuilder-process-de
stroy-not-killing-child-processes-in-winxp)

+) we have a test case
"DefaultExecutorTest#testExecuteAsyncWithTooLateUserTermination" which
kills a process using Watchdog - please note that we carefully avoided
to create s subprocess here since we rely on built-in shell commands -
we always try to prove that our code works ... :-)

+) which leaves the question why your thread waits for the whole 10
seconds - need to investigate that

Cheers,

Siegfried Goeschl
 
--------------------------------------------------------

The information contained in this message is intended only for the recipient, and may be a confidential attorney-client communication or may otherwise be privileged and confidential and protected from disclosure. If the reader of this message is not the intended recipient, or an employee or agent responsible for delivering this message to the intended recipient, please be aware that any dissemination or copying of this communication is strictly prohibited. If you have received this communication in error, please immediately notify us by replying to the message and deleting it from your computer. The McGraw-Hill Companies, Inc. reserves the right, subject to applicable local law, to monitor and review the content of any electronic message or information sent to or from McGraw-Hill employee e-mail addresses without informing the sender or recipient of the message.
--------------------------------------------------------

---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@...
For additional commands, e-mail: user-help@...

[Siegfried Goeschl]

Hi Ernest,

+) could you please open a JIRA and attach the patch (see
https://issues.apache.org/jira/browse/EXEC)
+) I will have a look at winp

Thanks in advance

Siegfried Goeschl


Mishkin, Ernest wrote:

> Siegfried,
>
> Thanks for looking into this.
>
> I found that my "main" thread was stuck trying to join with stream
> handling threads.
> I modified PumpStreamHandler::stop - added boolean interrupt parameter,
> where if interrupt == true I call [out|error|input]Thread.interrupt()
> instead of .join().
> Then I modified DefaultExecutor::executeInternal() to pass true to
> PumpStreamHandler::stop when watchdog != null &&
> watchdog.killedProcess().
> I also had to avoid calling closeStreams() in case of forced
> termination.
>
> The net effect is that now when timeout occurs, my execute() call is
> interrupted, and java code can continue (with the process's output up to
> the point of interruption captured), leaving the
> supposed-to-die-but-not-yet-dead native process (ping in my case)
> running (until it dies on its own).
> This is pretty much the behavior I need / was looking for.
>
> I also happened to fix NPE in setExitValues().
>
> If you have any interest in patches, let me know.
>
>
> Additionally, there's an open-source library hosted at [1] which
> provides a JNI wrapper around windows native process management so that
> one can truly kill a process.
> This can be integrated with commons-exec as an optional component,
> perhaps.
>
> Regards,
> Ernest
>
> [1] https://winp.dev.java.net/
>
>
>
>
> -----Original Message-----
> From: Siegfried Goeschl [mailto:siegfried.goeschl@...]
> Sent: Monday, June 22, 2009 1:19 PM
> To: Commons Users List
> Subject: Re: ExecuteWatchdog does not really terminate the "hung"
> process?
>
> Hi Ernest,
>
> this is a lurking developer .... :-)
>
> +) killing your top-level process has not the semantics you would expect
> (see http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4770092) - so
> under Windows it would never kill your "ping" process
>
> +) it seems that the new ProcessBuilder does not help either (see
> http://stackoverflow.com/questions/801609/java-processbuilder-process-de
> stroy-not-killing-child-processes-in-winxp)
>
> +) we have a test case
> "DefaultExecutorTest#testExecuteAsyncWithTooLateUserTermination" which
> kills a process using Watchdog - please note that we carefully avoided
> to create s subprocess here since we rely on built-in shell commands -
> we always try to prove that our code works ... :-)
>
> +) which leaves the question why your thread waits for the whole 10
> seconds - need to investigate that
>
> Cheers,
>
> Siegfried Goeschl
>  
> --------------------------------------------------------
>
> The information contained in this message is intended only for the recipient, and may be a confidential attorney-client communication or may otherwise be privileged and confidential and protected from disclosure. If the reader of this message is not the intended recipient, or an employee or agent responsible for delivering this message to the intended recipient, please be aware that any dissemination or copying of this communication is strictly prohibited. If you have received this communication in error, please immediately notify us by replying to the message and deleting it from your computer. The McGraw-Hill Companies, Inc. reserves the right, subject to applicable local law, to monitor and review the content of any electronic message or information sent to or from McGraw-Hill employee e-mail addresses without informing the sender or recipient of the message.
> --------------------------------------------------------
>
>
>  

---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@...
For additional commands, e-mail: user-help@...

[Siegfried Goeschl]

Hi Ernest,

I'm very interested in the patch but you still have to provide it ... :-)

Thanks in advance

Siegfried Goeschl

Mishkin, Ernest wrote:

> Siegfried,
>
> Thanks for looking into this.
>
> I found that my "main" thread was stuck trying to join with stream
> handling threads.
> I modified PumpStreamHandler::stop - added boolean interrupt parameter,
> where if interrupt == true I call [out|error|input]Thread.interrupt()
> instead of .join().
> Then I modified DefaultExecutor::executeInternal() to pass true to
> PumpStreamHandler::stop when watchdog != null &&
> watchdog.killedProcess().
> I also had to avoid calling closeStreams() in case of forced
> termination.
>
> The net effect is that now when timeout occurs, my execute() call is
> interrupted, and java code can continue (with the process's output up to
> the point of interruption captured), leaving the
> supposed-to-die-but-not-yet-dead native process (ping in my case)
> running (until it dies on its own).
> This is pretty much the behavior I need / was looking for.
>
> I also happened to fix NPE in setExitValues().
>
> If you have any interest in patches, let me know.
>
>
> Additionally, there's an open-source library hosted at [1] which
> provides a JNI wrapper around windows native process management so that
> one can truly kill a process.
> This can be integrated with commons-exec as an optional component,
> perhaps.
>
> Regards,
> Ernest
>
> [1] https://winp.dev.java.net/
>
>
>
>
> -----Original Message-----
> From: Siegfried Goeschl [mailto:siegfried.goeschl@...]
> Sent: Monday, June 22, 2009 1:19 PM
> To: Commons Users List
> Subject: Re: ExecuteWatchdog does not really terminate the "hung"
> process?
>
> Hi Ernest,
>
> this is a lurking developer .... :-)
>
> +) killing your top-level process has not the semantics you would expect
> (see http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4770092) - so
> under Windows it would never kill your "ping" process
>
> +) it seems that the new ProcessBuilder does not help either (see
> http://stackoverflow.com/questions/801609/java-processbuilder-process-de
> stroy-not-killing-child-processes-in-winxp)
>
> +) we have a test case
> "DefaultExecutorTest#testExecuteAsyncWithTooLateUserTermination" which
> kills a process using Watchdog - please note that we carefully avoided
> to create s subprocess here since we rely on built-in shell commands -
> we always try to prove that our code works ... :-)
>
> +) which leaves the question why your thread waits for the whole 10
> seconds - need to investigate that
>
> Cheers,
>
> Siegfried Goeschl
>  
> --------------------------------------------------------
>
> The information contained in this message is intended only for the recipient, and may be a confidential attorney-client communication or may otherwise be privileged and confidential and protected from disclosure. If the reader of this message is not the intended recipient, or an employee or agent responsible for delivering this message to the intended recipient, please be aware that any dissemination or copying of this communication is strictly prohibited. If you have received this communication in error, please immediately notify us by replying to the message and deleting it from your computer. The McGraw-Hill Companies, Inc. reserves the right, subject to applicable local law, to monitor and review the content of any electronic message or information sent to or from McGraw-Hill employee e-mail addresses without informing the sender or recipient of the message.
> --------------------------------------------------------
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: user-unsubscribe@...
> For additional commands, e-mail: user-help@...
>
>
>
>  

---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@...
For additional commands, e-mail: user-help@...

[Mishkin, Ernest]

I haven't forgotten... Hopefully today.

--Ernest

-----Original Message-----
From: Siegfried Goeschl [mailto:siegfried.goeschl@...]
Sent: Saturday, June 27, 2009 7:09 AM
To: Commons Users List
Subject: Re: ExecuteWatchdog does not really terminate the "hung"
process?

Hi Ernest,

I'm very interested in the patch but you still have to provide it ...
:-)

Thanks in advance

Siegfried Goeschl
 
--------------------------------------------------------

The information contained in this message is intended only for the recipient, and may be a confidential attorney-client communication or may otherwise be privileged and confidential and protected from disclosure. If the reader of this message is not the intended recipient, or an employee or agent responsible for delivering this message to the intended recipient, please be aware that any dissemination or copying of this communication is strictly prohibited. If you have received this communication in error, please immediately notify us by replying to the message and deleting it from your computer. The McGraw-Hill Companies, Inc. reserves the right, subject to applicable local law, to monitor and review the content of any electronic message or information sent to or from McGraw-Hill employee e-mail addresses without informing the sender or recipient of the message.
--------------------------------------------------------

---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@...
For additional commands, e-mail: user-help@...

[Mishkin, Ernest]

Siegfried,

Should I generate the patches against latest sources or 1.0 - release
sources?

--Ernest

-----Original Message-----
From: Mishkin, Ernest
Sent: Monday, June 29, 2009 8:23 AM
To: Commons Users List
Cc: siegfried.goeschl@...
Subject: **Caution-External**: RE: ExecuteWatchdog does not really
terminate the "hung" process?

I haven't forgotten... Hopefully today.

--Ernest
 
--------------------------------------------------------

The information contained in this message is intended only for the recipient, and may be a confidential attorney-client communication or may otherwise be privileged and confidential and protected from disclosure. If the reader of this message is not the intended recipient, or an employee or agent responsible for delivering this message to the intended recipient, please be aware that any dissemination or copying of this communication is strictly prohibited. If you have received this communication in error, please immediately notify us by replying to the message and deleting it from your computer. The McGraw-Hill Companies, Inc. reserves the right, subject to applicable local law, to monitor and review the content of any electronic message or information sent to or from McGraw-Hill employee e-mail addresses without informing the sender or recipient of the message.
--------------------------------------------------------

---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@...
For additional commands, e-mail: user-help@...

[Siegfried Goeschl]

Hi Ernest,

latest source would be nice ... :-)

Cheers,

Siegfried Goeschl

Mishkin, Ernest wrote:

> Siegfried,
>
> Should I generate the patches against latest sources or 1.0 - release
> sources?
>
> --Ernest
>
> -----Original Message-----
> From: Mishkin, Ernest
> Sent: Monday, June 29, 2009 8:23 AM
> To: Commons Users List
> Cc: siegfried.goeschl@...
> Subject: **Caution-External**: RE: ExecuteWatchdog does not really
> terminate the "hung" process?
>
> I haven't forgotten... Hopefully today.
>
> --Ernest
>  
> --------------------------------------------------------
>
> The information contained in this message is intended only for the recipient, and may be a confidential attorney-client communication or may otherwise be privileged and confidential and protected from disclosure. If the reader of this message is not the intended recipient, or an employee or agent responsible for delivering this message to the intended recipient, please be aware that any dissemination or copying of this communication is strictly prohibited. If you have received this communication in error, please immediately notify us by replying to the message and deleting it from your computer. The McGraw-Hill Companies, Inc. reserves the right, subject to applicable local law, to monitor and review the content of any electronic message or information sent to or from McGraw-Hill employee e-mail addresses without informing the sender or recipient of the message.
> --------------------------------------------------------
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: user-unsubscribe@...
> For additional commands, e-mail: user-help@...
>
>
>
>  

---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@...
For additional commands, e-mail: user-help@...