From c6669448da9ed7b174d495e58251f70f26ad4a5a Mon Sep 17 00:00:00 2001 From: Andrea Corbellini Date: Wed, 29 Oct 2025 20:35:58 -0700 Subject: [PATCH] stdio: make stdout block-buffered when not associated to a terminal --- library/std/src/io/mod.rs | 2 +- library/std/src/io/stdio.rs | 109 +++++++++++++++++++++--- library/std/src/io/stdio/tests.rs | 96 ++++++++++++++++++++- library/std/src/os/fd/owned.rs | 21 +++++ library/std/src/os/windows/io/handle.rs | 21 +++++ library/std/src/os/windows/io/raw.rs | 18 ++++ 6 files changed, 251 insertions(+), 16 deletions(-) diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index 25a4661a0bc9c..beeebef9081cd 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -346,7 +346,7 @@ mod util; const DEFAULT_BUF_SIZE: usize = crate::sys::io::DEFAULT_BUF_SIZE; -pub(crate) use stdio::cleanup; +pub(crate) use stdio::{StderrRaw, StdinRaw, StdoutRaw, cleanup}; struct Guard<'a> { buf: &'a mut Vec, diff --git a/library/std/src/io/stdio.rs b/library/std/src/io/stdio.rs index 2d80fe49e80a7..67e41acd04ef5 100644 --- a/library/std/src/io/stdio.rs +++ b/library/std/src/io/stdio.rs @@ -8,7 +8,8 @@ use crate::fmt; use crate::fs::File; use crate::io::prelude::*; use crate::io::{ - self, BorrowedCursor, BufReader, IoSlice, IoSliceMut, LineWriter, Lines, SpecReadByte, + self, BorrowedCursor, BufReader, BufWriter, IoSlice, IoSliceMut, LineWriter, Lines, + SpecReadByte, }; use crate::panic::{RefUnwindSafe, UnwindSafe}; use crate::sync::atomic::{Atomic, AtomicBool, Ordering}; @@ -43,19 +44,19 @@ static OUTPUT_CAPTURE_USED: Atomic = AtomicBool::new(false); /// /// This handle is not synchronized or buffered in any fashion. Constructed via /// the `std::io::stdio::stdin_raw` function. -struct StdinRaw(stdio::Stdin); +pub(crate) struct StdinRaw(stdio::Stdin); /// A handle to a raw instance of the standard output stream of this process. /// /// This handle is not synchronized or buffered in any fashion. Constructed via /// the `std::io::stdio::stdout_raw` function. -struct StdoutRaw(stdio::Stdout); +pub(crate) struct StdoutRaw(stdio::Stdout); /// A handle to a raw instance of the standard output stream of this process. /// /// This handle is not synchronized or buffered in any fashion. Constructed via /// the `std::io::stdio::stderr_raw` function. -struct StderrRaw(stdio::Stderr); +pub(crate) struct StderrRaw(stdio::Stderr); /// Constructs a new raw handle to the standard input of this process. /// @@ -576,6 +577,76 @@ impl fmt::Debug for StdinLock<'_> { } } +/// A buffered writer for stdout and stderr. +/// +/// This writer may be either [line-buffered](LineWriter) or [block-buffered](BufWriter), depending +/// on whether the underlying file is a terminal or not. +#[derive(Debug)] +enum StdioBufWriter { + LineBuffered(LineWriter), + BlockBuffered(BufWriter), +} + +impl StdioBufWriter { + /// Wraps a writer using the most appropriate buffering method. + /// + /// If `w` is a terminal, then the resulting `StdioBufWriter` will be line-buffered, otherwise + /// it will be block-buffered. + fn new(w: W) -> Self { + if w.is_terminal() { + Self::LineBuffered(LineWriter::new(w)) + } else { + Self::BlockBuffered(BufWriter::new(w)) + } + } +} + +impl StdioBufWriter { + /// Wraps a writer using a block-buffer with the given capacity. + fn with_capacity(cap: usize, w: W) -> Self { + Self::BlockBuffered(BufWriter::with_capacity(cap, w)) + } +} + +impl Write for StdioBufWriter { + fn write(&mut self, buf: &[u8]) -> io::Result { + match self { + Self::LineBuffered(w) => w.write(buf), + Self::BlockBuffered(w) => w.write(buf), + } + } + fn write_vectored(&mut self, bufs: &[IoSlice<'_>]) -> io::Result { + match self { + Self::LineBuffered(w) => w.write_vectored(bufs), + Self::BlockBuffered(w) => w.write_vectored(bufs), + } + } + fn is_write_vectored(&self) -> bool { + match self { + Self::LineBuffered(w) => w.is_write_vectored(), + Self::BlockBuffered(w) => w.is_write_vectored(), + } + } + fn flush(&mut self) -> io::Result<()> { + match self { + Self::LineBuffered(w) => w.flush(), + Self::BlockBuffered(w) => w.flush(), + } + } + fn write_all(&mut self, buf: &[u8]) -> io::Result<()> { + match self { + Self::LineBuffered(w) => w.write_all(buf), + Self::BlockBuffered(w) => w.write_all(buf), + } + } + fn write_all_vectored(&mut self, bufs: &mut [IoSlice<'_>]) -> io::Result<()> { + match self { + Self::LineBuffered(w) => w.write_all_vectored(bufs), + Self::BlockBuffered(w) => w.write_all_vectored(bufs), + } + } +} + /// A handle to the global standard output stream of the current process. /// /// Each handle shares a global buffer of data to be written to the standard @@ -606,10 +677,9 @@ impl fmt::Debug for StdinLock<'_> { /// [`io::stdout`]: stdout #[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: &'static ReentrantLock>>, + // FIXME: if this is not line buffered it should flush-on-panic or some + // form of flush-on-abort. + inner: &'static ReentrantLock>>, } /// A locked reference to the [`Stdout`] handle. @@ -638,10 +708,10 @@ pub struct Stdout { #[must_use = "if unused stdout will immediately unlock"] #[stable(feature = "rust1", since = "1.0.0")] pub struct StdoutLock<'a> { - inner: ReentrantLockGuard<'a, RefCell>>, + inner: ReentrantLockGuard<'a, RefCell>>, } -static STDOUT: OnceLock>>> = OnceLock::new(); +static STDOUT: OnceLock>>> = OnceLock::new(); /// Constructs a new handle to the standard output of the current process. /// @@ -716,7 +786,7 @@ static STDOUT: OnceLock>>> = OnceLoc pub fn stdout() -> Stdout { Stdout { inner: STDOUT - .get_or_init(|| ReentrantLock::new(RefCell::new(LineWriter::new(stdout_raw())))), + .get_or_init(|| ReentrantLock::new(RefCell::new(StdioBufWriter::new(stdout_raw())))), } } @@ -727,7 +797,7 @@ pub fn cleanup() { let mut initialized = false; let stdout = STDOUT.get_or_init(|| { initialized = true; - ReentrantLock::new(RefCell::new(LineWriter::with_capacity(0, stdout_raw()))) + ReentrantLock::new(RefCell::new(StdioBufWriter::with_capacity(0, stdout_raw()))) }); if !initialized { @@ -736,7 +806,7 @@ pub fn cleanup() { // might have leaked a StdoutLock, which would // otherwise cause a deadlock here. if let Some(lock) = stdout.try_lock() { - *lock.borrow_mut() = LineWriter::with_capacity(0, stdout_raw()); + *lock.borrow_mut() = StdioBufWriter::with_capacity(0, stdout_raw()); } } } @@ -1262,7 +1332,18 @@ macro_rules! impl_is_terminal { )*} } -impl_is_terminal!(File, Stdin, StdinLock<'_>, Stdout, StdoutLock<'_>, Stderr, StderrLock<'_>); +impl_is_terminal!( + File, + Stdin, + StdinLock<'_>, + StdinRaw, + Stdout, + StdoutLock<'_>, + StdoutRaw, + Stderr, + StderrLock<'_>, + StderrRaw, +); #[unstable( feature = "print_internals", diff --git a/library/std/src/io/stdio/tests.rs b/library/std/src/io/stdio/tests.rs index e68d8c29fbce2..2c369553a2574 100644 --- a/library/std/src/io/stdio/tests.rs +++ b/library/std/src/io/stdio/tests.rs @@ -1,6 +1,8 @@ use super::*; +use crate::assert_matches::assert_matches; +use crate::os::fd::{FromRawFd, OwnedFd}; use crate::panic::{RefUnwindSafe, UnwindSafe}; -use crate::sync::mpsc::sync_channel; +use crate::sync::mpsc::{TryRecvError, sync_channel}; use crate::thread; #[test] @@ -164,3 +166,95 @@ where [Start1, Acquire1, Start2, Release1, Acquire2, Release2, Acquire1, Release1] ); } + +#[test] +#[cfg(unix)] +fn stdiobufwriter_line_buffered() { + let mut fd1 = -1; + let mut fd2 = -1; + + if unsafe { + libc::openpty( + &raw mut fd1, + &raw mut fd2, + crate::ptr::null_mut(), + crate::ptr::null(), + crate::ptr::null(), + ) + } < 0 + { + panic!("openpty() failed with {:?}", io::Error::last_os_error()); + } + + let writer = unsafe { File::from_raw_fd(fd1) }; + let mut reader = unsafe { File::from_raw_fd(fd2) }; + + assert!(writer.is_terminal(), "file descriptor returned by openpty() must be a terminal"); + assert!(reader.is_terminal(), "file descriptor returned by openpty() must be a terminal"); + + let (sender, receiver) = sync_channel(64); + + thread::spawn(move || { + loop { + let mut buf = vec![0u8; 1024]; + let size = reader.read(&mut buf[..]).expect("read failed"); + buf.truncate(size); + sender.send(buf); + } + }); + + let mut writer = StdioBufWriter::new(writer); + assert_matches!( + writer, + StdioBufWriter::LineBuffered(_), + "StdioBufWriter should be line-buffered when created from a terminal" + ); + + writer.write_all(b"line 1\n").expect("write failed"); + assert_eq!(receiver.recv().expect("recv failed"), b"line 1\n"); + + writer.write_all(b"line 2\nextra ").expect("write failed"); + assert_eq!(receiver.recv().expect("recv failed"), b"line 2\n"); + + writer.write_all(b"line 3\n").expect("write failed"); + assert_eq!(receiver.recv().expect("recv failed"), b"extra line 3\n"); +} + +#[test] +fn stdiobufwriter_block_buffered() { + let (mut reader, mut writer) = io::pipe().expect("pipe() failed"); + + // Need to convert to an OwnedFd and then into a File because PipeReader/PipeWriter don't + // implement IsTerminal, but that is required by StdioBufWriter + let mut reader = File::from(OwnedFd::from(reader)); + let mut writer = File::from(OwnedFd::from(writer)); + + assert!(!reader.is_terminal(), "file returned by pipe() cannot be a terminal"); + assert!(!writer.is_terminal(), "file returned by pipe() cannot be a terminal"); + + let (sender, receiver) = sync_channel(64); + + thread::spawn(move || { + loop { + let mut buf = vec![0u8; 1024]; + let size = reader.read(&mut buf[..]).expect("read failed"); + buf.truncate(size); + sender.send(buf); + } + }); + + let mut writer = StdioBufWriter::new(writer); + assert_matches!( + writer, + StdioBufWriter::BlockBuffered(_), + "StdioBufWriter should be block-buffered when created from a file that is not a terminal" + ); + + writer.write_all(b"line 1\n").expect("write failed"); + writer.write_all(b"line 2\n").expect("write failed"); + writer.write_all(b"line 3\n").expect("write failed"); + assert_matches!(receiver.try_recv(), Err(TryRecvError::Empty)); + + writer.flush().expect("flush failed"); + assert_eq!(receiver.recv().expect("recv failed"), b"line 1\nline 2\nline 3\n"); +} diff --git a/library/std/src/os/fd/owned.rs b/library/std/src/os/fd/owned.rs index 6a0e7a640028b..7fbf4d88235fa 100644 --- a/library/std/src/os/fd/owned.rs +++ b/library/std/src/os/fd/owned.rs @@ -489,6 +489,13 @@ impl<'a> AsFd for io::StdinLock<'a> { } } +impl AsFd for io::StdinRaw { + #[inline] + fn as_fd(&self) -> BorrowedFd<'_> { + unsafe { BorrowedFd::borrow_raw(0) } + } +} + #[stable(feature = "io_safety", since = "1.63.0")] impl AsFd for io::Stdout { #[inline] @@ -506,6 +513,13 @@ impl<'a> AsFd for io::StdoutLock<'a> { } } +impl AsFd for io::StdoutRaw { + #[inline] + fn as_fd(&self) -> BorrowedFd<'_> { + unsafe { BorrowedFd::borrow_raw(1) } + } +} + #[stable(feature = "io_safety", since = "1.63.0")] impl AsFd for io::Stderr { #[inline] @@ -523,6 +537,13 @@ impl<'a> AsFd for io::StderrLock<'a> { } } +impl AsFd for io::StderrRaw { + #[inline] + fn as_fd(&self) -> BorrowedFd<'_> { + unsafe { BorrowedFd::borrow_raw(2) } + } +} + #[stable(feature = "anonymous_pipe", since = "1.87.0")] #[cfg(not(target_os = "trusty"))] impl AsFd for io::PipeReader { diff --git a/library/std/src/os/windows/io/handle.rs b/library/std/src/os/windows/io/handle.rs index 4fc04b79315f9..04e076a3f7e0c 100644 --- a/library/std/src/os/windows/io/handle.rs +++ b/library/std/src/os/windows/io/handle.rs @@ -562,6 +562,13 @@ impl<'a> AsHandle for crate::io::StdinLock<'a> { } } +impl AsHandle for crate::io::StdinRaw { + #[inline] + fn as_handle(&self) -> BorrowedHandle<'_> { + unsafe { BorrowedHandle::borrow_raw(self.as_raw_handle()) } + } +} + #[stable(feature = "io_safety", since = "1.63.0")] impl AsHandle for crate::io::Stdout { #[inline] @@ -578,6 +585,13 @@ impl<'a> AsHandle for crate::io::StdoutLock<'a> { } } +impl AsHandle for crate::io::StdoutRaw { + #[inline] + fn as_handle(&self) -> BorrowedHandle<'_> { + unsafe { BorrowedHandle::borrow_raw(self.as_raw_handle()) } + } +} + #[stable(feature = "io_safety", since = "1.63.0")] impl AsHandle for crate::io::Stderr { #[inline] @@ -594,6 +608,13 @@ impl<'a> AsHandle for crate::io::StderrLock<'a> { } } +impl AsHandle for crate::io::StderrRaw { + #[inline] + fn as_handle(&self) -> BorrowedHandle<'_> { + unsafe { BorrowedHandle::borrow_raw(self.as_raw_handle()) } + } +} + #[stable(feature = "io_safety", since = "1.63.0")] impl AsHandle for crate::process::ChildStdin { #[inline] diff --git a/library/std/src/os/windows/io/raw.rs b/library/std/src/os/windows/io/raw.rs index a3ec7440338d2..7667fc4e10d11 100644 --- a/library/std/src/os/windows/io/raw.rs +++ b/library/std/src/os/windows/io/raw.rs @@ -140,6 +140,24 @@ impl<'a> AsRawHandle for io::StderrLock<'a> { } } +impl AsRawHandle for io::StdinRaw { + fn as_raw_handle(&self) -> RawHandle { + stdio_handle(unsafe { sys::c::GetStdHandle(sys::c::STD_INPUT_HANDLE) as RawHandle }) + } +} + +impl AsRawHandle for io::StdoutRaw { + fn as_raw_handle(&self) -> RawHandle { + stdio_handle(unsafe { sys::c::GetStdHandle(sys::c::STD_OUTPUT_HANDLE) as RawHandle }) + } +} + +impl AsRawHandle for io::StderrRaw { + fn as_raw_handle(&self) -> RawHandle { + stdio_handle(unsafe { sys::c::GetStdHandle(sys::c::STD_ERROR_HANDLE) as RawHandle }) + } +} + // Translate a handle returned from `GetStdHandle` into a handle to return to // the user. fn stdio_handle(raw: RawHandle) -> RawHandle {