Periodically we check Process.isAlive() via sending Inspect to ProcessDestroyer. Prior to this, we would only be notified if a process exited if its stdout and stderr file descriptors had exited. This is incorrect since these descriptors can live on beyond the execution of a process (via forking).
When stdout or stderr exit, we fire an Inspect which allows us to exit in a quick manner in the ideal case, without having to wait for the next poll.
I think this is a quite safe implementation, and a big improvement over what was here before. Note that the tests didn't have to change, and only one additional test was added (which previously failed).
This PR modifies
BlockingProcessin a few ways:Periodically we check
Process.isAlive()via sendingInspecttoProcessDestroyer. Prior to this, we would only be notified if a process exited if its stdout and stderr file descriptors had exited. This is incorrect since these descriptors can live on beyond the execution of a process (via forking).DestroyandDestroyForciblynow result in calls toProcess.destroy()orProcess.destroyForcibly(), respectively delivering aSIGTERMorSIGKILL, on UNIX at least - https://github.com/frohoff/jdk8u-jdk/blob/da0da73ab82ed714When
stdoutorstderrexit, we fire anInspectwhich allows us to exit in a quick manner in the ideal case, without having to wait for the next poll.I think this is a quite safe implementation, and a big improvement over what was here before. Note that the tests didn't have to change, and only one additional test was added (which previously failed).