Skip to content

Commit

Permalink
fix: fix soundness hole in borrow_with
Browse files Browse the repository at this point in the history
closes #66
  • Loading branch information
matklad authored and indiv0 committed Nov 24, 2017
1 parent bffa402 commit d1f46be
Showing 1 changed file with 52 additions and 10 deletions.
62 changes: 52 additions & 10 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,25 +100,37 @@ impl<T> LazyCell<T> {
///
/// If the cell has not yet been filled, the cell is first filled using the
/// function provided.
///
/// # Panics
///
/// Panics if the cell becomes filled as a side effect of `f`.
pub fn borrow_with<F: FnOnce() -> T>(&self, f: F) -> &T {
let mut slot = unsafe { &mut *self.inner.get() };
if !slot.is_some() {
*slot = Some(f());
if let Some(value) = self.borrow() {
return value;
}

slot.as_ref().unwrap()
let value = f();
if let Err(_) = self.fill(value) {
panic!("borrow_with: cell was filled by closure")
}
self.borrow().unwrap()
}

/// Same as `borrow_with`, but allows the initializing function to fail.
///
/// # Panics
///
/// Panics if the cell becomes filled as a side effect of `f`.
pub fn try_borrow_with<E, F>(&self, f: F) -> Result<&T, E>
where F: FnOnce() -> Result<T, E>
{
let mut slot = unsafe { &mut *self.inner.get() };
if !slot.is_some() {
*slot = Some(f()?);
if let Some(value) = self.borrow() {
return Ok(value);
}

Ok(slot.as_ref().unwrap())
let value = f()?;
if let Err(_) = self.fill(value) {
panic!("try_borrow_with: cell was filled by closure")
}
Ok(self.borrow().unwrap())
}

/// Consumes this `LazyCell`, returning the underlying value.
Expand Down Expand Up @@ -296,6 +308,22 @@ mod tests {
assert_eq!(&1, value);
}

#[test]
#[should_panic]
fn test_borrow_with_sound_with_reentrancy() {
// Kudos to dbaupp for discovering this issue
// https://www.reddit.com/r/rust/comments/5vs9rt/lazycell_a_rust_library_providing_a_lazilyfilled/de527xm/
let lazycell: LazyCell<Box<i32>> = LazyCell::new();

let mut reference: Option<&i32> = None;

lazycell.borrow_with(|| {
let _ = lazycell.fill(Box::new(1));
reference = lazycell.borrow().map(|r| &**r);
Box::new(2)
});
}

#[test]
fn test_try_borrow_with_ok() {
let lazycell = LazyCell::new();
Expand All @@ -318,6 +346,20 @@ mod tests {
assert_eq!(result, Ok(&1));
}

#[test]
#[should_panic]
fn test_try_borrow_with_sound_with_reentrancy() {
let lazycell: LazyCell<Box<i32>> = LazyCell::new();

let mut reference: Option<&i32> = None;

let _ = lazycell.try_borrow_with::<(), _>(|| {
let _ = lazycell.fill(Box::new(1));
reference = lazycell.borrow().map(|r| &**r);
Ok(Box::new(2))
});
}

#[test]
fn test_into_inner() {
let lazycell = LazyCell::new();
Expand Down

0 comments on commit d1f46be

Please sign in to comment.