Skip to content

Commit

Permalink
Change get's semantics to never block
Browse files Browse the repository at this point in the history
Turns our, real world code can has **really** subtle bugs with
non-blocking semantics
  • Loading branch information
matklad committed Feb 5, 2020
1 parent 5978d18 commit b958ede
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 29 deletions.
16 changes: 6 additions & 10 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -787,9 +781,11 @@ pub mod sync {
F: FnOnce() -> Result<T, E>,
{
// 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.
Expand Down
32 changes: 13 additions & 19 deletions tests/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<u32> = 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();
}
}

0 comments on commit b958ede

Please sign in to comment.