[jira] [Created] (EXEC-121) Async execution does not guarantee that process destroyer is registered

["Marcono1234 (Jira)" <ji...@apache.org>]

Marcono1234 created EXEC-121:
--------------------------------

             Summary: 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


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)