Child process can be spawned even when tokio::process::Command::spawn returns an error

[Maki325]

Version
Tokio v1.39.2
rustc 1.76.0

Platform
WSL2 Ubuntu 20.04.6 LTS

Description
For both unix and windows imp for tokio::process::Command::spawn, spawning the std::process::Child is the first call. That could leave the child alive even if subsequent function calls fail.

In my case a panic happened, because I used the Tokio Child outside the Tokio runtime, which isn't the same as the spawn returning an error. But it's still seems possible for an error to happen in the function, which would leave the Child process alive, without the user being able to kill it, or access it in any way.

I could be wrong about it though, as I don't know if those functions in that order would never fail. Maybe they could only fail if the child got killed, and the panic is the only way for this to happen. Which would be fine, as the child would then not end up unreachable, but still running. I have tried looking deeper into the source code to check if that's the case, but it gets much deeper than my understanding of tokio/async runtimes.

Minimum code recreation
Calling tokio::process::Command::spawn outside the tokio runtime.

fn main() {
    let response = tokio::process::Command::new("ls").spawn();
    println!("{response:#?}");
}

This will cause a panic, but the ls command will still output the results to the terminal.

How to maybe handle this
I thought of two ways to handle this. Either:

• Return the std::process::Child as part of the error, so the user can handle it how they want
• Send SIGKILL to the child, to make sure that it's dead when erroring

[Darksonn]

@ipetkov Thoughts?

[ipetkov]

We mirror the behavior of the standard library here: (example of the same behavior with using only std code)

Similar to the behavior to the standard library, and unlike the futures paradigm of dropping-implies-cancellation, a spawned process will, by default, continue to execute even after the Child handle has been dropped.

If you want to ensure that a child process is killed whenever its handle is dropped (even during panic unwinding), you can use kill_on_drop(true)

[ipetkov]

Oh looks like kill_on_drop might not work with this exact panic since we haven't wrapped the inner child in our kill-on-drop wrapper. We should probably both do any checks which can panic sooner, and wrap the intermediate value as appropriate sooner

[Maki325]

The child should probably be wrapped with kill-on-drop as soon as it's spawned, and if everything is successful just unwrap it, so that the tokio::process::Command::spawn can wrap it itself.

If it's ok, I can try implementing it, and making a pull request.