I noticed that tokio::process diverges from std in that, dropping a child handle will kill the child. I think we should consider switching to std behavior when possible.
For example, tokio::spawn will return a JoinHandle, but dropping that JoinHandle will not cancel the spawned task. This is to match std::thread::spawn behavior.
cc/ @ipetkov
I'm in favor or making all spawns behave in the same way, even if in isolation other options could be preferable, just to avoid having to run to the docs every time when using any of the 4+ spawns. Otherwise these behaviors can get impossible to remember quite quickly.
I think we should consider switching to std behavior when possible.
I agree that APIs that closely match std should have the same behavior. However, I also think there's value in being able to use a child handle to kill the child process, as well.
WDYT about adding a new method to Command that sets whether or not the returned handle will kill the child on drop? Something like this:
// same behavior as std
let child = tokio::process::Command(...)
.spawn()
.unwrap();
// will kill the child process on drop
let child = tokio::process::Command(...)
.kill_on_drop()
.spawn()
.unwrap();
That way, the API could be a superset of std's, and the default behavior would be the same, but users could still use kill-on-drop behavior to prevent orphans in cases where it's desired.
After daydreaming a bit, I'm now imagining a Stop trait which will be implemented by return values of such spawn functions.
Then you could add some kind of general StopGuard usable for both tasks and processes:
{
let _ = StopGuard(
tokio::process::Command(...)
.spawn()
.unwrap()
);
// do whatever is needed
}
WDYT about adding a new method to
Commandthat sets whether or not the returned handle will kill the child on drop?
I prefer this approach over having a wrapper guard:
Command docsStopGuard<Child>, which also means the tokio APIs can largely be a drop in replacement for the std onesShould have PR up shortly with these changes
Most helpful comment
I agree that APIs that closely match
stdshould have the same behavior. However, I also think there's value in being able to use a child handle to kill the child process, as well.WDYT about adding a new method to
Commandthat sets whether or not the returned handle will kill the child on drop? Something like this:That way, the API could be a superset of
std's, and the default behavior would be the same, but users could still use kill-on-drop behavior to prevent orphans in cases where it's desired.