Skip to content

Commit

Permalink
Auto merge of rust-lang#77154 - fusion-engineering-forks:lazy-stdio, …
Browse files Browse the repository at this point in the history
…r=dtolnay

Remove std::io::lazy::Lazy in favour of SyncOnceCell

The (internal) std::io::lazy::Lazy was used to lazily initialize the stdout and stdin buffers (and mutexes). It uses atexit() to register a destructor to flush the streams on exit, and mark the streams as 'closed'. Using the stream afterwards would result in a panic.

Stdout uses a LineWriter which contains a BufWriter that will flush the buffer on drop. This one is important to be executed during shutdown, to make sure no buffered output is lost. It also forbids access to stdout afterwards, since the buffer is already flushed and gone.

Stdin uses a BufReader, which does not implement Drop. It simply forgets any previously read data that was not read from the buffer yet. This means that in the case of stdin, the atexit() function's only effect is making stdin inaccessible to the program, such that later accesses result in a panic. This is uncessary, as it'd have been safe to access stdin during shutdown of the program.

---

This change removes the entire io::lazy module in favour of SyncOnceCell. SyncOnceCell's fast path is much faster (a single atomic operation) than locking a sys_common::Mutex on every access like Lazy did.

However, SyncOnceCell does not use atexit() to drop the contained object during shutdown.

As noted above, this is not a problem for stdin. It simply means stdin is now usable during shutdown.

The atexit() call for stdout is moved to the stdio module. Unlike the now-removed Lazy struct, SyncOnceCell does not have a 'gone and unusable' state that panics. Instead of adding this again, this simply replaces the buffer with one with zero capacity. This effectively flushes the old buffer *and* makes any writes afterwards pass through directly without touching a buffer, making print!() available during shutdown without panicking.

---

In addition, because the contents of the SyncOnceCell are no longer dropped, we can now use `&'static` instead of `Arc` in `Stdout` and `Stdin`. This also saves two levels of indirection in `stdin()` and `stdout()`, since Lazy effectively stored a `Box<Arc<T>>`, and SyncOnceCell stores the `T` directly.
  • Loading branch information
bors committed Sep 27, 2020
2 parents 62fe055 + 6b8b9c4 commit c9e5e6a
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 100 deletions.
63 changes: 0 additions & 63 deletions library/std/src/io/lazy.rs

This file was deleted.

1 change: 0 additions & 1 deletion library/std/src/io/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,6 @@ mod buffered;
mod cursor;
mod error;
mod impls;
mod lazy;
pub mod prelude;
mod stdio;
mod util;
Expand Down
76 changes: 40 additions & 36 deletions library/std/src/io/stdio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@ use crate::io::prelude::*;

use crate::cell::RefCell;
use crate::fmt;
use crate::io::lazy::Lazy;
use crate::io::{self, BufReader, Initializer, IoSlice, IoSliceMut, LineWriter};
use crate::sync::{Arc, Mutex, MutexGuard, Once};
use crate::lazy::SyncOnceCell;
use crate::sync::{Mutex, MutexGuard};
use crate::sys::stdio;
use crate::sys_common;
use crate::sys_common::remutex::{ReentrantMutex, ReentrantMutexGuard};
use crate::thread::LocalKey;

Expand Down Expand Up @@ -217,7 +218,7 @@ fn handle_ebadf<T>(r: io::Result<T>, default: T) -> io::Result<T> {
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
pub struct Stdin {
inner: Arc<Mutex<BufReader<StdinRaw>>>,
inner: &'static Mutex<BufReader<StdinRaw>>,
}

/// A locked reference to the `Stdin` handle.
Expand Down Expand Up @@ -292,15 +293,11 @@ pub struct StdinLock<'a> {
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
pub fn stdin() -> Stdin {
static INSTANCE: Lazy<Mutex<BufReader<StdinRaw>>> = Lazy::new();
return Stdin {
inner: unsafe { INSTANCE.get(stdin_init).expect("cannot access stdin during shutdown") },
};

fn stdin_init() -> Arc<Mutex<BufReader<StdinRaw>>> {
// This must not reentrantly access `INSTANCE`
let stdin = stdin_raw();
Arc::new(Mutex::new(BufReader::with_capacity(stdio::STDIN_BUF_SIZE, stdin)))
static INSTANCE: SyncOnceCell<Mutex<BufReader<StdinRaw>>> = SyncOnceCell::new();
Stdin {
inner: INSTANCE.get_or_init(|| {
Mutex::new(BufReader::with_capacity(stdio::STDIN_BUF_SIZE, stdin_raw()))
}),
}
}

Expand Down Expand Up @@ -476,7 +473,7 @@ 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<StdoutRaw>>>>,
inner: &'static ReentrantMutex<RefCell<LineWriter<StdoutRaw>>>,
}

/// A locked reference to the `Stdout` handle.
Expand Down Expand Up @@ -534,19 +531,27 @@ pub struct StdoutLock<'a> {
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
pub fn stdout() -> Stdout {
static INSTANCE: Lazy<ReentrantMutex<RefCell<LineWriter<StdoutRaw>>>> = Lazy::new();
return Stdout {
inner: unsafe { INSTANCE.get(stdout_init).expect("cannot access stdout during shutdown") },
};

fn stdout_init() -> Arc<ReentrantMutex<RefCell<LineWriter<StdoutRaw>>>> {
// This must not reentrantly access `INSTANCE`
let stdout = stdout_raw();
unsafe {
let ret = Arc::new(ReentrantMutex::new(RefCell::new(LineWriter::new(stdout))));
ret.init();
ret
}
static INSTANCE: SyncOnceCell<ReentrantMutex<RefCell<LineWriter<StdoutRaw>>>> =
SyncOnceCell::new();
Stdout {
inner: INSTANCE.get_or_init(|| unsafe {
let _ = sys_common::at_exit(|| {
if let Some(instance) = INSTANCE.get() {
// Flush the data and disable buffering during shutdown
// by replacing the line writer by one with zero
// buffering capacity.
// We use try_lock() instead of lock(), because someone
// might have leaked a StdoutLock, which would
// otherwise cause a deadlock here.
if let Some(lock) = instance.try_lock() {
*lock.borrow_mut() = LineWriter::with_capacity(0, stdout_raw());
}
}
});
let r = ReentrantMutex::new(RefCell::new(LineWriter::new(stdout_raw())));
r.init();
r
}),
}
}

Expand Down Expand Up @@ -741,16 +746,15 @@ pub fn stderr() -> Stderr {
//
// This has the added benefit of allowing `stderr` to be usable during
// process shutdown as well!
static INSTANCE: ReentrantMutex<RefCell<StderrRaw>> =
unsafe { ReentrantMutex::new(RefCell::new(stderr_raw())) };

// When accessing stderr we need one-time initialization of the reentrant
// mutex. Afterwards we can just always use the now-filled-in `INSTANCE` value.
static INIT: Once = Once::new();
INIT.call_once(|| unsafe {
INSTANCE.init();
});
Stderr { inner: &INSTANCE }
static INSTANCE: SyncOnceCell<ReentrantMutex<RefCell<StderrRaw>>> = SyncOnceCell::new();

Stderr {
inner: INSTANCE.get_or_init(|| unsafe {
let r = ReentrantMutex::new(RefCell::new(stderr_raw()));
r.init();
r
}),
}
}

impl Stderr {
Expand Down
19 changes: 19 additions & 0 deletions src/test/ui/stdout-during-shutdown.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// run-pass
// check-run-results
// ignore-emscripten

// Emscripten doesn't flush its own stdout buffers on exit, which would fail
// this test. So this test is disabled on this platform.
// See https://emscripten.org/docs/getting_started/FAQ.html#what-does-exiting-the-runtime-mean-why-don-t-atexit-s-run

#![feature(rustc_private)]

extern crate libc;

fn main() {
extern "C" fn bye() {
print!(", world!");
}
unsafe { libc::atexit(bye) };
print!("hello");
}
1 change: 1 addition & 0 deletions src/test/ui/stdout-during-shutdown.run.stdout
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
hello, world!

0 comments on commit c9e5e6a

Please sign in to comment.