Skip to content

Commit

Permalink
Auto merge of #6004 - zachlute:fix-ctrlc-handling2, r=alexcrichton
Browse files Browse the repository at this point in the history
Add empty ctrlc handler on Windows.

Fixes #6000.

This is a 'better' version of PR #6002 that accomplishes the same thing using only `winapi` calls without the dependency on the `ctrlc` crate or spawning an additional thread.

----

When exec_replacing the cargo process, we want the new process to get any signals. This already works fine on Unix.

On Windows, pressing ctrlc kills the cargo process and doesn't pass the signal on to the child process. By adding an empty handler, we allow the child process to handle the signal instead.
  • Loading branch information
bors committed Sep 15, 2018
2 parents 509bf99 + 6a3f4b9 commit 044a525
Showing 1 changed file with 64 additions and 23 deletions.
87 changes: 64 additions & 23 deletions src/cargo/util/process_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,30 +150,23 @@ impl ProcessBuilder {
}
}

/// 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(unix)]
pub fn exec_replace(&self) -> CargoResult<()> {
use std::os::unix::process::CommandExt;

let mut command = self.build_command();
let error = command.exec();
Err(::util::CargoError::from(error)
.context(process_error(
&format!("could not execute process {}", self),
None,
None,
))
.into())
}

/// 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)]
/// Replaces the current process with the target process.
///
/// On Unix, this 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 isn't technically possible. Instead we emulate it to the best of our
/// ability. One aspect we fix here is that we specify a handler for the ctrl-c handler.
/// In doing so (and by effectively ignoring it) we should emulate proxying ctrl-c
/// handling to the application at hand, which will either terminate or handle it itself.
/// According to microsoft's documentation at:
/// https://docs.microsoft.com/en-us/windows/console/ctrl-c-and-ctrl-break-signals
/// the ctrl-c signal is sent to all processes attached to a terminal, which should
/// include our child process. If the child terminates then we'll reap them in Cargo
/// pretty quickly, and if the child handles the signal then we won't terminate
/// (and we shouldn't!) until the process itself later exits.
pub fn exec_replace(&self) -> CargoResult<()> {
self.exec()
imp::exec_replace(self)
}

/// Execute the process, returning the stdio output, or an error if non-zero exit status.
Expand Down Expand Up @@ -324,3 +317,51 @@ pub fn process<T: AsRef<OsStr>>(cmd: T) -> ProcessBuilder {
jobserver: None,
}
}

#[cfg(unix)]
mod imp {
use CargoResult;
use std::os::unix::process::CommandExt;
use util::{process_error, ProcessBuilder};

pub fn exec_replace(process_builder: &ProcessBuilder) -> CargoResult<()> {
let mut command = process_builder.build_command();
let error = command.exec();
Err(::util::CargoError::from(error)
.context(process_error(
&format!("could not execute process {}", process_builder),
None,
None,
))
.into())
}
}

#[cfg(windows)]
mod imp {
extern crate winapi;

use CargoResult;
use util::{process_error, ProcessBuilder};
use self::winapi::shared::minwindef::{BOOL, DWORD, FALSE, TRUE};
use self::winapi::um::consoleapi::SetConsoleCtrlHandler;

unsafe extern "system" fn ctrlc_handler(_: DWORD) -> BOOL {
// Do nothing. Let the child process handle it.
TRUE
}

pub fn exec_replace(process_builder: &ProcessBuilder) -> CargoResult<()> {
unsafe {
if SetConsoleCtrlHandler(Some(ctrlc_handler), TRUE) == FALSE {
return Err(process_error(
"Could not set Ctrl-C handler.",
None,
None).into());
}
}

// Just exec the process as normal.
process_builder.exec()
}
}

0 comments on commit 044a525

Please sign in to comment.