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

Change to non-line buffered output if output is not a TTY #64861

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
213 changes: 160 additions & 53 deletions src/libstd/io/stdio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::io::prelude::*;
use crate::cell::RefCell;
use crate::fmt;
use crate::io::lazy::Lazy;
use crate::io::{self, Initializer, BufReader, LineWriter, IoSlice, IoSliceMut};
use crate::io::{self, Initializer, BufReader, BufWriter, LineWriter, IoSlice, IoSliceMut};
use crate::sync::{Arc, Mutex, MutexGuard};
use crate::sys::stdio;
use crate::sys_common::remutex::{ReentrantMutex, ReentrantMutexGuard};
Expand Down Expand Up @@ -84,6 +84,11 @@ impl Read for StdinRaw {
Initializer::nop()
}
}
impl StdoutRaw {
fn should_be_line_buffered(&self) -> bool {
self.0.should_be_line_buffered()
}
}
impl Write for StdoutRaw {
fn write(&mut self, buf: &[u8]) -> io::Result<usize> { self.0.write(buf) }

Expand All @@ -103,48 +108,31 @@ impl Write for StderrRaw {
fn flush(&mut self) -> io::Result<()> { self.0.flush() }
}

enum Maybe<T> {
Real(T),
Fake,
}
/// Maps `EBADF` errors for reading and writing to success.
struct HandleEbadf<T>(T);

impl<W: io::Write> io::Write for Maybe<W> {
impl<W: Write> Write for HandleEbadf<W> {
fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
match *self {
Maybe::Real(ref mut w) => handle_ebadf(w.write(buf), buf.len()),
Maybe::Fake => Ok(buf.len())
}
handle_ebadf(self.0.write(buf), buf.len())
}

fn write_vectored(&mut self, bufs: &[IoSlice<'_>]) -> io::Result<usize> {
let total = bufs.iter().map(|b| b.len()).sum();
match self {
Maybe::Real(w) => handle_ebadf(w.write_vectored(bufs), total),
Maybe::Fake => Ok(total),
}
handle_ebadf(self.0.write_vectored(bufs), total)
}

fn flush(&mut self) -> io::Result<()> {
match *self {
Maybe::Real(ref mut w) => handle_ebadf(w.flush(), ()),
Maybe::Fake => Ok(())
}
handle_ebadf(self.0.flush(), ())
}
}

impl<R: io::Read> io::Read for Maybe<R> {
impl<R: Read> Read for HandleEbadf<R> {
fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
match *self {
Maybe::Real(ref mut r) => handle_ebadf(r.read(buf), 0),
Maybe::Fake => Ok(0)
}
handle_ebadf(self.0.read(buf), 0)
}

fn read_vectored(&mut self, bufs: &mut [IoSliceMut<'_>]) -> io::Result<usize> {
match self {
Maybe::Real(r) => handle_ebadf(r.read_vectored(bufs), 0),
Maybe::Fake => Ok(0)
}
handle_ebadf(self.0.read_vectored(bufs), 0)
}
unsafe fn initializer(&self) -> Initializer {
self.0.initializer()
}
}

Expand All @@ -155,6 +143,121 @@ fn handle_ebadf<T>(r: io::Result<T>, default: T) -> io::Result<T> {
}
}

enum StdioBufferKind {
LineBuffered,
Buffered,
Unbuffered,
}

/// A reader that can either be buffered or fake.
///
/// If it is fake, all reads succeed and indicate end-of-file.
enum StdioReader<R: Read> {
Buffered(BufReader<HandleEbadf<R>>),
Fake,
}

impl<R: Read> StdioReader<R> {
fn new_buffered(reader: R) -> StdioReader<R> {
let cap = stdio::STDIN_BUF_SIZE;
StdioReader::Buffered(BufReader::with_capacity(cap, HandleEbadf(reader)))
}
fn new_fake() -> StdioReader<R> {
StdioReader::Fake
}
}

impl<R: Read> Read for StdioReader<R> {
fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
match self {
StdioReader::Buffered(i) => i.read(buf),
StdioReader::Fake => Ok(0),
}
}
fn read_vectored(&mut self, bufs: &mut [io::IoSliceMut<'_>]) -> io::Result<usize> {
match self {
StdioReader::Buffered(i) => i.read_vectored(bufs),
StdioReader::Fake => Ok(0),
}
}
#[inline]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this inline necessary?

unsafe fn initializer(&self) -> Initializer {
match self {
StdioReader::Buffered(i) => i.initializer(),
StdioReader::Fake => Initializer::nop(),
}
}
}

impl<R: Read> BufRead for StdioReader<R> {
fn fill_buf(&mut self) -> io::Result<&[u8]> {
match self {
StdioReader::Buffered(i) => i.fill_buf(),
StdioReader::Fake => Ok(&[]),
}
}
fn consume(&mut self, amt: usize) {
match self {
StdioReader::Buffered(i) => i.consume(amt),
StdioReader::Fake => (),
}
}
}

/// A writer that can either be line-buffered, buffered, unbuffered or fake.
///
/// If it is fake, all outputs just succeed and are dropped silently.
enum StdioWriter<W: Write> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was a comment in the original FIXME suggesting we should call flush when dropping the handle if we're panicking. Did you have any thoughts on this @tbu-?

LineBuffered(LineWriter<HandleEbadf<W>>),
Buffered(BufWriter<HandleEbadf<W>>),
Unbuffered(HandleEbadf<W>),
Fake,
}

impl<W: Write> StdioWriter<W> {
fn new(writer: W, kind: StdioBufferKind) -> StdioWriter<W> {
let writer = HandleEbadf(writer);
match kind {
StdioBufferKind::LineBuffered =>
StdioWriter::LineBuffered(LineWriter::new(writer)),
StdioBufferKind::Buffered =>
StdioWriter::Buffered(BufWriter::new(writer)),
StdioBufferKind::Unbuffered =>
StdioWriter::Unbuffered(writer),
}
}
fn new_fake() -> StdioWriter<W> {
StdioWriter::Fake
}
}

impl<W: Write> Write for StdioWriter<W> {
fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
match self {
StdioWriter::LineBuffered(i) => i.write(buf),
StdioWriter::Buffered(i) => i.write(buf),
StdioWriter::Unbuffered(i) => i.write(buf),
StdioWriter::Fake => Ok(buf.len()),
}
}
fn write_vectored(&mut self, bufs: &[io::IoSlice<'_>]) -> io::Result<usize> {
match self {
StdioWriter::LineBuffered(i) => i.write_vectored(bufs),
StdioWriter::Buffered(i) => i.write_vectored(bufs),
StdioWriter::Unbuffered(i) => i.write_vectored(bufs),
StdioWriter::Fake => Ok(bufs.iter().map(|b| b.len()).sum()),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sum could probably wrap around, seems like it should saturate instead (not sure what's reasonable), or return the first non-empty len.

}
}
fn flush(&mut self) -> io::Result<()> {
match self {
StdioWriter::LineBuffered(i) => i.flush(),
StdioWriter::Buffered(i) => i.flush(),
StdioWriter::Unbuffered(i) => i.flush(),
StdioWriter::Fake => Ok(()),
}
}
}

/// A handle to the standard input stream of a process.
///
/// Each handle is a shared reference to a global buffer of input data to this
Expand All @@ -176,7 +279,7 @@ fn handle_ebadf<T>(r: io::Result<T>, default: T) -> io::Result<T> {
/// an error.
#[stable(feature = "rust1", since = "1.0.0")]
pub struct Stdin {
inner: Arc<Mutex<BufReader<Maybe<StdinRaw>>>>,
inner: Arc<Mutex<StdioReader<StdinRaw>>>,
}

/// A locked reference to the `Stdin` handle.
Expand All @@ -194,7 +297,7 @@ pub struct Stdin {
/// an error.
#[stable(feature = "rust1", since = "1.0.0")]
pub struct StdinLock<'a> {
inner: MutexGuard<'a, BufReader<Maybe<StdinRaw>>>,
inner: MutexGuard<'a, StdioReader<StdinRaw>>,
}

/// Constructs a new handle to the standard input of the current process.
Expand Down Expand Up @@ -240,21 +343,21 @@ pub struct StdinLock<'a> {
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
pub fn stdin() -> Stdin {
static INSTANCE: Lazy<Mutex<BufReader<Maybe<StdinRaw>>>> = Lazy::new();
static INSTANCE: Lazy<Mutex<StdioReader<StdinRaw>>> = Lazy::new();
return Stdin {
inner: unsafe {
INSTANCE.get(stdin_init).expect("cannot access stdin during shutdown")
},
};

fn stdin_init() -> Arc<Mutex<BufReader<Maybe<StdinRaw>>>> {
fn stdin_init() -> Arc<Mutex<StdioReader<StdinRaw>>> {
// This must not reentrantly access `INSTANCE`
let stdin = match stdin_raw() {
Ok(stdin) => Maybe::Real(stdin),
_ => Maybe::Fake
Ok(stdin) => StdioReader::new_buffered(stdin),
_ => StdioReader::new_fake(),
};

Arc::new(Mutex::new(BufReader::with_capacity(stdio::STDIN_BUF_SIZE, stdin)))
Arc::new(Mutex::new(stdin))
}
}

Expand Down Expand Up @@ -398,10 +501,7 @@ impl fmt::Debug for StdinLock<'_> {
/// [`io::stdout`]: fn.stdout.html
#[stable(feature = "rust1", since = "1.0.0")]
pub struct Stdout {
// FIXME: this should be LineWriter or BufWriter depending on the state of
// stdout (tty or not). Note that if this is not line buffered it
// should also flush-on-panic or some form of flush-on-abort.
inner: Arc<ReentrantMutex<RefCell<LineWriter<Maybe<StdoutRaw>>>>>,
inner: Arc<ReentrantMutex<RefCell<StdioWriter<StdoutRaw>>>>,
}

/// A locked reference to the `Stdout` handle.
Expand All @@ -418,7 +518,7 @@ pub struct Stdout {
/// [`Stdout::lock`]: struct.Stdout.html#method.lock
#[stable(feature = "rust1", since = "1.0.0")]
pub struct StdoutLock<'a> {
inner: ReentrantMutexGuard<'a, RefCell<LineWriter<Maybe<StdoutRaw>>>>,
inner: ReentrantMutexGuard<'a, RefCell<StdioWriter<StdoutRaw>>>,
}

/// Constructs a new handle to the standard output of the current process.
Expand Down Expand Up @@ -464,20 +564,27 @@ pub struct StdoutLock<'a> {
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
pub fn stdout() -> Stdout {
static INSTANCE: Lazy<ReentrantMutex<RefCell<LineWriter<Maybe<StdoutRaw>>>>> = Lazy::new();
static INSTANCE: Lazy<ReentrantMutex<RefCell<StdioWriter<StdoutRaw>>>> = Lazy::new();
return Stdout {
inner: unsafe {
INSTANCE.get(stdout_init).expect("cannot access stdout during shutdown")
},
};

fn stdout_init() -> Arc<ReentrantMutex<RefCell<LineWriter<Maybe<StdoutRaw>>>>> {
fn stdout_init() -> Arc<ReentrantMutex<RefCell<StdioWriter<StdoutRaw>>>> {
// This must not reentrantly access `INSTANCE`
let stdout = match stdout_raw() {
Ok(stdout) => Maybe::Real(stdout),
_ => Maybe::Fake,
Ok(stdout) => {
let buffering = if stdout.should_be_line_buffered() {
StdioBufferKind::LineBuffered
} else {
StdioBufferKind::Buffered
};
StdioWriter::new(stdout, buffering)
},
_ => StdioWriter::new_fake(),
};
Arc::new(ReentrantMutex::new(RefCell::new(LineWriter::new(stdout))))
Arc::new(ReentrantMutex::new(RefCell::new(stdout)))
}
}

Expand Down Expand Up @@ -565,7 +672,7 @@ impl fmt::Debug for StdoutLock<'_> {
/// an error.
#[stable(feature = "rust1", since = "1.0.0")]
pub struct Stderr {
inner: Arc<ReentrantMutex<RefCell<Maybe<StderrRaw>>>>,
inner: Arc<ReentrantMutex<RefCell<StdioWriter<StderrRaw>>>>,
}

/// A locked reference to the `Stderr` handle.
Expand All @@ -581,7 +688,7 @@ pub struct Stderr {
/// an error.
#[stable(feature = "rust1", since = "1.0.0")]
pub struct StderrLock<'a> {
inner: ReentrantMutexGuard<'a, RefCell<Maybe<StderrRaw>>>,
inner: ReentrantMutexGuard<'a, RefCell<StdioWriter<StderrRaw>>>,
}

/// Constructs a new handle to the standard error of the current process.
Expand Down Expand Up @@ -623,18 +730,18 @@ pub struct StderrLock<'a> {
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
pub fn stderr() -> Stderr {
static INSTANCE: Lazy<ReentrantMutex<RefCell<Maybe<StderrRaw>>>> = Lazy::new();
static INSTANCE: Lazy<ReentrantMutex<RefCell<StdioWriter<StderrRaw>>>> = Lazy::new();
return Stderr {
inner: unsafe {
INSTANCE.get(stderr_init).expect("cannot access stderr during shutdown")
},
};

fn stderr_init() -> Arc<ReentrantMutex<RefCell<Maybe<StderrRaw>>>> {
fn stderr_init() -> Arc<ReentrantMutex<RefCell<StdioWriter<StderrRaw>>>> {
// This must not reentrantly access `INSTANCE`
let stderr = match stderr_raw() {
Ok(stderr) => Maybe::Real(stderr),
_ => Maybe::Fake,
Ok(stderr) => StdioWriter::new(stderr, StdioBufferKind::Unbuffered),
_ => StdioWriter::new_fake(),
};
Arc::new(ReentrantMutex::new(RefCell::new(stderr)))
}
Expand Down
3 changes: 3 additions & 0 deletions src/libstd/sys/cloudabi/stdio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ impl Stdout {
pub fn new() -> io::Result<Stdout> {
Ok(Stdout(()))
}
pub fn should_be_line_buffered(&self) -> bool {
true
}
}

impl io::Write for Stdout {
Expand Down
4 changes: 4 additions & 0 deletions src/libstd/sys/sgx/stdio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ impl io::Read for Stdin {

impl Stdout {
pub fn new() -> io::Result<Stdout> { Ok(Stdout(())) }
pub fn should_be_line_buffered(&self) -> bool {
// FIXME: Implement me.
true
}
}

impl io::Write for Stdout {
Expand Down
5 changes: 5 additions & 0 deletions src/libstd/sys/unix/stdio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ impl io::Read for Stdin {

impl Stdout {
pub fn new() -> io::Result<Stdout> { Ok(Stdout(())) }
pub fn should_be_line_buffered(&self) -> bool {
unsafe {
libc::isatty(libc::STDOUT_FILENO) != 0
}
}
}

impl io::Write for Stdout {
Expand Down
4 changes: 4 additions & 0 deletions src/libstd/sys/vxworks/stdio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ impl io::Read for Stdin {

impl Stdout {
pub fn new() -> io::Result<Stdout> { Ok(Stdout(())) }
pub fn should_be_line_buffered(&self) -> bool {
// FIXME: Implement me.
true
}
}

impl io::Write for Stdout {
Expand Down
Loading