Added interrupt function for std::process::Child

[JonathanWoollett-Light]

Added an interrupt function to std::process::Child which acts similar to the kill function but uses the SIGINT signal instead of SIGKILL. This allows the process to catch the signal and gracefully exit.

When working with binaries this is very important, considering a test where you need to spawn a server, interact with this server, then shutdown the server (this server may create local resources which require manual cleanup) (a little easier and safer than unsafe { libc::kill(child.id() as i32, libc::SIGINT); }).

#[test]
fn stress_test() {
    let server = std::process::Command::new("cargo")
            .args(["run", "--bin", "my-server"])
            .spawn()
            .unwrap();
    // Some interaction
    server.interrupt();
   // Some finishing work
}

It is better practice to interrupt applications to allow graceful shutdowns than to kill applications as this may leave remnants the application was not able to clean up. Really killing should only be used when applications are un-trusted or refuse to gracefully exit.

I would argue, interrupt should probably be more commonly used than kill.

Although this is not changed in this PR, it might make sense in the future for std::process::Child to implement std::ops::Drop like:

impl std::ops::Drop for std::process::Child {
    fn drop(&mut self) {
        self.interrupt();
    } 
}

The Fuschia implementation currently simply mimics kill I will look into Fuschia to see if I can fix this (I currently know basically nothing here).

ACP: rust-lang/libs-team#97

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @joshtriplett (or someone else) soon.

Please see the contribution instructions for more information.

[rust-log-analyzer]

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

configure: rust.debug-assertions := True
configure: rust.overflow-checks := True
configure: llvm.assertions      := True
configure: dist.missing-tools   := True
configure: build.configure-args := ['--enable-sccache', '--disable-manage-submodu ...
configure: writing `config.toml` in current directory
configure: 
configure: run `python /checkout/x.py --help`
Attempting with retry: make prepare
---
    Checking addr2line v0.16.0
error[E0599]: no method named `interrupt` found for struct `Process` in the current scope
    --> library/std/src/process.rs:1891:21
     |
1891 |         self.handle.interrupt()
     |                     ^^^^^^^^^ method not found in `Process`
    ::: library/std/src/sys/windows/process.rs:584:1
     |
584  | pub struct Process {
584  | pub struct Process {
     | ------------------ method `interrupt` not found for this struct
For more information about this error, try `rustc --explain E0599`.
error: could not compile `std` due to previous error
Build completed unsuccessfully in 0:00:21

[ChrisDenton]

For a new API you'll also need to create an API Change Proposal (ACP) to get approval from the libs-api-team.

[jkugelman]

SIGINT is the signal sent when a user presses Ctrl+C. Programs that kill other programs conventionally send SIGTERM.

[Dylan-DPC]

@JonathanWoollett-Light any updates on this?

[JonathanWoollett-Light]

@JonathanWoollett-Light any updates on this?

Seems to maintain consistently with other APIs little will change.

It may be:

impl Child {
    #[cfg(unix)]
    fn send_signal(x: i32) {
        unsafe { libc::send_signal(self.pid() as i32, x) }
    }
}

But you will still need to depend on libc to get the signal numbers.

If you want these kind of safe wrappers, just use Nix https://github.com/nix-rust/nix.

[Dylan-DPC]

@JonathanWoollett-Light thanks. does this still need to be a draft? Reviewers are likely not going to review it when a pr is in draft. If it is ready you can click on »Ready for review« and use @rustbot ready to switch to the review labels.

[bors]

☔ The latest upstream changes (presumably #117285) made this pull request unmergeable. Please resolve the merge conflicts.

[Dylan-DPC]

@JonathanWoollett-Light any updates on this? not sure what is pending here. IF nothing is you can mark it as ready

[joshtriplett]

r? libs

[JonathanWoollett-Light]

@Dylan-DPC I need to rebase the PR, I should be able to get around to this by the end of next week.

[Dylan-DPC]

Closing this as inactive. Feel free to reöpen this pr or create a new pr if you get the time to work on this. Thanks