Skip to content

Fix a soundness hole allowing multiple mutable borrows #124

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Nov 1, 2018
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
language: rust
matrix:
include:
- rust: 1.21.0
- rust: 1.24.1
- rust: stable
- os: osx
- rust: beta
Expand Down
24 changes: 12 additions & 12 deletions src/heap_lazy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,27 +8,27 @@
extern crate std;

use self::std::prelude::v1::*;
use self::std::cell::Cell;
use self::std::sync::Once;
pub use self::std::sync::ONCE_INIT;

pub struct Lazy<T: Sync>(*const T, Once);
pub struct Lazy<T: Sync>(Cell<*const T>, Once);

impl<T: Sync> Lazy<T> {
pub const INIT: Self = Lazy(0 as *const T, ONCE_INIT);
pub const INIT: Self = Lazy(Cell::new(0 as *const T), ONCE_INIT);

#[inline(always)]
pub fn get<F>(&'static mut self, f: F) -> &T
pub fn get<F>(&'static self, f: F) -> &T
where
F: FnOnce() -> T,
{
unsafe {
let r = &mut self.0;
self.1.call_once(|| {
*r = Box::into_raw(Box::new(f()));
});

&*self.0
}
self.1.call_once(|| {
self.0.set(Box::into_raw(Box::new(f())));
});

// `self.0` is guaranteed to have a value by this point
// The `Once` will catch and propegate panics
unsafe { &*self.0.get() }
}
}

Expand All @@ -38,6 +38,6 @@ unsafe impl<T: Sync> Sync for Lazy<T> {}
#[doc(hidden)]
macro_rules! __lazy_static_create {
($NAME:ident, $T:ty) => {
static mut $NAME: $crate::lazy::Lazy<$T> = $crate::lazy::Lazy::INIT;
static $NAME: $crate::lazy::Lazy<$T> = $crate::lazy::Lazy::INIT;
};
}
23 changes: 12 additions & 11 deletions src/inline_lazy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,27 +9,28 @@ extern crate core;
extern crate std;

use self::std::prelude::v1::*;
use self::std::cell::Cell;
use self::std::sync::Once;
pub use self::std::sync::ONCE_INIT;

pub struct Lazy<T: Sync>(Option<T>, Once);
pub struct Lazy<T: Sync>(Cell<Option<T>>, Once);

impl<T: Sync> Lazy<T> {
pub const INIT: Self = Lazy(None, ONCE_INIT);
pub const INIT: Self = Lazy(Cell::new(None), ONCE_INIT);

#[inline(always)]
pub fn get<F>(&'static mut self, f: F) -> &T
pub fn get<F>(&'static self, f: F) -> &T
where
F: FnOnce() -> T,
{
{
let r = &mut self.0;
self.1.call_once(|| {
*r = Some(f());
});
}
self.1.call_once(|| {
self.0.set(Some(f()));
});

// `self.0` is guaranteed to be `Some` by this point
// The `Once` will catch and propegate panics
unsafe {
match self.0 {
match *self.0.as_ptr() {
Some(ref x) => x,
None => unreachable_unchecked(),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth a debug_assert!(false) or so?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea 👍 It seems like a fair thing to be more defensive about

}
Expand All @@ -43,7 +44,7 @@ unsafe impl<T: Sync> Sync for Lazy<T> {}
#[doc(hidden)]
macro_rules! __lazy_static_create {
($NAME:ident, $T:ty) => {
static mut $NAME: $crate::lazy::Lazy<$T> = $crate::lazy::Lazy::INIT;
static $NAME: $crate::lazy::Lazy<$T> = $crate::lazy::Lazy::INIT;
};
}

Expand Down