Open Maki325 opened 2 weeks ago
@ipetkov Thoughts?
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)
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
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.
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 thestd::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.
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:
std::process::Child
as part of the error, so the user can handle it how they wantSIGKILL
to the child, to make sure that it's dead when erroring