Tokio: Consider matching `std::process` behavior

Created on 14 Nov 2019  路  4Comments  路  Source: tokio-rs/tokio

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

Most helpful comment

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.

All 4 comments

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 Command that sets whether or not the returned handle will kill the child on drop?

I prefer this approach over having a wrapper guard:

  • It is easy to discover when looking at the Command docs
  • It is easy to chain which is a common use of the command APIs
  • It does not require updating any type reference to something like StopGuard<Child>, which also means the tokio APIs can largely be a drop in replacement for the std ones

Should have PR up shortly with these changes

Was this page helpful?
0 / 5 - 0 ratings