about summary refs log tree commit diff
diff options
context:
space:
mode:
authorMara Bos <m-ou.se@m-ou.se>2020-09-24 17:45:50 +0200
committerMara Bos <m-ou.se@m-ou.se>2020-09-24 18:18:48 +0200
commitbab15f773afd5724023d9a065b5af276e2468ff5 (patch)
tree16a5dc00cdd0272d64358e850d0857ebf2c9f2ce
parent4eff9b0b29a8898c839d46f3c66526710afed68a (diff)
downloadrust-bab15f773afd5724023d9a065b5af276e2468ff5.tar.gz
rust-bab15f773afd5724023d9a065b5af276e2468ff5.zip
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.
-rw-r--r--library/std/src/io/lazy.rs63
-rw-r--r--library/std/src/io/mod.rs1
-rw-r--r--library/std/src/io/stdio.rs74
3 files changed, 40 insertions, 98 deletions
diff --git a/library/std/src/io/lazy.rs b/library/std/src/io/lazy.rs
deleted file mode 100644
index 1968d498bbe..00000000000
--- a/library/std/src/io/lazy.rs
+++ /dev/null
@@ -1,63 +0,0 @@
-use crate::cell::Cell;
-use crate::ptr;
-use crate::sync::Arc;
-use crate::sys_common;
-use crate::sys_common::mutex::Mutex;
-
-pub struct Lazy<T> {
-    // We never call `lock.init()`, so it is UB to attempt to acquire this mutex reentrantly!
-    lock: Mutex,
-    ptr: Cell<*mut Arc<T>>,
-}
-
-#[inline]
-const fn done<T>() -> *mut Arc<T> {
-    1_usize as *mut _
-}
-
-unsafe impl<T> Sync for Lazy<T> {}
-
-impl<T> Lazy<T> {
-    pub const fn new() -> Lazy<T> {
-        Lazy { lock: Mutex::new(), ptr: Cell::new(ptr::null_mut()) }
-    }
-}
-
-impl<T: Send + Sync + 'static> Lazy<T> {
-    /// Safety: `init` must not call `get` on the variable that is being
-    /// initialized.
-    pub unsafe fn get(&'static self, init: fn() -> Arc<T>) -> Option<Arc<T>> {
-        let _guard = self.lock.lock();
-        let ptr = self.ptr.get();
-        if ptr.is_null() {
-            Some(self.init(init))
-        } else if ptr == done() {
-            None
-        } else {
-            Some((*ptr).clone())
-        }
-    }
-
-    // Must only be called with `lock` held
-    unsafe fn init(&'static self, init: fn() -> Arc<T>) -> Arc<T> {
-        // If we successfully register an at exit handler, then we cache the
-        // `Arc` allocation in our own internal box (it will get deallocated by
-        // the at exit handler). Otherwise we just return the freshly allocated
-        // `Arc`.
-        let registered = sys_common::at_exit(move || {
-            let ptr = {
-                let _guard = self.lock.lock();
-                self.ptr.replace(done())
-            };
-            drop(Box::from_raw(ptr))
-        });
-        // This could reentrantly call `init` again, which is a problem
-        // because our `lock` allows reentrancy!
-        // That's why `get` is unsafe and requires the caller to ensure no reentrancy happens.
-        let ret = init();
-        if registered.is_ok() {
-            self.ptr.set(Box::into_raw(Box::new(ret.clone())));
-        }
-        ret
-    }
-}
diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs
index adea8a804e3..d9d03807819 100644
--- a/library/std/src/io/mod.rs
+++ b/library/std/src/io/mod.rs
@@ -285,7 +285,6 @@ mod buffered;
 mod cursor;
 mod error;
 mod impls;
-mod lazy;
 pub mod prelude;
 mod stdio;
 mod util;
diff --git a/library/std/src/io/stdio.rs b/library/std/src/io/stdio.rs
index 9974b65f1e1..e0e5ccd0625 100644
--- a/library/std/src/io/stdio.rs
+++ b/library/std/src/io/stdio.rs
@@ -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::{Arc, Mutex, MutexGuard};
 use crate::sys::stdio;
+use crate::sys_common;
 use crate::sys_common::remutex::{ReentrantMutex, ReentrantMutexGuard};
 use crate::thread::LocalKey;
 
@@ -292,15 +293,13 @@ 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<Arc<Mutex<BufReader<StdinRaw>>>> = SyncOnceCell::new();
+    Stdin {
+        inner: INSTANCE
+            .get_or_init(|| {
+                Arc::new(Mutex::new(BufReader::with_capacity(stdio::STDIN_BUF_SIZE, stdin_raw())))
+            })
+            .clone(),
     }
 }
 
@@ -534,19 +533,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<Arc<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());
+                        }
+                    }
+                });
+                Arc::new(ReentrantMutex::new(RefCell::new(LineWriter::new(stdout_raw()))))
+            })
+            .clone(),
     }
 }
 
@@ -714,16 +721,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 {