From 0747f2898e83df7e601189c0f31762e84328becb Mon Sep 17 00:00:00 2001 From: GnomedDev Date: Wed, 16 Oct 2024 21:10:24 +0100 Subject: [PATCH] Remove the Arc rt::init allocation for thread info --- library/std/src/lib.rs | 1 + library/std/src/rt.rs | 2 +- library/std/src/thread/mod.rs | 170 ++++++++++++++++++-------- tests/debuginfo/thread.rs | 8 +- tests/rustdoc/demo-allocator-54478.rs | 1 + 5 files changed, 124 insertions(+), 58 deletions(-) diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index 3ab652383689c..ef1bc7dbc1883 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -364,6 +364,7 @@ #![feature(str_internals)] #![feature(strict_provenance)] #![feature(strict_provenance_atomic_ptr)] +#![feature(sync_unsafe_cell)] #![feature(ub_checks)] // tidy-alphabetical-end // diff --git a/library/std/src/rt.rs b/library/std/src/rt.rs index 80e7c3c026bd7..b2492238bd37b 100644 --- a/library/std/src/rt.rs +++ b/library/std/src/rt.rs @@ -110,7 +110,7 @@ unsafe fn init(argc: isize, argv: *const *const u8, sigpipe: u8) { // handle does not match the current ID, we should attempt to use the // current thread ID here instead of unconditionally creating a new // one. Also see #130210. - let thread = Thread::new_main(thread::current_id()); + let thread = unsafe { Thread::new_main(thread::current_id()) }; if let Err(_thread) = thread::set_current(thread) { // `thread::current` will create a new handle if none has been set yet. // Thus, if someone uses it before main, this call will fail. That's a diff --git a/library/std/src/thread/mod.rs b/library/std/src/thread/mod.rs index 3975388850998..70aa3170c6ebd 100644 --- a/library/std/src/thread/mod.rs +++ b/library/std/src/thread/mod.rs @@ -158,9 +158,12 @@ #[cfg(all(test, not(any(target_os = "emscripten", target_os = "wasi"))))] mod tests; +use core::cell::SyncUnsafeCell; +use core::ffi::CStr; +use core::mem::MaybeUninit; + use crate::any::Any; use crate::cell::UnsafeCell; -use crate::ffi::CStr; use crate::marker::PhantomData; use crate::mem::{self, ManuallyDrop, forget}; use crate::num::NonZero; @@ -1125,7 +1128,7 @@ pub fn park_timeout(dur: Duration) { let guard = PanicGuard; // SAFETY: park_timeout is called on the parker owned by this thread. unsafe { - current().inner.as_ref().parker().park_timeout(dur); + current().0.parker().park_timeout(dur); } // No panic occurred, do not abort. forget(guard); @@ -1232,30 +1235,31 @@ impl ThreadId { // Thread //////////////////////////////////////////////////////////////////////////////// -/// The internal representation of a `Thread`'s name. -enum ThreadName { - Main, - Other(ThreadNameString), - Unnamed, -} - // This module ensures private fields are kept private, which is necessary to enforce the safety requirements. mod thread_name_string { use core::str; - use super::ThreadName; use crate::ffi::{CStr, CString}; /// Like a `String` it's guaranteed UTF-8 and like a `CString` it's null terminated. pub(crate) struct ThreadNameString { inner: CString, } + + impl ThreadNameString { + pub fn as_str(&self) -> &str { + // SAFETY: `self.inner` is only initialised via `String`, which upholds the validity invariant of `str`. + unsafe { str::from_utf8_unchecked(self.inner.to_bytes()) } + } + } + impl core::ops::Deref for ThreadNameString { type Target = CStr; fn deref(&self) -> &CStr { &self.inner } } + impl From for ThreadNameString { fn from(s: String) -> Self { Self { @@ -1263,34 +1267,82 @@ mod thread_name_string { } } } - impl ThreadName { - pub fn as_cstr(&self) -> Option<&CStr> { - match self { - ThreadName::Main => Some(c"main"), - ThreadName::Other(other) => Some(other), - ThreadName::Unnamed => None, - } - } - - pub fn as_str(&self) -> Option<&str> { - // SAFETY: `as_cstr` can only return `Some` for a fixed CStr or a `ThreadNameString`, - // which is guaranteed to be UTF-8. - self.as_cstr().map(|s| unsafe { str::from_utf8_unchecked(s.to_bytes()) }) - } - } } pub(crate) use thread_name_string::ThreadNameString; -/// The internal representation of a `Thread` handle -struct Inner { - name: ThreadName, // Guaranteed to be UTF-8 +static MAIN_THREAD_INFO: SyncUnsafeCell<(MaybeUninit, MaybeUninit)> = + SyncUnsafeCell::new((MaybeUninit::uninit(), MaybeUninit::uninit())); + +/// The internal representation of a `Thread` that is not the main thread. +struct OtherInner { + name: Option, id: ThreadId, parker: Parker, } +/// The internal representation of a `Thread` handle. +#[derive(Clone)] +enum Inner { + /// Represents the main thread. May only be constructed by Thread::new_main. + Main(&'static (ThreadId, Parker)), + /// Represents any other thread. + Other(Pin>), +} + impl Inner { - fn parker(self: Pin<&Self>) -> Pin<&Parker> { - unsafe { Pin::map_unchecked(self, |inner| &inner.parker) } + fn id(&self) -> ThreadId { + match self { + Self::Main((thread_id, _)) => *thread_id, + Self::Other(other) => other.id, + } + } + + fn cname(&self) -> Option<&CStr> { + match self { + Self::Main(_) => Some(c"main"), + Self::Other(other) => other.name.as_deref(), + } + } + + fn name(&self) -> Option<&str> { + match self { + Self::Main(_) => Some("main"), + Self::Other(other) => other.name.as_ref().map(ThreadNameString::as_str), + } + } + + fn into_raw(self) -> *const () { + match self { + // Just return the pointer to `MAIN_THREAD_INFO`. + Self::Main(ptr) => crate::ptr::from_ref(ptr).cast(), + Self::Other(arc) => { + // Safety: We only expose an opaque pointer, which maintains the `Pin` invariant. + let inner = unsafe { Pin::into_inner_unchecked(arc) }; + Arc::into_raw(inner) as *const () + } + } + } + + /// # Safety + /// + /// See [`Thread::from_raw`]. + unsafe fn from_raw(ptr: *const ()) -> Self { + // If the pointer is to `MAIN_THREAD_INFO`, we know it is the `Main` variant. + if crate::ptr::eq(ptr.cast(), &MAIN_THREAD_INFO) { + Self::Main(unsafe { &*ptr.cast() }) + } else { + // Safety: Upheld by caller + Self::Other(unsafe { Pin::new_unchecked(Arc::from_raw(ptr as *const OtherInner)) }) + } + } + + fn parker(&self) -> Pin<&Parker> { + match self { + Self::Main((_, parker_ref)) => Pin::static_ref(parker_ref), + Self::Other(inner) => unsafe { + Pin::map_unchecked(inner.as_ref(), |inner| &inner.parker) + }, + } } } @@ -1314,33 +1366,47 @@ impl Inner { /// docs of [`Builder`] and [`spawn`] for more details. /// /// [`thread::current`]: current::current -pub struct Thread { - inner: Pin>, -} +pub struct Thread(Inner); impl Thread { /// Used only internally to construct a thread object without spawning. pub(crate) fn new(id: ThreadId, name: String) -> Thread { - Self::new_inner(id, ThreadName::Other(name.into())) + Self::new_inner(id, Some(ThreadNameString::from(name))) } pub(crate) fn new_unnamed(id: ThreadId) -> Thread { - Self::new_inner(id, ThreadName::Unnamed) + Self::new_inner(id, None) } - /// Constructs the thread handle for the main thread. - pub(crate) fn new_main(id: ThreadId) -> Thread { - Self::new_inner(id, ThreadName::Main) + /// Used in runtime to construct main thread + /// + /// # Safety + /// + /// This must only ever be called once, and must be called on the main thread. + pub(crate) unsafe fn new_main(thread_id: ThreadId) -> Thread { + // Safety: As this is only called once and on the main thread, nothing else is accessing MAIN_THREAD_INFO + // as the only other read occurs in `main_thread_info` *after* the main thread has been constructed, + // and this function is the only one that constructs the main thread. + // + // Pre-main thread spawning cannot hit this either, as the caller promises that this is only called on the main thread. + let main_thread_info = unsafe { &mut *MAIN_THREAD_INFO.get() }; + + unsafe { Parker::new_in_place((&raw mut main_thread_info.1).cast()) }; + main_thread_info.0.write(thread_id); + + // Store a `'static` ref to the initialised ThreadId and Parker, + // to avoid having to repeatedly prove initialisation. + Self(Inner::Main(unsafe { &*MAIN_THREAD_INFO.get().cast() })) } - fn new_inner(id: ThreadId, name: ThreadName) -> Thread { + fn new_inner(id: ThreadId, name: Option) -> Thread { // We have to use `unsafe` here to construct the `Parker` in-place, // which is required for the UNIX implementation. // // SAFETY: We pin the Arc immediately after creation, so its address never // changes. let inner = unsafe { - let mut arc = Arc::::new_uninit(); + let mut arc = Arc::::new_uninit(); let ptr = Arc::get_mut_unchecked(&mut arc).as_mut_ptr(); (&raw mut (*ptr).name).write(name); (&raw mut (*ptr).id).write(id); @@ -1348,7 +1414,7 @@ impl Thread { Pin::new_unchecked(arc.assume_init()) }; - Thread { inner } + Self(Inner::Other(inner)) } /// Like the public [`park`], but callable on any handle. This is used to @@ -1357,7 +1423,7 @@ impl Thread { /// # Safety /// May only be called from the thread to which this handle belongs. pub(crate) unsafe fn park(&self) { - unsafe { self.inner.as_ref().parker().park() } + unsafe { self.0.parker().park() } } /// Atomically makes the handle's token available if it is not already. @@ -1393,7 +1459,7 @@ impl Thread { #[stable(feature = "rust1", since = "1.0.0")] #[inline] pub fn unpark(&self) { - self.inner.as_ref().parker().unpark(); + self.0.parker().unpark(); } /// Gets the thread's unique identifier. @@ -1413,7 +1479,7 @@ impl Thread { #[stable(feature = "thread_id", since = "1.19.0")] #[must_use] pub fn id(&self) -> ThreadId { - self.inner.id + self.0.id() } /// Gets the thread's name. @@ -1456,7 +1522,11 @@ impl Thread { #[stable(feature = "rust1", since = "1.0.0")] #[must_use] pub fn name(&self) -> Option<&str> { - self.inner.name.as_str() + self.0.name() + } + + fn cname(&self) -> Option<&CStr> { + self.0.cname() } /// Consumes the `Thread`, returning a raw pointer. @@ -1480,9 +1550,7 @@ impl Thread { /// ``` #[unstable(feature = "thread_raw", issue = "97523")] pub fn into_raw(self) -> *const () { - // Safety: We only expose an opaque pointer, which maintains the `Pin` invariant. - let inner = unsafe { Pin::into_inner_unchecked(self.inner) }; - Arc::into_raw(inner) as *const () + self.0.into_raw() } /// Constructs a `Thread` from a raw pointer. @@ -1504,11 +1572,7 @@ impl Thread { #[unstable(feature = "thread_raw", issue = "97523")] pub unsafe fn from_raw(ptr: *const ()) -> Thread { // Safety: Upheld by caller. - unsafe { Thread { inner: Pin::new_unchecked(Arc::from_raw(ptr as *const Inner)) } } - } - - fn cname(&self) -> Option<&CStr> { - self.inner.name.as_cstr() + unsafe { Thread(Inner::from_raw(ptr)) } } } diff --git a/tests/debuginfo/thread.rs b/tests/debuginfo/thread.rs index 0415f586f5d90..dc8cb0832192b 100644 --- a/tests/debuginfo/thread.rs +++ b/tests/debuginfo/thread.rs @@ -12,15 +12,15 @@ // cdb-check:join_handle,d [Type: std::thread::JoinHandle >] // cdb-check: [...] __0 [Type: std::thread::JoinInner >] // -// cdb-command:dx t,d +// cdb-command:dx -r3 t,d // cdb-check:t,d : [...] [Type: std::thread::Thread *] -// cdb-check:[...] inner [...][Type: core::pin::Pin >] +// cdb-check: [...] __0 : Other [Type: enum2$] +// cdb-check: [...] __0 [Type: core::pin::Pin >] use std::thread; #[allow(unused_variables)] -fn main() -{ +fn main() { let join_handle = thread::spawn(|| { println!("Initialize a thread"); }); diff --git a/tests/rustdoc/demo-allocator-54478.rs b/tests/rustdoc/demo-allocator-54478.rs index dd98e80f03ade..80acfc0ff58a1 100644 --- a/tests/rustdoc/demo-allocator-54478.rs +++ b/tests/rustdoc/demo-allocator-54478.rs @@ -40,6 +40,7 @@ //! } //! //! fn main() { +//! drop(String::from("An allocation")); //! assert!(unsafe { HIT }); //! } //! ```