From 6bc27a8011cbe978b105e3b3bea1ee7e45e65ba5 Mon Sep 17 00:00:00 2001 From: joboet Date: Wed, 17 Sep 2025 17:23:53 +0200 Subject: [PATCH 1/2] std: pass the `Thread` for the newly spawned thread to the platform --- library/std/src/rt.rs | 10 +++-- library/std/src/sys/thread/hermit.rs | 41 +++++++++++++-------- library/std/src/sys/thread/mod.rs | 44 +++++++--------------- library/std/src/sys/thread/sgx.rs | 19 ++++++---- library/std/src/sys/thread/solid.rs | 23 ++++++------ library/std/src/sys/thread/teeos.rs | 37 ++++++++++--------- library/std/src/sys/thread/unix.rs | 38 ++++++++++++++----- library/std/src/sys/thread/unsupported.rs | 7 +--- library/std/src/sys/thread/wasip1.rs | 25 ++++++++----- library/std/src/sys/thread/windows.rs | 45 ++++++++++++++++------- library/std/src/sys/thread/xous.rs | 37 ++++++++++++------- library/std/src/thread/current.rs | 16 +++++--- library/std/src/thread/mod.rs | 22 ++++------- 13 files changed, 204 insertions(+), 160 deletions(-) diff --git a/library/std/src/rt.rs b/library/std/src/rt.rs index b3f3b301e3db6..497982c7bdf54 100644 --- a/library/std/src/rt.rs +++ b/library/std/src/rt.rs @@ -109,14 +109,16 @@ fn handle_rt_panic(e: Box) -> T { // `compiler/rustc_session/src/config/sigpipe.rs`. #[cfg_attr(test, allow(dead_code))] unsafe fn init(argc: isize, argv: *const *const u8, sigpipe: u8) { + // Remember the main thread ID to give it the correct name. Do this + // immediately to make sure the correct name available to the platform + // functions. + // SAFETY: this is the only time and place where we call this function. + unsafe { main_thread::set(thread::current_id()) }; + #[cfg_attr(target_os = "teeos", allow(unused_unsafe))] unsafe { sys::init(argc, argv, sigpipe) }; - - // Remember the main thread ID to give it the correct name. - // SAFETY: this is the only time and place where we call this function. - unsafe { main_thread::set(thread::current_id()) }; } /// Clean up the thread-local runtime state. This *should* be run after all other diff --git a/library/std/src/sys/thread/hermit.rs b/library/std/src/sys/thread/hermit.rs index 4d9f3b114c2a0..90149c6daff6c 100644 --- a/library/std/src/sys/thread/hermit.rs +++ b/library/std/src/sys/thread/hermit.rs @@ -13,17 +13,23 @@ unsafe impl Sync for Thread {} pub const DEFAULT_MIN_STACK_SIZE: usize = 1 << 20; +struct ThreadData { + handle: crate::thread::Thread, + main: Box, +} + impl Thread { pub unsafe fn new_with_coreid( stack: usize, - p: Box, + handle: crate::thread::Thread, + main: Box, core_id: isize, ) -> io::Result { - let p = Box::into_raw(Box::new(p)); + let data = Box::into_raw(Box::new(ThreadData { handle, main })); let tid = unsafe { hermit_abi::spawn2( thread_start, - p.expose_provenance(), + data.expose_provenance(), hermit_abi::Priority::into(hermit_abi::NORMAL_PRIO), stack, core_id, @@ -31,10 +37,11 @@ impl Thread { }; return if tid == 0 { - // The thread failed to start and as a result p was not consumed. Therefore, it is - // safe to reconstruct the box so that it gets deallocated. + // The thread failed to start and as a result data was not consumed. + // Therefore, it is safe to reconstruct the box so that it gets + // deallocated. unsafe { - drop(Box::from_raw(p)); + drop(Box::from_raw(data)); } Err(io::const_error!(io::ErrorKind::Uncategorized, "unable to create thread!")) } else { @@ -42,24 +49,26 @@ impl Thread { }; extern "C" fn thread_start(main: usize) { - unsafe { - // Finally, let's run some code. - Box::from_raw(ptr::with_exposed_provenance::>(main).cast_mut())(); + let data = unsafe { + Box::from_raw(ptr::with_exposed_provenance::(main).cast_mut()) + }; - // run all destructors - crate::sys::thread_local::destructors::run(); - crate::rt::thread_cleanup(); - } + crate::thread::set_current(data.handle); + (data.main)(); + + // run all destructors + unsafe { crate::sys::thread_local::destructors::run() }; + crate::rt::thread_cleanup(); } } pub unsafe fn new( stack: usize, - _name: Option<&str>, - p: Box, + handle: crate::thread::Thread, + main: Box, ) -> io::Result { unsafe { - Thread::new_with_coreid(stack, p, -1 /* = no specific core */) + Thread::new_with_coreid(stack, handle, main, -1 /* = no specific core */) } } diff --git a/library/std/src/sys/thread/mod.rs b/library/std/src/sys/thread/mod.rs index 6bb7fc1a20e2f..6cef381a2f3b2 100644 --- a/library/std/src/sys/thread/mod.rs +++ b/library/std/src/sys/thread/mod.rs @@ -4,7 +4,7 @@ cfg_select! { pub use hermit::{Thread, available_parallelism, sleep, yield_now, DEFAULT_MIN_STACK_SIZE}; #[expect(dead_code)] mod unsupported; - pub use unsupported::{current_os_id, set_name}; + pub use unsupported::current_os_id; } all(target_vendor = "fortanix", target_env = "sgx") => { mod sgx; @@ -21,41 +21,32 @@ cfg_select! { // as-is with the SGX target. #[expect(dead_code)] mod unsupported; - pub use unsupported::{available_parallelism, set_name}; + pub use unsupported::available_parallelism; } target_os = "solid_asp3" => { mod solid; pub use solid::{Thread, sleep, yield_now, DEFAULT_MIN_STACK_SIZE}; #[expect(dead_code)] mod unsupported; - pub use unsupported::{available_parallelism, current_os_id, set_name}; + pub use unsupported::{available_parallelism, current_os_id}; } target_os = "teeos" => { mod teeos; pub use teeos::{Thread, sleep, yield_now, DEFAULT_MIN_STACK_SIZE}; #[expect(dead_code)] mod unsupported; - pub use unsupported::{available_parallelism, current_os_id, set_name}; + pub use unsupported::{available_parallelism, current_os_id}; } target_os = "uefi" => { mod uefi; pub use uefi::{available_parallelism, sleep}; #[expect(dead_code)] mod unsupported; - pub use unsupported::{Thread, current_os_id, set_name, yield_now, DEFAULT_MIN_STACK_SIZE}; + pub use unsupported::{Thread, current_os_id, yield_now, DEFAULT_MIN_STACK_SIZE}; } target_family = "unix" => { mod unix; pub use unix::{Thread, available_parallelism, current_os_id, sleep, yield_now, DEFAULT_MIN_STACK_SIZE}; - #[cfg(not(any( - target_env = "newlib", - target_os = "l4re", - target_os = "emscripten", - target_os = "redox", - target_os = "hurd", - target_os = "aix", - )))] - pub use unix::set_name; #[cfg(any( target_os = "freebsd", target_os = "netbsd", @@ -69,17 +60,8 @@ cfg_select! { target_os = "vxworks", ))] pub use unix::sleep_until; - #[expect(dead_code)] - mod unsupported; - #[cfg(any( - target_env = "newlib", - target_os = "l4re", - target_os = "emscripten", - target_os = "redox", - target_os = "hurd", - target_os = "aix", - ))] - pub use unsupported::set_name; + // This is used to name the main thread in the platform init function. + pub(super) use unix::set_name; } all(target_os = "wasi", target_env = "p1") => { mod wasip1; @@ -88,7 +70,7 @@ cfg_select! { pub use wasip1::{Thread, available_parallelism}; #[expect(dead_code)] mod unsupported; - pub use unsupported::{current_os_id, set_name}; + pub use unsupported::current_os_id; #[cfg(not(target_feature = "atomics"))] pub use unsupported::{Thread, available_parallelism}; } @@ -100,7 +82,7 @@ cfg_select! { // Note that unlike WASIp1 even if the wasm `atomics` feature is enabled // there is no support for threads, not even experimentally, not even in // wasi-libc. Thus this is unconditionally unsupported. - pub use unsupported::{Thread, available_parallelism, current_os_id, set_name, yield_now, DEFAULT_MIN_STACK_SIZE}; + pub use unsupported::{Thread, available_parallelism, current_os_id, yield_now, DEFAULT_MIN_STACK_SIZE}; } all(target_family = "wasm", target_feature = "atomics") => { mod wasm; @@ -108,11 +90,11 @@ cfg_select! { #[expect(dead_code)] mod unsupported; - pub use unsupported::{Thread, available_parallelism, current_os_id, set_name, yield_now, DEFAULT_MIN_STACK_SIZE}; + pub use unsupported::{Thread, available_parallelism, current_os_id, yield_now, DEFAULT_MIN_STACK_SIZE}; } target_os = "windows" => { mod windows; - pub use windows::{Thread, available_parallelism, current_os_id, set_name, set_name_wide, sleep, yield_now, DEFAULT_MIN_STACK_SIZE}; + pub use windows::{Thread, available_parallelism, current_os_id, set_name_wide, sleep, yield_now, DEFAULT_MIN_STACK_SIZE}; } target_os = "xous" => { mod xous; @@ -120,11 +102,11 @@ cfg_select! { #[expect(dead_code)] mod unsupported; - pub use unsupported::{current_os_id, set_name}; + pub use unsupported::current_os_id; } _ => { mod unsupported; - pub use unsupported::{Thread, available_parallelism, current_os_id, set_name, sleep, yield_now, DEFAULT_MIN_STACK_SIZE}; + pub use unsupported::{Thread, available_parallelism, current_os_id, sleep, yield_now, DEFAULT_MIN_STACK_SIZE}; } } diff --git a/library/std/src/sys/thread/sgx.rs b/library/std/src/sys/thread/sgx.rs index f20ef7d86b9c7..ecf725f967e9f 100644 --- a/library/std/src/sys/thread/sgx.rs +++ b/library/std/src/sys/thread/sgx.rs @@ -25,19 +25,24 @@ mod task_queue { } pub(super) struct Task { - p: Box, + handle: crate::thread::Thread, + main: Box, done: JoinNotifier, } impl Task { - pub(super) fn new(p: Box) -> (Task, JoinHandle) { + pub(super) fn new( + handle: crate::thread::Thread, + main: Box, + ) -> (Task, JoinHandle) { let (done, recv) = wait_notify::new(); let done = JoinNotifier(Some(done)); - (Task { p, done }, recv) + (Task { handle, main, done }, recv) } pub(super) fn run(self) -> JoinNotifier { - (self.p)(); + crate::thread::set_current(self.handle); + (self.main)(); self.done } } @@ -95,12 +100,12 @@ impl Thread { // unsafe: see thread::Builder::spawn_unchecked for safety requirements pub unsafe fn new( _stack: usize, - _name: Option<&str>, - p: Box, + handle: crate::thread::Thread, + main: Box, ) -> io::Result { let mut queue_lock = task_queue::lock(); unsafe { usercalls::launch_thread()? }; - let (task, handle) = task_queue::Task::new(p); + let (task, handle) = task_queue::Task::new(handle, main); queue_lock.push(task); Ok(Thread(handle)) } diff --git a/library/std/src/sys/thread/solid.rs b/library/std/src/sys/thread/solid.rs index 46a84faa80225..871b6b6482517 100644 --- a/library/std/src/sys/thread/solid.rs +++ b/library/std/src/sys/thread/solid.rs @@ -27,9 +27,9 @@ unsafe impl Sync for Thread {} /// State data shared between a parent thread and child thread. It's dropped on /// a transition to one of the final states. struct ThreadInner { - /// This field is used on thread creation to pass a closure from - /// `Thread::new` to the created task. - start: UnsafeCell>>, + /// This field is used on thread creation to pass the thread handle and closure + /// to the newly created thread. + data: UnsafeCell)>>, /// A state machine. Each transition is annotated with `[...]` in the /// source code. @@ -65,7 +65,7 @@ struct ThreadInner { lifecycle: Atomic, } -// Safety: The only `!Sync` field, `ThreadInner::start`, is only touched by +// Safety: The only `!Sync` field, `ThreadInner::data`, is only touched by // the task represented by `ThreadInner`. unsafe impl Sync for ThreadInner {} @@ -86,11 +86,11 @@ impl Thread { /// See `thread::Builder::spawn_unchecked` for safety requirements. pub unsafe fn new( stack: usize, - _name: Option<&str>, - p: Box, + handle: crate::thread::Thread, + main: Box, ) -> io::Result { let inner = Box::new(ThreadInner { - start: UnsafeCell::new(ManuallyDrop::new(p)), + data: UnsafeCell::new(ManuallyDrop::new((handle, main))), lifecycle: AtomicUsize::new(LIFECYCLE_INIT), }); @@ -100,10 +100,11 @@ impl Thread { let inner = unsafe { &*p_inner }; // Safety: Since `trampoline` is called only once for each - // `ThreadInner` and only `trampoline` touches `start`, - // `start` contains contents and is safe to mutably borrow. - let p = unsafe { ManuallyDrop::take(&mut *inner.start.get()) }; - p(); + // `ThreadInner` and only `trampoline` touches `data`, + // `data` contains contents and is safe to mutably borrow. + let (handle, main) = unsafe { ManuallyDrop::take(&mut *inner.data.get()) }; + crate::thread::set_current(handle); + main(); // Fix the current thread's state just in case, so that the // destructors won't abort diff --git a/library/std/src/sys/thread/teeos.rs b/library/std/src/sys/thread/teeos.rs index cad100395c9f8..2863448fa200f 100644 --- a/library/std/src/sys/thread/teeos.rs +++ b/library/std/src/sys/thread/teeos.rs @@ -22,14 +22,19 @@ pub struct Thread { unsafe impl Send for Thread {} unsafe impl Sync for Thread {} +struct ThreadData { + handle: crate::thread::Thread, + main: Box, +} + impl Thread { // unsafe: see thread::Builder::spawn_unchecked for safety requirements pub unsafe fn new( stack: usize, - _name: Option<&str>, - p: Box, + handle: crate::thread::Thread, + main: Box, ) -> io::Result { - let p = Box::into_raw(Box::new(p)); + let data = Box::into_raw(Box::new(ThreadData { handle, main })); let mut native: libc::pthread_t = unsafe { mem::zeroed() }; let mut attr: libc::pthread_attr_t = unsafe { mem::zeroed() }; assert_eq!(unsafe { libc::pthread_attr_init(&mut attr) }, 0); @@ -62,16 +67,17 @@ impl Thread { } }; - let ret = unsafe { libc::pthread_create(&mut native, &attr, thread_start, p as *mut _) }; - // Note: if the thread creation fails and this assert fails, then p will - // be leaked. However, an alternative design could cause double-free + let ret = unsafe { libc::pthread_create(&mut native, &attr, thread_start, data as *mut _) }; + // Note: if the thread creation fails and this assert fails, then data + // will be leaked. However, an alternative design could cause double-free // which is clearly worse. assert_eq!(unsafe { libc::pthread_attr_destroy(&mut attr) }, 0); return if ret != 0 { - // The thread failed to start and as a result p was not consumed. Therefore, it is - // safe to reconstruct the box so that it gets deallocated. - drop(unsafe { Box::from_raw(p) }); + // The thread failed to start and as a result data was not consumed. + // Therefore, it is safe to reconstruct the box so that it gets + // deallocated. + drop(unsafe { Box::from_raw(data) }); Err(io::Error::from_raw_os_error(ret)) } else { // The new thread will start running earliest after the next yield. @@ -80,15 +86,10 @@ impl Thread { Ok(Thread { id: native }) }; - extern "C" fn thread_start(main: *mut libc::c_void) -> *mut libc::c_void { - unsafe { - // Next, set up our stack overflow handler which may get triggered if we run - // out of stack. - // this is not necessary in TEE. - //let _handler = stack_overflow::Handler::new(); - // Finally, let's run some code. - Box::from_raw(main as *mut Box)(); - } + extern "C" fn thread_start(data: *mut libc::c_void) -> *mut libc::c_void { + let data = unsafe { Box::from_raw(data as *mut ThreadData) }; + crate::thread::set_current(data.handle); + (data.main)(); ptr::null_mut() } } diff --git a/library/std/src/sys/thread/unix.rs b/library/std/src/sys/thread/unix.rs index 2d2c4f9021288..a91e891683fc1 100644 --- a/library/std/src/sys/thread/unix.rs +++ b/library/std/src/sys/thread/unix.rs @@ -31,8 +31,8 @@ pub const DEFAULT_MIN_STACK_SIZE: usize = 256 * 1024; pub const DEFAULT_MIN_STACK_SIZE: usize = 0; // 0 indicates that the stack size configured in the ESP-IDF/NuttX menuconfig system should be used struct ThreadData { - name: Option>, - f: Box, + handle: crate::thread::Thread, + main: Box, } pub struct Thread { @@ -49,10 +49,10 @@ impl Thread { #[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces pub unsafe fn new( stack: usize, - name: Option<&str>, - f: Box, + handle: crate::thread::Thread, + main: Box, ) -> io::Result { - let data = Box::into_raw(Box::new(ThreadData { name: name.map(Box::from), f })); + let data = Box::into_raw(Box::new(ThreadData { handle, main })); let mut native: libc::pthread_t = mem::zeroed(); let mut attr: mem::MaybeUninit = mem::MaybeUninit::uninit(); assert_eq!(libc::pthread_attr_init(attr.as_mut_ptr()), 0); @@ -102,9 +102,10 @@ impl Thread { } let ret = libc::pthread_create(&mut native, attr.as_ptr(), thread_start, data as *mut _); - // Note: if the thread creation fails and this assert fails, then p will - // be leaked. However, an alternative design could cause double-free + // Note: if the thread creation fails and this assert fails, then data + // will be leaked. However, an alternative design could cause double-free // which is clearly worse. + // FIXME(joboet): AAAaaaAAhh! Just use a `DropGuard` for the attributes! assert_eq!(libc::pthread_attr_destroy(attr.as_mut_ptr()), 0); return if ret != 0 { @@ -119,11 +120,20 @@ impl Thread { extern "C" fn thread_start(data: *mut libc::c_void) -> *mut libc::c_void { unsafe { let data = Box::from_raw(data as *mut ThreadData); + + if let Some(name) = data.handle.cname() { + set_name(name); + } + + // `set_name` will not initialize the thread handle, so this + // will not abort. + crate::thread::set_current(data.handle); + // Next, set up our stack overflow handler which may get triggered if we run // out of stack. - let _handler = stack_overflow::Handler::new(data.name); + let _handler = stack_overflow::Handler::new(); // Finally, let's run some code. - (data.f)(); + (data.main)(); } ptr::null_mut() } @@ -529,6 +539,16 @@ pub fn set_name(name: &CStr) { debug_assert_eq!(res, libc::OK); } +#[cfg(any( + target_env = "newlib", + target_os = "l4re", + target_os = "emscripten", + target_os = "redox", + target_os = "hurd", + target_os = "aix", +))] +pub fn set_name(_name: &CStr) {} + #[cfg(not(target_os = "espidf"))] pub fn sleep(dur: Duration) { let mut secs = dur.as_secs(); diff --git a/library/std/src/sys/thread/unsupported.rs b/library/std/src/sys/thread/unsupported.rs index a5001efa3b405..33138a8b452f5 100644 --- a/library/std/src/sys/thread/unsupported.rs +++ b/library/std/src/sys/thread/unsupported.rs @@ -1,4 +1,3 @@ -use crate::ffi::CStr; use crate::io; use crate::num::NonZero; use crate::time::Duration; @@ -11,7 +10,7 @@ impl Thread { // unsafe: see thread::Builder::spawn_unchecked for safety requirements pub unsafe fn new( _stack: usize, - _name: Option<&str>, + _handle: crate::thread::Thread, _p: Box, ) -> io::Result { Err(io::Error::UNSUPPORTED_PLATFORM) @@ -34,10 +33,6 @@ pub fn yield_now() { // do nothing } -pub fn set_name(_name: &CStr) { - // nope -} - pub fn sleep(_dur: Duration) { panic!("can't sleep"); } diff --git a/library/std/src/sys/thread/wasip1.rs b/library/std/src/sys/thread/wasip1.rs index 83001fad49c81..7cd8829149111 100644 --- a/library/std/src/sys/thread/wasip1.rs +++ b/library/std/src/sys/thread/wasip1.rs @@ -70,15 +70,21 @@ impl Drop for Thread { pub const DEFAULT_MIN_STACK_SIZE: usize = 1024 * 1024; +#[cfg(target_feature = "atomics")] +struct ThreadData { + handle: crate::thread::Thread, + main: Box, +} + #[cfg(target_feature = "atomics")] impl Thread { // unsafe: see thread::Builder::spawn_unchecked for safety requirements pub unsafe fn new( stack: usize, - _name: Option<&str>, - p: Box, + handle: crate::thread::Thread, + main: Box, ) -> io::Result { - let p = Box::into_raw(Box::new(p)); + let data = Box::into_raw(Box::new(ThreadData { handle, main })); let mut native: libc::pthread_t = unsafe { mem::zeroed() }; let mut attr: libc::pthread_attr_t = unsafe { mem::zeroed() }; assert_eq!(unsafe { libc::pthread_attr_init(&mut attr) }, 0); @@ -100,7 +106,7 @@ impl Thread { } }; - let ret = unsafe { libc::pthread_create(&mut native, &attr, thread_start, p as *mut _) }; + let ret = unsafe { libc::pthread_create(&mut native, &attr, thread_start, data as *mut _) }; // Note: if the thread creation fails and this assert fails, then p will // be leaked. However, an alternative design could cause double-free // which is clearly worse. @@ -110,18 +116,17 @@ impl Thread { // The thread failed to start and as a result p was not consumed. Therefore, it is // safe to reconstruct the box so that it gets deallocated. unsafe { - drop(Box::from_raw(p)); + drop(Box::from_raw(data)); } Err(io::Error::from_raw_os_error(ret)) } else { Ok(Thread { id: native }) }; - extern "C" fn thread_start(main: *mut libc::c_void) -> *mut libc::c_void { - unsafe { - // Finally, let's run some code. - Box::from_raw(main as *mut Box)(); - } + extern "C" fn thread_start(data: *mut libc::c_void) -> *mut libc::c_void { + let data = unsafe { Box::from_raw(data as *mut ThreadData) }; + crate::thread::set_current(data.handle); + (data.main)(); ptr::null_mut() } } diff --git a/library/std/src/sys/thread/windows.rs b/library/std/src/sys/thread/windows.rs index a5640c51c4a5d..73921dce9fc8f 100644 --- a/library/std/src/sys/thread/windows.rs +++ b/library/std/src/sys/thread/windows.rs @@ -17,15 +17,20 @@ pub struct Thread { handle: Handle, } +struct ThreadData { + handle: crate::thread::Thread, + main: Box, +} + impl Thread { // unsafe: see thread::Builder::spawn_unchecked for safety requirements #[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces pub unsafe fn new( stack: usize, - _name: Option<&str>, - p: Box, + handle: crate::thread::Thread, + main: Box, ) -> io::Result { - let p = Box::into_raw(Box::new(p)); + let data = Box::into_raw(Box::new(ThreadData { handle, main })); // CreateThread rounds up values for the stack size to the nearest page size (at least 4kb). // If a value of zero is given then the default stack size is used instead. @@ -36,7 +41,7 @@ impl Thread { ptr::null_mut(), stack, Some(thread_start), - p as *mut _, + data as *mut _, c::STACK_SIZE_PARAM_IS_A_RESERVATION, ptr::null_mut(), ); @@ -45,19 +50,33 @@ impl Thread { return if let Ok(handle) = ret.try_into() { Ok(Thread { handle: Handle::from_inner(handle) }) } else { - // The thread failed to start and as a result p was not consumed. Therefore, it is - // safe to reconstruct the box so that it gets deallocated. - unsafe { drop(Box::from_raw(p)) }; + // The thread failed to start and as a result data was not consumed. + // Therefore, it is safe to reconstruct the box so that it gets + // deallocated. + unsafe { drop(Box::from_raw(data)) }; Err(io::Error::last_os_error()) }; - unsafe extern "system" fn thread_start(main: *mut c_void) -> u32 { - // Next, reserve some stack space for if we otherwise run out of stack. - stack_overflow::reserve_stack(); - // Finally, let's run some code. + unsafe extern "system" fn thread_start(data: *mut c_void) -> u32 { // SAFETY: We are simply recreating the box that was leaked earlier. // It's the responsibility of the one who call `Thread::new` to ensure this is safe to call here. - unsafe { Box::from_raw(main as *mut Box)() }; + let data = unsafe { Box::from_raw(data as *mut ThreadData) }; + + // Call `set_current` immediately, as `set_name` may call the global + // allocator, which in turn might try to access the thread handle. + crate::thread::set_current(data.handle.clone()); + + // Reserve some stack space for if we otherwise run out of stack. + stack_overflow::reserve_stack(); + + // Name the thread... + if let Some(name) = data.handle.cname() { + set_name(name); + } + + // ... and run its main function. + (data.main)(); + 0 } } @@ -98,7 +117,7 @@ pub fn current_os_id() -> Option { if id == 0 { None } else { Some(id.into()) } } -pub fn set_name(name: &CStr) { +fn set_name(name: &CStr) { if let Ok(utf8) = name.to_str() { if let Ok(utf16) = to_u16s(utf8) { unsafe { diff --git a/library/std/src/sys/thread/xous.rs b/library/std/src/sys/thread/xous.rs index 133e15a0928c6..43b8900dd86f0 100644 --- a/library/std/src/sys/thread/xous.rs +++ b/library/std/src/sys/thread/xous.rs @@ -17,14 +17,19 @@ pub const DEFAULT_MIN_STACK_SIZE: usize = 131072; const MIN_STACK_SIZE: usize = 4096; pub const GUARD_PAGE_SIZE: usize = 4096; +struct ThreadData { + handle: crate::thread::Thread, + main: Box, +} + impl Thread { // unsafe: see thread::Builder::spawn_unchecked for safety requirements pub unsafe fn new( stack: usize, - _name: Option<&str>, - p: Box, + handle: crate::thread::Thread, + main: Box, ) -> io::Result { - let p = Box::into_raw(Box::new(p)); + let data = Box::into_raw(Box::new(ThreadData { handle, main })); let mut stack_size = crate::cmp::max(stack, MIN_STACK_SIZE); if (stack_size & 4095) != 0 { @@ -62,25 +67,31 @@ impl Thread { }; let guard_page_pre = stack_plus_guard_pages.as_ptr() as usize; - let tid = create_thread( + match create_thread( thread_start as *mut usize, &mut stack_plus_guard_pages[GUARD_PAGE_SIZE..(stack_size + GUARD_PAGE_SIZE)], - p as usize, + data.expose_provenance(), guard_page_pre, stack_size, 0, - ) - .map_err(|code| io::Error::from_raw_os_error(code as i32))?; + ) { + Ok(tid) => return Ok(Thread { tid }), + Err(code) => { + drop(unsafe { Box::from_raw(data) }); + return Err(io::Error::from_raw_os_error(code as i32)); + } + } extern "C" fn thread_start( - main: *mut usize, + data: *mut usize, guard_page_pre: usize, stack_size: usize, ) -> ! { - unsafe { - // Run the contents of the new thread. - Box::from_raw(main as *mut Box)(); - } + let data = unsafe { Box::from_raw(data as *mut ThreadData) }; + crate::thread::set_current(data.handle); + + // Run the contents of the new thread. + (data.main)(); // Destroy TLS, which will free the TLS page and call the destructor for // any thread local storage (if any). @@ -105,8 +116,6 @@ impl Thread { ); } } - - Ok(Thread { tid }) } pub fn join(self) { diff --git a/library/std/src/thread/current.rs b/library/std/src/thread/current.rs index 7da1621da45ce..dbfd280d1127e 100644 --- a/library/std/src/thread/current.rs +++ b/library/std/src/thread/current.rs @@ -113,24 +113,28 @@ pub(super) mod id { } } -/// Tries to set the thread handle for the current thread. Fails if a handle was -/// already set or if the thread ID of `thread` would change an already-set ID. -pub(super) fn set_current(thread: Thread) -> Result<(), Thread> { +/// Set the thread handle for the current thread. +/// +/// As this function aborts the process if a handle was already set or if the +/// thread ID of `thread` would change an already-set ID, it should be the +/// first thing called upon thread entry. +pub(crate) fn set_current(thread: Thread) { if CURRENT.get() != NONE { - return Err(thread); + // Use an abort to save binary size (see #123356). + rtabort!("preexisting thread handle"); } match id::get() { Some(id) if id == thread.id() => {} None => id::set(thread.id()), - _ => return Err(thread), + // Use an abort to save binary size (see #123356). + _ => rtabort!("preexisting thread ID"), } // Make sure that `crate::rt::thread_cleanup` will be run, which will // call `drop_current`. crate::sys::thread_local::guard::enable(); CURRENT.set(thread.into_raw().cast_mut()); - Ok(()) } /// Gets the id of the thread that invokes it. diff --git a/library/std/src/thread/mod.rs b/library/std/src/thread/mod.rs index 4d09b2b4e9d2e..af48b19b8be16 100644 --- a/library/std/src/thread/mod.rs +++ b/library/std/src/thread/mod.rs @@ -183,8 +183,10 @@ mod current; #[stable(feature = "rust1", since = "1.0.0")] pub use current::current; +#[allow(unused)] // Unused on platforms without threads. +pub(crate) use current::set_current; +use current::try_with_current; pub(crate) use current::{current_id, current_or_unnamed, current_os_id, drop_current}; -use current::{set_current, try_with_current}; mod spawnhook; @@ -541,18 +543,6 @@ impl Builder { let f = MaybeDangling::new(f); let main = move || { - if let Err(_thread) = set_current(their_thread.clone()) { - // Both the current thread handle and the ID should not be - // initialized yet. Since only the C runtime and some of our - // platform code run before this, this point shouldn't be - // reachable. Use an abort to save binary size (see #123356). - rtabort!("something here is badly broken!"); - } - - if let Some(name) = their_thread.cname() { - imp::set_name(name); - } - let f = f.into_inner(); let try_result = panic::catch_unwind(panic::AssertUnwindSafe(|| { crate::sys::backtrace::__rust_begin_short_backtrace(|| hooks.run()); @@ -595,7 +585,7 @@ impl Builder { // Similarly, the `sys` implementation must guarantee that no references to the closure // exist after the thread has terminated, which is signaled by `Thread::join` // returning. - native: unsafe { imp::Thread::new(stack_size, my_thread.name(), main)? }, + native: unsafe { imp::Thread::new(stack_size, their_thread, main)? }, thread: my_thread, packet: my_packet, }) @@ -1672,7 +1662,9 @@ impl Thread { unsafe { Thread { inner: Pin::new_unchecked(Arc::from_raw(ptr as *const Inner)) } } } - fn cname(&self) -> Option<&CStr> { + // This isn't used on platforms without an API to name threads. + #[allow(unused)] + pub(crate) fn cname(&self) -> Option<&CStr> { if let Some(name) = &self.inner.name { Some(name.as_cstr()) } else if main_thread::get() == Some(self.inner.id) { From ce0c3e91a524e7494bef033961f983bb0471491a Mon Sep 17 00:00:00 2001 From: joboet Date: Wed, 17 Sep 2025 17:24:25 +0200 Subject: [PATCH 2/2] std: don't call `current_os_id` from signal handler --- .../std/src/sys/pal/unix/stack_overflow.rs | 33 ++++++++++--------- .../pal/unix/stack_overflow/thread_info.rs | 10 ++++-- 2 files changed, 24 insertions(+), 19 deletions(-) diff --git a/library/std/src/sys/pal/unix/stack_overflow.rs b/library/std/src/sys/pal/unix/stack_overflow.rs index 0d2100d66bc09..413d01dc7c780 100644 --- a/library/std/src/sys/pal/unix/stack_overflow.rs +++ b/library/std/src/sys/pal/unix/stack_overflow.rs @@ -8,8 +8,8 @@ pub struct Handler { } impl Handler { - pub unsafe fn new(thread_name: Option>) -> Handler { - make_handler(false, thread_name) + pub unsafe fn new() -> Handler { + make_handler(false) } fn null() -> Handler { @@ -118,8 +118,15 @@ mod imp { if let Some(thread_info) = thread_info && thread_info.guard_page_range.contains(&fault_addr) { - let name = thread_info.thread_name.as_deref().unwrap_or(""); - let tid = crate::thread::current_os_id(); + // Hey you! Yes, you modifying the stack overflow message! + // Please make sure that all functions called here are + // actually async-signal-safe. If they're not, try retrieving + // the information beforehand and storing it in `ThreadInfo`. + // Thank you! + // - says Jonas after having had to watch his carefully + // written code get made unsound again. + let tid = thread_info.tid; + let name = thread_info.name.as_deref().unwrap_or(""); rtprintpanic!("\nthread '{name}' ({tid}) has overflowed its stack\n"); rtabort!("stack overflow"); } @@ -158,12 +165,12 @@ mod imp { if !NEED_ALTSTACK.load(Ordering::Relaxed) { // haven't set up our sigaltstack yet NEED_ALTSTACK.store(true, Ordering::Release); - let handler = unsafe { make_handler(true, None) }; + let handler = unsafe { make_handler(true) }; MAIN_ALTSTACK.store(handler.data, Ordering::Relaxed); mem::forget(handler); if let Some(guard_page_range) = guard_page_range.take() { - set_current_info(guard_page_range, Some(Box::from("main"))); + set_current_info(guard_page_range); } } @@ -229,14 +236,14 @@ mod imp { /// # Safety /// Mutates the alternate signal stack #[forbid(unsafe_op_in_unsafe_fn)] - pub unsafe fn make_handler(main_thread: bool, thread_name: Option>) -> Handler { + pub unsafe fn make_handler(main_thread: bool) -> Handler { if !NEED_ALTSTACK.load(Ordering::Acquire) { return Handler::null(); } if !main_thread { if let Some(guard_page_range) = unsafe { current_guard() } { - set_current_info(guard_page_range, thread_name); + set_current_info(guard_page_range); } } @@ -632,10 +639,7 @@ mod imp { pub unsafe fn cleanup() {} - pub unsafe fn make_handler( - _main_thread: bool, - _thread_name: Option>, - ) -> super::Handler { + pub unsafe fn make_handler(_main_thread: bool) -> super::Handler { super::Handler::null() } @@ -719,10 +723,7 @@ mod imp { pub unsafe fn cleanup() {} - pub unsafe fn make_handler( - main_thread: bool, - _thread_name: Option>, - ) -> super::Handler { + pub unsafe fn make_handler(main_thread: bool) -> super::Handler { if !main_thread { reserve_stack(); } diff --git a/library/std/src/sys/pal/unix/stack_overflow/thread_info.rs b/library/std/src/sys/pal/unix/stack_overflow/thread_info.rs index e81429b98a6c7..42eb0cd9a61af 100644 --- a/library/std/src/sys/pal/unix/stack_overflow/thread_info.rs +++ b/library/std/src/sys/pal/unix/stack_overflow/thread_info.rs @@ -32,8 +32,9 @@ use crate::sync::atomic::{AtomicUsize, Ordering}; use crate::sys::os::errno_location; pub struct ThreadInfo { + pub tid: u64, + pub name: Option>, pub guard_page_range: Range, - pub thread_name: Option>, } static LOCK: Mutex<()> = Mutex::new(()); @@ -108,14 +109,17 @@ fn spin_lock_in_setup(this: usize) -> UnlockOnDrop { } } -pub fn set_current_info(guard_page_range: Range, thread_name: Option>) { +pub fn set_current_info(guard_page_range: Range) { + let tid = crate::thread::current_os_id(); + let name = crate::thread::with_current_name(|name| name.map(Box::from)); + let this = errno_location().addr(); let _lock_guard = LOCK.lock(); let _spin_guard = spin_lock_in_setup(this); // SAFETY: we own the spin lock, so `THREAD_INFO` cannot be aliased. let thread_info = unsafe { &mut *(&raw mut THREAD_INFO) }; - thread_info.insert(this, ThreadInfo { guard_page_range, thread_name }); + thread_info.insert(this, ThreadInfo { tid, name, guard_page_range }); } pub fn delete_current_info() {