Commands that spawn child processes can sometimes leave sub-sub-processes lingering if the invoked Command does not reap them on its own.
Counterintuitively, I think we should be leveraging the detached: true flag by default to address this. This would be in-line with the scoped managed resource approach that Effect is already taking with Command execution today. Whether we should offer an escape hatch for this default or not I don't have an opinion on.
First of all, note that detached: true behavior differs by platform. More about that here: https://nodejs.org/api/child_process.html#child_process_options_detached
I'm proposing that we set detached: true by default for non-windows systems.
detached: process.platform !== "win32"
This will cause the new process to be assigned as a leader of a new process group that can then be terminated in its entirety at our discretion.
Instead of just forwarding a kill signal to the process handle (handle.kill(signal)), we'd then utilise taskkill with /T on windows (kill the entire process tree) and process.kill(-pid, signal) (negative pid to kill the entire process group) on non-windows systems.
References:
https://nodejs.org/api/child_process.html#child_process_options_detached
https://github.com/Kikobeats/kill-process-group/blob/master/src/index.js
Commands that spawn child processes can sometimes leave sub-sub-processes lingering if the invokedCommanddoes not reap them on its own.Counterintuitively, I think we should be leveraging the
detached: trueflag by default to address this. This would be in-line with the scoped managed resource approach that Effect is already taking withCommandexecution today. Whether we should offer an escape hatch for this default or not I don't have an opinion on.First of all, note that
detached: truebehavior differs by platform. More about that here: https://nodejs.org/api/child_process.html#child_process_options_detachedI'm proposing that we set
detached: trueby default for non-windows systems.This will cause the new process to be assigned as a leader of a new process group that can then be terminated in its entirety at our discretion.
Instead of just forwarding a kill signal to the process handle (
handle.kill(signal)), we'd then utilisetaskkillwith/Ton windows (kill the entire process tree) andprocess.kill(-pid, signal)(negative pid to kill the entire process group) on non-windows systems.References:
https://nodejs.org/api/child_process.html#child_process_options_detached
https://github.com/Kikobeats/kill-process-group/blob/master/src/index.js