Skip to content

Handle SIGPIPE (when running choosers, currently) #2635

@nc7s

Description

@nc7s

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:

  1. Just exit, either with some handling or continue letting it panic. It already panics, so this won't hurt more.
  2. Retry running the chooser. As long as the chooser is idempotent, we are safe, albeit consuming more computation and time.
  3. 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:

just/src/subcommand.rs

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(),
          });
        }
      }
    }

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions