Skip to content

std: Fix unsoundness of std::thread::ScopedKey #25952

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 1 commit into from
Jun 16, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/libstd/thread/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@ pub use self::local::{LocalKey, LocalKeyState};
pub use self::scoped_tls::ScopedKey;

#[doc(hidden)] pub use self::local::__KeyInner as __LocalKeyInner;
#[doc(hidden)] pub use self::scoped_tls::__KeyInner as __ScopedKeyInner;

////////////////////////////////////////////////////////////////////////////////
// Builder
Expand Down
79 changes: 47 additions & 32 deletions src/libstd/thread/scoped_tls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@

use prelude::v1::*;

#[doc(hidden)]
pub use self::imp::KeyInner as __KeyInner;

/// Type representing a thread local storage key corresponding to a reference
/// to the type parameter `T`.
///
Expand All @@ -53,7 +56,7 @@ use prelude::v1::*;
#[unstable(feature = "scoped_tls",
reason = "scoped TLS has yet to have wide enough use to fully consider \
stabilizing its interface")]
pub struct ScopedKey<T> { inner: imp::KeyInner<T> }
pub struct ScopedKey<T> { inner: fn() -> &'static imp::KeyInner<T> }

/// Declare a new scoped thread local storage key.
///
Expand All @@ -64,51 +67,60 @@ pub struct ScopedKey<T> { inner: imp::KeyInner<T> }
/// information.
#[macro_export]
#[allow_internal_unstable]
#[cfg(not(no_elf_tls))]
macro_rules! scoped_thread_local {
(static $name:ident: $t:ty) => (
#[cfg_attr(not(any(windows,
target_os = "android",
target_os = "ios",
target_os = "openbsd",
target_arch = "aarch64")),
thread_local)]
static $name: ::std::thread::ScopedKey<$t> =
::std::thread::ScopedKey::new();
__scoped_thread_local_inner!($t);
);
(pub static $name:ident: $t:ty) => (
#[cfg_attr(not(any(windows,
target_os = "android",
target_os = "ios",
target_os = "openbsd",
target_arch = "aarch64")),
thread_local)]
pub static $name: ::std::thread::ScopedKey<$t> =
::std::thread::ScopedKey::new();
__scoped_thread_local_inner!($t);
);
}

#[doc(hidden)]
#[unstable(feature = "thread_local_internals",
reason = "should not be necessary")]
#[macro_export]
#[allow_internal_unstable]
#[cfg(no_elf_tls)]
macro_rules! scoped_thread_local {
(static $name:ident: $t:ty) => (
static $name: ::std::thread::ScopedKey<$t> =
::std::thread::ScopedKey::new();
);
(pub static $name:ident: $t:ty) => (
pub static $name: ::std::thread::ScopedKey<$t> =
::std::thread::ScopedKey::new();
);
macro_rules! __scoped_thread_local_inner {
($t:ty) => {{
static _KEY: ::std::thread::__ScopedKeyInner<$t> =
::std::thread::__ScopedKeyInner::new();
fn _getit() -> &'static ::std::thread::__ScopedKeyInner<$t> { &_KEY }
::std::thread::ScopedKey::new(_getit)
}}
}

#[doc(hidden)]
#[unstable(feature = "thread_local_internals",
reason = "should not be necessary")]
#[macro_export]
#[allow_internal_unstable]
#[cfg(not(no_elf_tls))]
macro_rules! __scoped_thread_local_inner {
($t:ty) => {{
#[cfg_attr(not(any(windows,
target_os = "android",
target_os = "ios",
target_os = "openbsd",
target_arch = "aarch64")),
thread_local)]
static _KEY: ::std::thread::__ScopedKeyInner<$t> =
::std::thread::__ScopedKeyInner::new();
fn _getit() -> &'static ::std::thread::__ScopedKeyInner<$t> { &_KEY }
::std::thread::ScopedKey::new(_getit)
}}
}

#[unstable(feature = "scoped_tls",
reason = "scoped TLS has yet to have wide enough use to fully consider \
stabilizing its interface")]
impl<T> ScopedKey<T> {
#[doc(hidden)]
pub const fn new() -> ScopedKey<T> {
ScopedKey { inner: imp::KeyInner::new() }
pub const fn new(inner: fn() -> &'static imp::KeyInner<T>) -> ScopedKey<T> {
ScopedKey { inner: inner }
}

/// Inserts a value into this scoped thread local storage slot for a
Expand Down Expand Up @@ -153,13 +165,14 @@ impl<T> ScopedKey<T> {
}
}

let inner = (self.inner)();
let prev = unsafe {
let prev = self.inner.get();
self.inner.set(t as *const T as *mut T);
let prev = inner.get();
inner.set(t as *const T as *mut T);
prev
};

let _reset = Reset { key: &self.inner, val: prev };
let _reset = Reset { key: inner, val: prev };
cb()
}

Expand All @@ -186,7 +199,7 @@ impl<T> ScopedKey<T> {
F: FnOnce(&T) -> R
{
unsafe {
let ptr = self.inner.get();
let ptr = (self.inner)().get();
assert!(!ptr.is_null(), "cannot access a scoped thread local \
variable without calling `set` first");
cb(&*ptr)
Expand All @@ -195,7 +208,7 @@ impl<T> ScopedKey<T> {

/// Test whether this TLS key has been `set` for the current thread.
pub fn is_set(&'static self) -> bool {
unsafe { !self.inner.get().is_null() }
unsafe { !(self.inner)().get().is_null() }
}
}

Expand All @@ -205,6 +218,7 @@ impl<T> ScopedKey<T> {
target_os = "openbsd",
target_arch = "aarch64",
no_elf_tls)))]
#[doc(hidden)]
mod imp {
use std::cell::Cell;

Expand All @@ -227,6 +241,7 @@ mod imp {
target_os = "openbsd",
target_arch = "aarch64",
no_elf_tls))]
#[doc(hidden)]
mod imp {
use prelude::v1::*;

Expand Down