From b958edee03906edd70450d3ea696cd611dd7e138 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 5 Feb 2020 16:35:46 +0100 Subject: [PATCH] Change get's semantics to never block Turns our, real world code can has **really** subtle bugs with non-blocking semantics --- src/lib.rs | 16 ++++++---------- tests/test.rs | 32 +++++++++++++------------------- 2 files changed, 19 insertions(+), 29 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index d1a227e..15fc0d2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -659,15 +659,9 @@ pub mod sync { /// Gets the reference to the underlying value. /// - /// Returns `None` if the cell is empty, or being initialized. This - /// method never blocks. + /// Returns `None` if the cell is empty, or being initialized. pub fn get(&self) -> Option<&T> { - if self.0.is_initialized() { - // Safe b/c value is initialized. - Some(unsafe { self.get_unchecked() }) - } else { - None - } + self.get_or_try_init(|| Err(())).ok() } /// Gets the mutable reference to the underlying value. @@ -787,9 +781,11 @@ pub mod sync { F: FnOnce() -> Result, { // Fast path check - if let Some(value) = self.get() { - return Ok(value); + if self.0.is_initialized() { + // Safe b/c value is initialized. + return Ok(unsafe { self.get_unchecked() }); } + self.0.initialize(f)?; // Safe b/c value is initialized. diff --git a/tests/test.rs b/tests/test.rs index 7cd085d..b2dd8c7 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -553,25 +553,19 @@ mod sync { } #[test] - #[cfg_attr(miri, ignore)] // miri doesn't support Barrier - fn get_does_not_block() { - use std::sync::Barrier; + #[cfg_attr(miri, ignore)] + fn get_blocks() { + static CELL: OnceCell = OnceCell::new(); - let cell = OnceCell::new(); - let barrier = Barrier::new(2); - scope(|scope| { - scope.spawn(|_| { - cell.get_or_init(|| { - barrier.wait(); - barrier.wait(); - "hello".to_string() - }); - }); - barrier.wait(); - assert_eq!(cell.get(), None); - barrier.wait(); - }) - .unwrap(); - assert_eq!(cell.get(), Some(&"hello".to_string())); + let mut thread = None; + + let res = *CELL.get_or_init(|| { + thread = Some(std::thread::spawn(|| assert_eq!(CELL.get(), Some(&92)))); + std::thread::sleep(std::time::Duration::from_millis(100)); + 92 + }); + assert_eq!(res, 92); + + thread.unwrap().join().unwrap(); } }