Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Process::signal(0) returns Ok() when child process was killed externally #13124

Closed
japaric opened this issue Mar 25, 2014 · 0 comments · Fixed by #13131
Closed

Process::signal(0) returns Ok() when child process was killed externally #13124

japaric opened this issue Mar 25, 2014 · 0 comments · Fixed by #13131

Comments

@japaric
Copy link
Member

japaric commented Mar 25, 2014

Scripts to reproduce

// loopback.rs
use std::io::stdio::stdin;

fn main() {
    for line in stdin().lines() {
        match line {
            Ok(line) => print!("{}", line),
            Err(e)   => fail!("whoops: {}", e),
        }
    }
}
// heartbeat.rs
use std::io::process::Process;
use std::io::timer::Timer;

fn main() {
    let mut timer = Timer::new().unwrap();
    let periodic = timer.periodic(1000);

    let mut child = Process::new("./loopback", []).unwrap();

    loop {
        match child.signal(0) {
            Err(_) => fail!("child process is dead"),
            Ok(_)  => println!("*heartbeat*"),
        }

        periodic.recv();
    }
}

Steps to reproduce:

$ ./heartbeat
$ killall loopback
# heartbeat never ends

Version:

rustc 0.10-pre (e6468a8 2014-03-24 10:01:57 -0700)
host: x86_64-unknown-linux-gnu

Probably related to issue #13123

bors added a commit that referenced this issue Mar 28, 2014
It turns out that on linux, and possibly other platforms, child processes will
continue to accept signals until they have been *reaped*. This means that once
the child has exited, it will succeed to receive signals until waitpid() has
been invoked on it.

This is unfortunate behavior, and differs from what is seen on OSX and windows.
This commit changes the behavior of Process::signal() to be the same across
platforms, and updates the documentation of Process::kill() to note that when
signaling a foreign process it may accept signals until reaped.

Implementation-wise, this invokes waitpid() with WNOHANG before each signal to
the child to ensure that if the child has exited that we will reap it. Other
possibilities include installing a SIGCHLD signal handler, but at this time I
believe that that's too complicated.

Closes #13124
alexcrichton added a commit to alexcrichton/rust that referenced this issue Feb 10, 2016
On Unix we have to be careful to not call `waitpid` twice, but we don't have to
be careful on Windows due to the way process handles work there. As a result the
cached `Option<ExitStatus>` is only necessary on Unix, and it's also just an
implementation detail of the Unix module.

At the same time. also update some code in `kill` on Unix to avoid a wonky
waitpid with WNOHANG. This was added in 0e190b9 to solve rust-lang#13124, but the
`signal(0)` method is not supported any more so there's no need to for this
workaround. I believe that this is no longer necessary as it's not really doing
anything.
flip1995 pushed a commit to flip1995/rust that referenced this issue Jul 25, 2024
…exendoo

Lintcheck: Update crates and expand CI testset to 200 crates

This PR adds a new `ci_crates.toml` to lintcheck for our CI. The 200 crates take about 14 minutes, which is slightly more than the 10 I aimed for but still reasonable. The testset is constructed from:

* 5 crates that compile to binaries
* 4 crates that have been mentioned in ICE issues
* 1 crates "random" crates from `lintcheck_crates.toml`
* 190 crates from the top 200 crates from crates.io

During testing, I noticed a few panics in lintcheck. I've fixed them where possible, or at least improved the error message.

The new test set generates 500+ MB of json lints, which are compressed to a ~24mb artifact.

---

This PR also updates our `lintcheck_crates.toml`. I mainly updated the versions, removed some very outdated crates, and added some new ones. I targeted 25 crates as those are pretty fast to lint and a good precursor for our CI.

---

Optional TODO:

* It's likely that some crates are compiled several times. We could potentially safe some time, by using `--recursive` in our CI.
    This is something I want to investigate, but it shouldn't be a blocker for this PR.

---

r? `@Alexendoo`

changelog: none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant