coreutils icon indicating copy to clipboard operation
coreutils copied to clipboard

Discussion: Guidelines for printing to stdout

Open tertsdiepraam opened this issue 3 years ago • 4 comments

TLDR: What should we recommend for printing to stdout?

  • print!/println!?
  • write!/writeln! to locked stdout?
  • write!/writeln! on a BufWriter?
  • Some other solution?

Printing to stdout sounds simple, but Rust (rightfully) exposes a lot of complexity that other languages attempt to hide. We have several reasons to care about this complexity:

  • We want to gracefully handle a variety of edge cases (such as https://github.com/uutils/coreutils/issues/2925).
  • We want to make the utils as fast or faster than GNU (or at the very least in the same ballpark as GNU) and IO can be a significant bottleneck. This can really make a difference, for instance in this new PR: https://github.com/uutils/coreutils/pull/3092

The print!/println! macros are relatively unsuitable for both of these cases. It will lock stdout for every call and only does line buffering by default (I believe). Additionally, it will panic on a write error, so handling edge cases becomes difficult (if not impossible).

Click to see the function that `println!` calls

Source: https://doc.rust-lang.org/src/std/io/stdio.rs.html

/// Write `args` to the capture buffer if enabled and possible, or `global_s`
/// otherwise. `label` identifies the stream in a panic message.
///
/// This function is used to print error messages, so it takes extra
/// care to avoid causing a panic when `local_s` is unusable.
/// For instance, if the TLS key for the local stream is
/// already destroyed, or if the local stream is locked by another
/// thread, it will just fall back to the global stream.
///
/// However, if the actual I/O causes an error, this function does panic.
fn print_to<T>(args: fmt::Arguments<'_>, global_s: fn() -> T, label: &str)
where
    T: Write,
{
    if OUTPUT_CAPTURE_USED.load(Ordering::Relaxed)
        && OUTPUT_CAPTURE.try_with(|s| {
            // Note that we completely remove a local sink to write to in case
            // our printing recursively panics/prints, so the recursive
            // panic/print goes to the global sink instead of our local sink.
            s.take().map(|w| {
                let _ = w.lock().unwrap_or_else(|e| e.into_inner()).write_fmt(args);
                s.set(Some(w));
            })
        }) == Ok(Some(()))
    {
        // Succesfully wrote to capture buffer.
        return;
    }

    if let Err(e) = global_s().write_fmt(args) {
        panic!("failed printing to {}: {}", label, e);
    }
}

There is a an alternative using Write on StdoutLock (possibly wrapped in a BufWriter). This gives us much more control with locking, buffering and error handling as write! returns a result. However, this imposes a surprisingly heavy maintenance burden.

The current version of ls uses the BufWriter approach and illustrates this burden nicely. First, the BufWriter is passed to many functions, which is not horrible, but becomes quite annoying. Second, the errors were reported in the wrong order, because they were written to an unbuffered stderr, but interleaved with a buffered stdout. The output that was generated before an error was therefore buffered, then the error was written and only afterwards was the buffered output written to the terminal. Currently, this is fixed by explicitly flushing the output before writing an error. This works, but is not ideal and not something I'd want to see implemented in every util.

More info on this issue for ls can be found in https://github.com/uutils/coreutils/pull/2809 and https://github.com/uutils/coreutils/issues/2785

So I'd like to discuss this and then set our conclusions as guidelines for all utils in the project. Any ideas are welcome! Also, if I got anything wrong with this post please correct me! I would also love to hear how other Rust programs deal with this issue.

tertsdiepraam avatar Feb 07 '22 13:02 tertsdiepraam

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Feb 07 '25 05:02 stale[bot]

I guess it is still relevant :)

sylvestre avatar Feb 07 '25 13:02 sylvestre

There are a few cases in the close-stdout.sh GNU test that fail because println! causes a panic when stdout points to /dev/full.

karlmcdowall avatar Feb 12 '25 00:02 karlmcdowall

First, #5907 makes me think that we should look into the Write with BufWriter approach for stderr as well. Given that, should we provide macros that abstract away that logic and use them throughout coreutils? I'm thinking of something like uuio_init!() that expands to a static STDOUT: LazyLock<...> (and similar for stderr), and then uu(e)print(ln)! that write to those (I think we can always reference them with crate::STDOUT as long as we only call uuio_init! at the top of each crate). I'm not sure how this would work with interleaving; we might need to add more macros that allow us to explicitly flush the one we aren't writing to. Is this something that's worth pursuing? I'd be happy to write a PR to uucore if so.

Qelxiros avatar May 09 '25 01:05 Qelxiros