[
https://issues.apache.org/jira/browse/EXEC-121?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17801211#comment-17801211
]
Gary D. Gregory commented on EXEC-121:
--------------------------------------
Hello [~Marcono1234]
Thank you for your report.
PRs on GitHub are welcome :-)
> 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)
