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

Thread panic - broken pipe when piping output to pager #15

Open
anthroid opened this issue Feb 8, 2021 · 9 comments · May be fixed by #21
Open

Thread panic - broken pipe when piping output to pager #15

anthroid opened this issue Feb 8, 2021 · 9 comments · May be fixed by #21

Comments

@anthroid
Copy link

anthroid commented Feb 8, 2021

To reproduce, any of these commands cause a thread panic when piping the output of as-tree to a pager (tested with less, more, most, bat):

ls /usr/lib | as-tree | less (same when replacing less with more, most, bat, etc)

fd | as-tree | bat

fd | as-tree | cat | less

With RUST_BACKTRACE=1, the trace output is:

thread 'main' panicked at 'failed printing to stdout: Broken pipe (os error 32)', library/std/src/io/stdio.rs:1021:9
stack backtrace:
   0: rust_begin_unwind
             at /build/rust/src/rustc-1.49.0-src/library/std/src/panicking.rs:495:5
   1: std::panicking::begin_panic_fmt
             at /build/rust/src/rustc-1.49.0-src/library/std/src/panicking.rs:437:5
   2: std::io::stdio::print_to
             at /build/rust/src/rustc-1.49.0-src/library/std/src/io/stdio.rs:1021:9
   3: std::io::stdio::_print
             at /build/rust/src/rustc-1.49.0-src/library/std/src/io/stdio.rs:1033:5
   4: as_tree::PathTrie::_print
   5: as_tree::PathTrie::_print
   6: as_tree::PathTrie::_print
   7: as_tree::main
@anthroid anthroid changed the title Thread panic - broken pipe when piping to pager Thread panic - broken pipe when piping output to pager Feb 8, 2021
@anthroid
Copy link
Author

anthroid commented Feb 8, 2021

Just tested a little further. I suspected this is was because as-tree was getting cut off while writing to stdout, so I tested a few short cases:

ls ~ | as-tree | less
No issue (assuming only a single page of output)

Using a longer output:
ls /usr/lib | as-tree | less

  • Scroll all the way to the end of the buffer, no issue.
  • Don't scroll to the end, press q - Thread panic

@cstrahan
Copy link

I was just about to open an issue for precisely this. Would be great to have as-tree gracefully exit in this scenario.

@jez
Copy link
Owner

jez commented Feb 10, 2021 via email

@cstrahan
Copy link

@jez assuming your terminal height is less than 10,000 characters (quite probable 😅), this should do the trick:

seq 10000 | as-tree | less -FirSwX

then press q to exit less.

@jez
Copy link
Owner

jez commented Feb 10, 2021

@cstrahan awesome, thanks for the concise reproducer. I see

...
├── 1048
├── 1049
thread 'main' panicked at 'failed printing to stdout: Broken pipe (os error 32)', src/libstd/io/stdio.rs:805:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
[1]    71608 done       seq 10000 |
       71609 abort      as-tree |
       71610 done       less -FirSwX

when i run it, which looks like the above.

@cstrahan
Copy link

If you want a reproducer that doesn't depend on a tty:

This uses a sleep so that the "a" char is piped into as-tree after the bash process exits:

$ (sleep 1; echo a) | as-tree | bash -c ''
thread 'main' panicked at 'failed printing to stdout: Broken pipe (os error 32)', src/libstd/io/stdio.rs:878:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

If you have process at the end of your pipeline that consumes all of as-tree's stdout then you don't run into this problem because that guarantees that the process outlives as-tree:

$ (sleep 1; echo a) | as-tree | bash -c 'cat /dev/stdin >/dev/null'

The problem with using a pager (right now) is that it won't necessary consume all of its stdin (e.g. as-tree's stdout): it might buffer just enough to show within the dimensions of the terminal, and then the user might exit the pager before the pager ever consumes all of stdin. If the pager doesn't consume all of its stdin, then as-tree will be blocked on writing to as-tree's stdout and then become unblocked with a write failure as the pipe is broken.

@cstrahan
Copy link

You can remove the sleep from my previous examples by using stdbuf to disable output buffering for as-tree's stdout; also, the bash -c '' can be replaced with a subshell that just immediately exits:

$ ( echo a ) | stdbuf -o0 as-tree | ( exit )
thread 'main' panicked at 'failed printing to stdout: Broken pipe (os error 32)', src/libstd/io/stdio.rs:878:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
$ ( echo a ) | stdbuf -o0 as-tree | ( cat /dev/stdin > /dev/null )

@cstrahan
Copy link

FWIW, looks like this is how fd handles this:

https://github.com/sharkdp/fd/blob/b928af7d9c4c174791cb23a58bbc4e724c2fbb9b/src/output.rs#L36-L39

    if r.is_err() {
        // Probably a broken pipe. Exit gracefully.
        process::exit(ExitCode::GeneralError.into());
    }

@anthroid
Copy link
Author

Similar to how exa handles it as well:
https://github.com/ogham/exa/blob/13b91cced4cab012413b25c9d3e30c63548639d0/src/main.rs#L75-L78

    Err(e) if e.kind() == ErrorKind::BrokenPipe => {
        warn!("Broken pipe error: {}", e);
        exit(exits::SUCCESS);
    }

sharkdp added a commit to sharkdp/as-tree that referenced this issue Jul 5, 2021
Fixes a panic when writing to a closed pipe by explicitly ignoring this
error type.

This commit also changes the behavior of `as-tree` on empty inputs which
would previously result in a (superfluous) newline.

closes jez#15
closes jez#19
@sharkdp sharkdp linked a pull request Jul 5, 2021 that will close this issue
mre added a commit to lycheeverse/lychee that referenced this issue Mar 1, 2022
Make sure that broken pipes (e.g. when a reader of a
pipe prematurely exits during execution) get handled gracefully.
This change also moves some error messages to stderr by using
eprintln.

More info: jez/as-tree#15
mre added a commit to lycheeverse/lychee that referenced this issue Mar 2, 2022
Make sure that broken pipes (e.g. when a reader of a
pipe prematurely exits during execution) get handled gracefully.
This change also moves some error messages to stderr by using
eprintln.

More info: jez/as-tree#15
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.

3 participants