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)
