-
Notifications
You must be signed in to change notification settings - Fork 1k
internal: no_std thread local
#6356
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
Open
Person-93
wants to merge
10
commits into
PyO3:main
Choose a base branch
from
Person-93:no_std_thread_local
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+209
−1
Open
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
8733d18
implement LocalKey using tss api
Person-93 86160a8
use platform thread_local instead of std::thread_local
Person-93 1dcdc5b
fix typo
Person-93 cdfb8e3
simplify cfg options
Person-93 fb4f7c4
fix local key new assign wrong state
Person-93 3adf89d
add test case
Person-93 9e814d3
track callers for panic
Person-93 ebaa82d
fix: race condition initializing local key
Person-93 be76bc4
try fixing the face condition again
Person-93 cf56f87
use OnceCell instead of UnsafeCell and loop
Person-93 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,194 @@ | ||
| use core::cell::{Cell, UnsafeCell}; | ||
| use core::ffi::c_void; | ||
| use core::fmt::{Debug, Display}; | ||
| use core::marker::PhantomData; | ||
| use core::ptr::{self, NonNull}; | ||
| use core::sync::atomic::{AtomicBool, AtomicPtr, AtomicU8, Ordering}; | ||
|
|
||
| use once_cell::sync::{Lazy, OnceCell}; | ||
| #[cfg(not(Py_LIMITED_API))] | ||
| use pyo3_ffi::Py_tss_NEEDS_INIT; | ||
|
|
||
| use crate::ffi::{ | ||
| PyThread_tss_alloc, PyThread_tss_create, PyThread_tss_delete, PyThread_tss_free, | ||
| PyThread_tss_get, PyThread_tss_set, | ||
| }; | ||
| use crate::platform::prelude::*; | ||
|
|
||
| pub struct LocalKey<T: 'static> { | ||
| #[cfg(Py_LIMITED_API)] | ||
| inner: OnceCell<NonNull<crate::ffi::Py_tss_t>>, | ||
|
|
||
| #[cfg(not(Py_LIMITED_API))] | ||
| inner: OnceCell<UnsafeCell<crate::ffi::Py_tss_t>>, | ||
|
|
||
| destroyed: AtomicBool, | ||
| init: fn() -> T, | ||
| } | ||
|
|
||
| // SAFETY: the unsafecell is only accessed by python tss functions which are thread safe | ||
| #[cfg(not(Py_LIMITED_API))] | ||
| unsafe impl<T: 'static> Sync for LocalKey<T> {} | ||
|
|
||
| impl<T: 'static> Debug for LocalKey<T> { | ||
| fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { | ||
| f.debug_struct("LocalKey").finish_non_exhaustive() | ||
| } | ||
| } | ||
|
|
||
| #[derive(Clone, Copy, Eq, PartialEq, Debug)] | ||
| pub struct AccessError; | ||
|
|
||
| impl Display for AccessError { | ||
| fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { | ||
| Display::fmt("already destroyed", f) | ||
| } | ||
| } | ||
|
|
||
| impl core::error::Error for AccessError {} | ||
|
|
||
| // This ensures the panicking code is outlined from `with` for `LocalKey`. | ||
| #[cfg_attr(not(panic = "immediate-abort"), inline(never))] | ||
| #[track_caller] | ||
| #[cold] | ||
| fn panic_access_error(err: AccessError) -> ! { | ||
| panic!("cannot access a Thread Local Storage value during or after destruction: {err:?}") | ||
| } | ||
|
|
||
| impl<T: 'static> LocalKey<T> { | ||
| pub const unsafe fn new(init: fn() -> T) -> LocalKey<T> { | ||
| LocalKey { | ||
| inner: OnceCell::new(), | ||
| destroyed: AtomicBool::new(false), | ||
| init, | ||
| } | ||
| } | ||
|
|
||
| #[track_caller] | ||
| #[inline] | ||
| pub fn with<F, R>(&'static self, f: F) -> R | ||
| where | ||
| F: FnOnce(&T) -> R, | ||
| { | ||
| match self.try_with(f) { | ||
| Ok(r) => r, | ||
| Err(err) => panic_access_error(err), | ||
| } | ||
| } | ||
|
|
||
| #[inline] | ||
| #[track_caller] | ||
| pub fn try_with<F, R>(&'static self, f: F) -> Result<R, AccessError> | ||
| where | ||
| F: FnOnce(&T) -> R, | ||
| { | ||
| if self.destroyed.load(Ordering::SeqCst) { | ||
| return Err(AccessError); | ||
| } | ||
| let val = self.get_val(); | ||
| Ok(f(val)) | ||
| } | ||
|
|
||
| #[track_caller] | ||
| fn get_val<'a>(&'static self) -> &'a T { | ||
| let inner = self.get_raw(); | ||
| // SAFETY: inner is a valid tss key | ||
| let val: *mut T = unsafe { PyThread_tss_get(inner.as_ptr()) }.cast(); | ||
| match NonNull::new(val) { | ||
| // SAFETY: no mut ref is ever created from this pointer | ||
| Some(val) => unsafe { val.as_ref() }, | ||
| None => { | ||
| let val = Box::new((self.init)()); | ||
| let val = Box::into_raw(val); | ||
| // SAFETY: inner is a valid tss key | ||
| let result = unsafe { PyThread_tss_set(inner.as_ptr(), val.cast()) }; | ||
| assert_eq!(result, 0, "failed to set thread specific value"); | ||
| // SAFETY: val was just allocated above | ||
| unsafe { NonNull::new_unchecked(val).as_ref() } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| fn get_raw(&self) -> NonNull<crate::ffi::Py_tss_t> { | ||
| cfg_select! { | ||
| Py_LIMITED_API => self.inner.get_or_init(initialize_tss), | ||
| _ => NonNull::new(self.inner.get_or_init(initialize_tss).get()).unwrap(), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| #[cfg(Py_LIMITED_API)] | ||
| fn initialize_tss() -> NonNull<crate::ffi::Py_tss_t> { | ||
| // SAFETY: no requirements | ||
| let tss = unsafe { PyThread_tss_alloc() }; | ||
| let tss = NonNull::new(tss).unwrap(); | ||
| // SAFETY: ptr obtained by calling PyThread_tss_alloc | ||
| let result = unsafe { PyThread_tss_create(tss.as_ptr()) }; | ||
| assert_eq!(result, 0, "failed to created thread specific storage"); | ||
| tss | ||
| } | ||
|
|
||
| #[cfg(not(Py_LIMITED_API))] | ||
| fn initialize_tss() -> UnsafeCell<crate::ffi::Py_tss_t> { | ||
| let mut tss = Py_tss_NEEDS_INIT; | ||
| // SAFETY: tss is initialized with Py_tss_NEEDS_INIT | ||
| let result = unsafe { PyThread_tss_create(&raw mut tss) }; | ||
| assert_eq!(result, 0, "failed to created thread specific storage"); | ||
| UnsafeCell::new(tss) | ||
| } | ||
|
|
||
| impl<T: 'static> Drop for LocalKey<T> { | ||
| fn drop(&mut self) { | ||
| self.destroyed.store(true, Ordering::SeqCst); | ||
| let inner = self.get_raw(); | ||
| cfg_select! { | ||
| Py_LIMITED_API => { | ||
| // SAFETY: inner is returned by PyThread_tss_alloc and is not used after this call | ||
| unsafe { PyThread_tss_free(inner.as_ptr()) }; | ||
| }, | ||
| _ => { | ||
| // SAFETY: inner is not used again after this call | ||
| unsafe { PyThread_tss_delete(inner.as_ptr()) }; | ||
| }, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| #[macro_export] | ||
| #[doc(hidden)] | ||
| macro_rules! thread_local { | ||
| ($($(#[$attr:meta])* $vis:vis static $name:ident : $ty:ty = $(const)? $init:expr;)+) => { | ||
| $( | ||
| $(#[$attr])* | ||
| #[allow(unused_braces)] | ||
| // SAFETY: correctly initializes a LocalKey | ||
| $vis static $name: $crate::platform::thread::LocalKey<$ty> = unsafe { | ||
| $crate::platform::thread::LocalKey::new({ | ||
| fn init() -> $ty { | ||
| $init | ||
| } | ||
| init | ||
| }) | ||
| }; | ||
| )+ | ||
| }; | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::LocalKey; | ||
|
|
||
| use core::cell::Cell; | ||
|
|
||
| #[test] | ||
| fn create_and_set_thread_local() { | ||
| crate::thread_local! { | ||
| static NUM: Cell<u32> = Cell::new(42); | ||
| }; | ||
|
|
||
| assert_eq!(NUM.with(|val| val.get()), 42); | ||
|
|
||
| NUM.with(|val| val.set(18)); | ||
| assert_eq!(NUM.with(|val| val.get()), 18); | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess in both of these cases all the values stored for each thread are leaked?
Is there a way that we can trigger destruction of thread-local values on native thread exit? It might not matter for the types which we currently store in thread locals, maybe not ever. So this might just be a question for sake of curiosity which we could document here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would require calling platform-specific APIs.