mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #122494 - joboet:simplify_key_tls, r=m-ou-se

Browse Source 
Simplify key-based thread locals

This PR simplifies key-based thread-locals by:
* unifying the macro expansion of `const` and non-`const` initializers
* reducing the amount of code in the expansion
* simply reallocating on recursive initialization instead of going through `LazyKeyInner`
* replacing `catch_unwind` with the shared `abort_on_dtor_unwind`

It does not change the initialization behaviour described in #110897.
This commit is contained in:
bors 2024-05-24 15:34:07 +00:00
commit 9e297bf54d
4 changed files with 63 additions and 189 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
library/std/src/sys/mod.rs
View File

@ -9,8 +9,6 @@
pub mod os_str;
pub mod path;
pub mod sync;
#[allow(dead_code)]
#[allow(unused_imports)]
pub mod thread_local;
// FIXME(117276): remove this, move feature implementations into individual

1
library/std/src/sys/thread_local/fast_local/lazy.rs
View File

@ -1,6 +1,5 @@
use crate::cell::UnsafeCell;
use crate::hint::unreachable_unchecked;
use crate::mem::forget;
use crate::ptr;
use crate::sys::thread_local::abort_on_dtor_unwind;
use crate::sys::thread_local_dtor::register_dtor;

87
library/std/src/sys/thread_local/mod.rs
View File

@ -1,4 +1,5 @@
#![unstable(feature = "thread_local_internals", reason = "should not be necessary", issue = "none")]
#![cfg_attr(test, allow(unused))]
// There are three thread-local implementations: "static", "fast", "OS".
// The "OS" thread local key type is accessed via platform-specific API calls and is slow, while the
@ -24,94 +25,12 @@
}
}
// Not used by the fast-local TLS anymore.
// FIXME(#110897): remove this.
#[allow(unused)]
mod lazy {
use crate::cell::UnsafeCell;
use crate::hint;
use crate::mem;
pub struct LazyKeyInner<T> {
inner: UnsafeCell<Option<T>>,
}
impl<T> LazyKeyInner<T> {
pub const fn new() -> LazyKeyInner<T> {
LazyKeyInner { inner: UnsafeCell::new(None) }
}
pub unsafe fn get(&self) -> Option<&'static T> {
// SAFETY: The caller must ensure no reference is ever handed out to
// the inner cell nor mutable reference to the Option<T> inside said
// cell. This make it safe to hand a reference, though the lifetime
// of 'static is itself unsafe, making the get method unsafe.
unsafe { (*self.inner.get()).as_ref() }
}
/// The caller must ensure that no reference is active: this method
/// needs unique access.
pub unsafe fn initialize<F: FnOnce() -> T>(&self, init: F) -> &'static T {
// Execute the initialization up front, *then* move it into our slot,
// just in case initialization fails.
let value = init();
let ptr = self.inner.get();
// SAFETY:
//
// note that this can in theory just be `*ptr = Some(value)`, but due to
// the compiler will currently codegen that pattern with something like:
//
// ptr::drop_in_place(ptr)
// ptr::write(ptr, Some(value))
//
// Due to this pattern it's possible for the destructor of the value in
// `ptr` (e.g., if this is being recursively initialized) to re-access
// TLS, in which case there will be a `&` and `&mut` pointer to the same
// value (an aliasing violation). To avoid setting the "I'm running a
// destructor" flag we just use `mem::replace` which should sequence the
// operations a little differently and make this safe to call.
//
// The precondition also ensures that we are the only one accessing
// `self` at the moment so replacing is fine.
unsafe {
let _ = mem::replace(&mut *ptr, Some(value));
}
// SAFETY: With the call to `mem::replace` it is guaranteed there is
// a `Some` behind `ptr`, not a `None` so `unreachable_unchecked`
// will never be reached.
unsafe {
// After storing `Some` we want to get a reference to the contents of
// what we just stored. While we could use `unwrap` here and it should
// always work it empirically doesn't seem to always get optimized away,
// which means that using something like `try_with` can pull in
// panicking code and cause a large size bloat.
match *ptr {
Some(ref x) => x,
None => hint::unreachable_unchecked(),
}
}
}
/// Watch out: unsynchronized internal mutability!
///
/// # Safety
/// Causes UB if any reference to the value is used after this.
#[allow(unused)]
pub(crate) unsafe fn take(&self) -> Option<T> {
let mutable: *mut _ = UnsafeCell::get(&self.inner);
// SAFETY: That's the caller's problem.
unsafe { mutable.replace(None) }
}
}
}
/// Run a callback in a scenario which must not unwind (such as a `extern "C"
/// fn` declared in a user crate). If the callback unwinds anyway, then
/// `rtabort` with a message about thread local panicking on drop.
#[inline]
pub fn abort_on_dtor_unwind(f: impl FnOnce()) {
#[allow(dead_code)]
fn abort_on_dtor_unwind(f: impl FnOnce()) {
// Using a guard like this is lower cost.
let guard = DtorUnwindGuard;
f();

154
library/std/src/sys/thread_local/os_local.rs
View File

@ -1,7 +1,8 @@
use super::lazy::LazyKeyInner;
use super::abort_on_dtor_unwind;
use crate::cell::Cell;
use crate::sys_common::thread_local_key::StaticKey as OsStaticKey;
use crate::{fmt, marker, panic, ptr};
use crate::marker::PhantomData;
use crate::ptr;
use crate::sys_common::thread_local_key::StaticKey as OsKey;
#[doc(hidden)]
#[allow_internal_unstable(thread_local_internals)]
@ -10,38 +11,9 @@
#[rustc_macro_transparency = "semitransparent"]
pub macro thread_local_inner {
// used to generate the `LocalKey` value for const-initialized thread locals
(@key $t:ty, const $init:expr) => {{
#[inline]
#[deny(unsafe_op_in_unsafe_fn)]
unsafe fn __getit(
_init: $crate::option::Option<&mut $crate::option::Option<$t>>,
) -> $crate::option::Option<&'static $t> {
const INIT_EXPR: $t = $init;
// On platforms without `#[thread_local]` we fall back to the
// same implementation as below for os thread locals.
#[inline]
const fn __init() -> $t { INIT_EXPR }
static __KEY: $crate::thread::local_impl::Key<$t> =
$crate::thread::local_impl::Key::new();
unsafe {
__KEY.get(move || {
if let $crate::option::Option::Some(init) = _init {
if let $crate::option::Option::Some(value) = init.take() {
return value;
} else if $crate::cfg!(debug_assertions) {
$crate::unreachable!("missing initial value");
}
}
__init()
})
}
}
unsafe {
$crate::thread::LocalKey::new(__getit)
}
}},
(@key $t:ty, const $init:expr) => {
$crate::thread::local_impl::thread_local_inner!(@key $t, { const INIT_EXPR: $t = $init; INIT_EXPR })
},
// used to generate the `LocalKey` value for `thread_local!`
(@key $t:ty, $init:expr) => {
@ -55,20 +27,11 @@ fn __init() -> $t { $init }
unsafe fn __getit(
init: $crate::option::Option<&mut $crate::option::Option<$t>>,
) -> $crate::option::Option<&'static $t> {
static __KEY: $crate::thread::local_impl::Key<$t> =
$crate::thread::local_impl::Key::new();
use $crate::thread::local_impl::Key;
static __KEY: Key<$t> = Key::new();
unsafe {
__KEY.get(move || {
if let $crate::option::Option::Some(init) = init {
if let $crate::option::Option::Some(value) = init.take() {
return value;
} else if $crate::cfg!(debug_assertions) {
$crate::unreachable!("missing default value");
}
}
__init()
})
__KEY.get(init, __init)
}
}
@ -85,78 +48,78 @@ unsafe fn __getit(
/// Use a regular global static to store this key; the state provided will then be
/// thread-local.
#[allow(missing_debug_implementations)]
pub struct Key<T> {
// OS-TLS key that we'll use to key off.
os: OsStaticKey,
marker: marker::PhantomData<Cell<T>>,
}
impl<T> fmt::Debug for Key<T> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("Key").finish_non_exhaustive()
}
os: OsKey,
marker: PhantomData<Cell<T>>,
}
unsafe impl<T> Sync for Key<T> {}
struct Value<T: 'static> {
inner: LazyKeyInner<T>,
value: T,
key: &'static Key<T>,
}
impl<T: 'static> Key<T> {
#[rustc_const_unstable(feature = "thread_local_internals", issue = "none")]
pub const fn new() -> Key<T> {
Key { os: OsStaticKey::new(Some(destroy_value::<T>)), marker: marker::PhantomData }
Key { os: OsKey::new(Some(destroy_value::<T>)), marker: PhantomData }
}
/// It is a requirement for the caller to ensure that no mutable
/// reference is active when this method is called.
pub unsafe fn get(&'static self, init: impl FnOnce() -> T) -> Option<&'static T> {
// SAFETY: See the documentation for this method.
/// Get the value associated with this key, initializating it if necessary.
///
/// # Safety
/// * the returned reference must not be used after recursive initialization
/// or thread destruction occurs.
pub unsafe fn get(
&'static self,
i: Option<&mut Option<T>>,
f: impl FnOnce() -> T,
) -> Option<&'static T> {
// SAFETY: (FIXME: get should actually be safe)
let ptr = unsafe { self.os.get() as *mut Value<T> };
if ptr.addr() > 1 {
// SAFETY: the check ensured the pointer is safe (its destructor
// is not running) + it is coming from a trusted source (self).
if let Some(ref value) = unsafe { (*ptr).inner.get() } {
return Some(value);
}
}
unsafe { Some(&(*ptr).value) }
} else {
// SAFETY: At this point we are sure we have no value and so
// initializing (or trying to) is safe.
unsafe { self.try_initialize(init) }
unsafe { self.try_initialize(ptr, i, f) }
}
}
// `try_initialize` is only called once per os thread local variable,
// except in corner cases where thread_local dtors reference other
// thread_local's, or it is being recursively initialized.
unsafe fn try_initialize(&'static self, init: impl FnOnce() -> T) -> Option<&'static T> {
// SAFETY: No mutable references are ever handed out meaning getting
// the value is ok.
let ptr = unsafe { self.os.get() as *mut Value<T> };
unsafe fn try_initialize(
&'static self,
ptr: *mut Value<T>,
i: Option<&mut Option<T>>,
f: impl FnOnce() -> T,
) -> Option<&'static T> {
if ptr.addr() == 1 {
// destructor is running
return None;
}
let ptr = if ptr.is_null() {
// If the lookup returned null, we haven't initialized our own
// local copy, so do that now.
let ptr = Box::into_raw(Box::new(Value { inner: LazyKeyInner::new(), key: self }));
// SAFETY: At this point we are sure there is no value inside
// ptr so setting it will not affect anyone else.
let value = i.and_then(Option::take).unwrap_or_else(f);
let ptr = Box::into_raw(Box::new(Value { value, key: self }));
// SAFETY: (FIXME: get should actually be safe)
let old = unsafe { self.os.get() as *mut Value<T> };
// SAFETY: `ptr` is a correct pointer that can be destroyed by the key destructor.
unsafe {
self.os.set(ptr as *mut u8);
}
ptr
} else {
// recursive initialization
ptr
};
if !old.is_null() {
// If the variable was recursively initialized, drop the old value.
// SAFETY: We cannot be inside a `LocalKey::with` scope, as the
// initializer has already returned and the next scope only starts
// after we return the pointer. Therefore, there can be no references
// to the old value.
drop(unsafe { Box::from_raw(old) });
}
// SAFETY: ptr has been ensured as non-NUL just above an so can be
// dereferenced safely.
unsafe { Some((*ptr).inner.initialize(init)) }
// SAFETY: We just created this value above.
unsafe { Some(&(*ptr).value) }
}
}
@ -170,16 +133,11 @@ unsafe fn try_initialize(&'static self, init: impl FnOnce() -> T) -> Option<&'st
//
// Note that to prevent an infinite loop we reset it back to null right
// before we return from the destructor ourselves.
//
// Wrap the call in a catch to ensure unwinding is caught in the event
// a panic takes place in a destructor.
if let Err(_) = panic::catch_unwind(|| unsafe {
let ptr = Box::from_raw(ptr as *mut Value<T>);
abort_on_dtor_unwind(|| {
let ptr = unsafe { Box::from_raw(ptr as *mut Value<T>) };
let key = ptr.key;
key.os.set(ptr::without_provenance_mut(1));
unsafe { key.os.set(ptr::without_provenance_mut(1)) };
drop(ptr);
key.os.set(ptr::null_mut());
}) {
rtabort!("thread local panicked on drop");
}
unsafe { key.os.set(ptr::null_mut()) };
});
}