cargo run kills child processes upon exit

[kuviman]

Is this expected behaviour?

fn main() {
    std::process::Command::new("cmd").arg("/C").arg("start").arg("cmd.exe").spawn().unwrap();
    std::thread::sleep_ms(1000);
}

Running this with cargo run opens cmd.exe for only one second, but running directly via target/release/project.exe opens it forever.

[alexcrichton]

It's currently intended that we kill all child processes. That was originally intended, however, so that ctrl-c on Cargo would behave the same way on Windows as it does on Unix by default (kill the process tree). I'm actually not sure why everything is killed on exit though. I think that's mainly because if a subprocess failed we want to be sure to clean up after it, but if everything was successful then leaking seems fine

[SiegeLord]

This breaks RustGnuplot on Windows. It spawns a persistent gnuplot process showing all the plots that is meant to outlive the spawning Rust program, but since cargo appears to kill everything, it closes all the plots. This works fine on Linux.

[alexcrichton]

Ok thinking about this for a bit and looking through the history, the main motivation for adding the job objects in the first place was emulating the same behavior on Unix and Windows for ctrl-c. That doesn't have to do with successful execution, however! I think that instead what we want to do is to when control flow of the program reaches the destructor for the job object we instead just remove all processes from the job object or otherwise configure the job object to not terminate everything on closure.

I believe this'll emulate the same behavior on Unix and Windows (and what I'd expect as the "naive intuitive behavior" as well). That way if you ctrl-c on both platforms you'll kill everything, and otherwise everything will remain running that's otherwise running.

The fix here needs to go in two locations:

• First in Cargo
• Second in in rustup

Both those locations will need to be tweaked to, in the destructor, reconfigure the job object to not kill all processes or otherwise remove all processes from the job object.

[Seeker14491]

Reading the linked code for the job object in Cargo, I see that in the job object destructor, processes are killed manually with the kill_remaining() method, and then the job object is configured to not kill anything. The stated reason for doing this is to be able to whitelist certain processes to outlive the job object.

If we want all running processes to keep running when cargo exits successfully, then all we would need to do is remove the call to the kill_remaining() method. I'd like to know why we went through the trouble of writing the whole kill_remaining() method to implement the whitelist approach though. Are there some processes that should always be killed when cargo exits, even on a successful exit?

[alexcrichton]

@Seeker14491 job objects were originally added to make ctrl-c work across platforms. On Unix ctrl-c kills the process group, not a single process, which means everything gets taken down. On Windows however only one process killed, and by using job objects we also guaranteed unix-like behavior of the whole group of processes being killed.

The initial implementation simply let the job object kill everything on Windows, even on success. This was a bug at the time and didn't really come up till much later (now). When implementing leaking processes it simply preserved the previous behavior, killing everything on all exits.

All that to say, no, there shouldn't be anything that Cargo should kill on successful exits. It should continue to kill everything on abnormal exits though!

[Seeker14491]

@alexcrichton,
Alright, so as I understand it, Cargo currently does the following:

• On normal exit: Kills all processes except mspdbsrv.exe (because it's whitelisted in the kill_remaining() method)
• On ctrl-c: Kills all processes including mspdbsrv.exe (bug?).

The correct behavior would be:

• On normal exit: Kill nothing.
• On ctrl-c: Kill all processes except mspdbsrv.exe.

Does this sound correct?

[alexcrichton]

@Seeker14491 that sounds about right yeah! I don't know how we can configure Cargo to not kill mspdbsrv.exe on ctrl-c while still killing everything else on ctrl-c, but if it works it works! That's a relatively minor bug, though, and the main thing to fix I believe is to avoid killing anything on a normal exit.

[hlavaatch]

Could you also not kill processes on ctrl+c when cargo run, but send them the ctrl+c instead so that they can handle it themselves as intended?

[alexcrichton]

@hlavaatch idealy we would yeah! I unfortunately don't know how to personally do that myself though :(

[hlavaatch]

But I do! :)

What happens is Windows console sends ctrl-c to all processes associated with the given window.
Cargo has no handler so the default handler is used which kills everything.

There is a crate for handling ctrl-c called ctrlc
All that is needed is to install an no-op empty handler in cargo just prior launching the process that is run.
I have tested this fix and it works fine!
What I did exactly:

• added ctrlc crate to windows target dependencies in Cargo.toml under

[target.'cfg(windows)'.dependencies]
ctrlc = "3.1.1"

• added extern crate on top of cargo/ops/cargo_run.rs

#[cfg(target_os = "windows")]
extern crate ctrlc; 

• added a bit of code in run() in cargo/ops/cargo_run.rs that registers empty no-op ctrl-c handler just before the process launch:

    config.shell().status("Running", process.to_string())?;

    #[cfg(target_os="windows")]
    ctrlc::set_handler(move || {} )?;

    let result = process.exec_replace();

With this hack cargo run no longer interferes with the ctrl-c handling of the crate that is being run :)

[Seeker14491]

@hlavaatch could you create a new issue and link to it here?