[
https://issues.apache.org/jira/browse/EXEC-121?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17803850#comment-17803850
]
Marcono1234 commented on EXEC-121:
----------------------------------
[~ggregory], have created https://github.com/apache/commons-exec/pull/144;
feedback is appreciated!
> Async execution does not guarantee that process destroyer is registered
> -----------------------------------------------------------------------
>
> Key: EXEC-121
> URL: https://issues.apache.org/jira/browse/EXEC-121
> Project: Commons Exec
> Issue Type: Bug
> Affects Versions: 1.3
> Reporter: Marcono1234
> Priority: Minor
>
> h3. Bug
> When using one of the async {{Executor.execute}} methods, i.e. one taking an
> {{ExecuteResultHandler}} argument, it is not guaranteed that the process
> destroyer is registered.
> The reason for this is that launching of the process and registration of the
> process destroyer is done already in the separate executor thread. So it is
> possible that a race condition occurs where the process is currently
> launching but in the mean time the JVM exits, so the process destroyer is
> never registered and the process is therefore not destroyed.
> While I assume for normal use cases it is unlikely that the JVM exits right
> after the process was launched, such situations could occur if a (possibly
> unrelated) error occurs and the whole application should terminate. In such
> cases it would probably be important that any launched process is destroyed
> as well.
> h3. Example
> Here is a small example demonstrating this:
> {code}
> Executor e = new DaemonExecutor();
> e.setProcessDestroyer(new ShutdownHookProcessDestroyer());
> // This is to get the timing of this test right, that is, finish in `main`
> right after process was launched
> CountDownLatch latch = new CountDownLatch(1);
> e.setStreamHandler(new PumpStreamHandler() {
> @Override
> public void start() {
> latch.countDown();
> try {
> Thread.sleep(1000);
> } catch (InterruptedException e) {
> e.printStackTrace();
> }
> super.start();
> }
> });
> CommandLine c = new CommandLine("notepad.exe"); // for Windows
> e.execute(c, new ExecuteResultHandler() {
> @Override
> public void onProcessFailed(ExecuteException e) {
> e.printStackTrace();
> }
> @Override
> public void onProcessComplete(int exitValue) {
> System.out.println("Completed: " + exitValue);
> }
> });
> latch.await();
> {code}
> This is a bit contrived to make sure it gets the timing right, but it can
> also be reproduced with "real world" code. What happens is that Notepad is
> launched, the JVM exits but Notepad remains open (= the bug).
> If you add for example a {{Thread.sleep(3000);}} at the end of {{main}} it
> works as expected and Notepad is closed when the JVM exits.
> h3. Possible solution
> One solution might be to launch the process and register the process
> destroyer outside the executor thread. This would also have the nice side
> effect that execution fails fast if the process cannot even be launched in
> the first place.
> But on the other hand it would probably slow down starting async execution
> because it has to wait until the process is launched (though not until it
> exits; that is still handled by the executor thread).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
