Stop cargo from interfering with Ctrl-C handling on cargo run on Windows

[hlavaatch]

Currently when rust application crate is run with cargo run on Windows and user presses ctrl+C in the console window, application gets killed even though application itself is handling Ctrl-C itself (for example for clean shutdown). This is because cargo itself does not handle Ctrl-C and default handler functionality seems to be to kill everything.

Ctrl-C handling on Windows is managed by Control handlers.
There is a ctrlc crate that can be used to provide a handler for the ctrl-C signal that is run when Ctrl-C is received. I have tested it and it works.

I have found that when I install an empty no-op handler in cargo when running the application on cargo run, everything works as it should - cargo ignores the Ctrl-C and the result depends on what the application does, as one would expect.

What is needed is to add the crtlc crate to windows target dependencies, and in cargo/ops/cargo_run.rs
just before executing the process to run install an empty no-op handler using

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

As discussed here

[hlavaatch]

Found a perfect place for setting the handler, cargo/util/process_builder.rs, in windows specific variant of ProcessBuilder::exec_replace(), in front of the self.exec()

    /// On unix, executes the process using the unix syscall `execvp`, which will block this
    /// process, and will only return if there is an error. On windows this is a synonym for
    /// `exec`.
    #[cfg(windows)]
    pub fn exec_replace(&self) -> CargoResult<()> {
        ctrlc::set_handler(move || {} )?;  // <-- here!
        self.exec()
    }

[Seeker14491]

Does Unix not have this issue? Is that why this fix is Windows-specific?

[zachlute]

I poked at this awhile back and reached a similar conclusion. This isn't a problem on Unix because the cargo process gets replaced by the process it's running, and so that process gets any SIGINT/SIGTERM.

9a0801d

But as @hlavaatch mentioned, the problem still exists on Windows. I tried a similar strategy to fix it then, and it seemed to work, but I wasn't experienced enough with Cargo or Rust then to feel comfortable sharing that change then. Maybe it's time I poke again.

FWIW, I think Rustup has the same issue when launching commands, so fixing this for Cargo is only part of the story, maybe?

[zachlute]

Hah, yeah, just dug up my old change and it's pretty much the exact same thing. I tested it with my old test case (running it on my own program with a ctrlc handler) and still seems to work fine, I'll put together a PR for it, though I don't think it's possible to add an actual test for this in any reasonable way, unfortunately.

[Seeker14491]

It's worth remembering that when you run a cargo command, it's actually rustup that runs, which then executes the real Cargo executable. This bit me recently when I sent a pull request fixing a bug with cargo doc --open on Windows; it ended up actually breaking the command when used through rustup. So it might be a good idea to test the fix with Cargo run through rustup.

[zachlute]

Yeah, that's kind of what I was alluding to with "only part of the story" up there. FWIW, copying a locally built cargo with this change to my nightly rustup location is not broken. That is to say, this change does not fix the problem for that case, but it exhibits the same 'broken' behavior it did previously, and the same behavior running the current nightly cargo.exe does without this change.

So I think this change is safe to merge either way, because it fixes the case of running a naked cargo.exe, but another change is needed to fix rustup. I'll look into that, too.