-
Notifications
You must be signed in to change notification settings - Fork 578
Description
This started from a bug report in Debian. On s390x (and as later discovered, also on ppc64el), chooser tests (in tests/choose.rs) sometimes fail due to SIGPIPE, i.e. child process stopped reading before writes from parent process finished, IIUC. I added a patch setting signal(SIGPIPE, SIG_DFL)
, to no avail. (Now looking at it again, this is obviously in the wrong direction, but at least didn't cause more trouble.)
BurntSushi explained in an issue that println!()
would panic on SIGPIPE (probably also other errors), thus writeln!(std::io::stdout(), ...)
is a better choice, giving you the choice to handle the error. However, the code referred by that issue didn't actually handle it, only showed this pattern, then simply bubbled up the error with ?
. The part of just that deals with the chooser child process did the same. OTOH, ripgrep does "handle" it by exiting on SIGPIPE.
In our case, head -n1
only reads the 1st line, then exits; the exit-2 script doesn't read anything, it merely exits with code 2. Basically exited before the main just process finished writing. User defined choosers might pose the same problem.
Why only on s390x (and ppc64el)? probably because they are slower, particularly, slower in handling signals and/or pipes.
AFAICT, there are these few options:
- Just exit, either with some handling or continue letting it panic. It already panics, so this won't hurt more.
- Retry running the chooser. As long as the chooser is idempotent, we are safe, albeit consuming more computation and time.
- Continue with the output already printed by the chooser. Since SIGPIPE is sent when the child process stopped reading, we can assume it's already "satisfied" with the input given so far, like
head -n1
discussed above.
FWIW, option 3 runs through all existing tests with success, with this:
Lines 242 to 252 in 7c63fb6
for recipe in recipes { | |
writeln!( | |
child.stdin.as_mut().unwrap(), | |
"{}", | |
recipe.namepath.spaced() | |
) | |
.map_err(|io_error| Error::ChooserWrite { | |
io_error, | |
chooser: chooser.clone(), | |
})?; | |
} |
changed to:
for recipe in recipes {
if let Err(e) = writeln!(
child.stdin.as_mut().unwrap(),
"{}",
recipe.namepath.spaced()
) {
if e.kind() != std::io::ErrorKind::BrokenPipe {
return Err(Error::ChooserWrite {
io_error: e,
chooser: chooser.clone(),
});
}
}
}