Skip to content

Commit

Permalink
std: lazily allocate the main thread handle
Browse files Browse the repository at this point in the history
#123550 eliminated the allocation of the main thread handle, but at the cost of greatly increased complexity. This PR proposes another approach: Instead of creating the main thread handle itself, the runtime simply remembers the thread ID of the main thread. The main thread handle is then only allocated when it is used, using the same lazy-initialization mechanism as for non-runtime use of `thread::current`, and the `name` method uses the thread ID to identify the main thread handle and return the correct name ("main") for it.

Thereby, we also allow accessing thread::current before main: as the runtime no longer tries to install its own handle, this will no longer trigger an abort. Rather, the name returned from name will only be "main" after the runtime initialization code has run, but I think that is acceptable.

This new approach also requires some changes to the signal handling code, as calling `thread::current` would now allocate when called on the main thread, which is not acceptable. I fixed this by adding a new function (`with_current_name`) that performs all the naming logic without allocation or without initializing the thread ID (which could allocate on some platforms).
  • Loading branch information
joboet committed Nov 25, 2024
1 parent 2a9b85b commit f13ff4b
Show file tree
Hide file tree
Showing 6 changed files with 134 additions and 99 deletions.
7 changes: 4 additions & 3 deletions library/std/src/panicking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,15 +247,16 @@ fn default_hook(info: &PanicHookInfo<'_>) {
let location = info.location().unwrap();

let msg = payload_as_str(info.payload());
let thread = thread::try_current();
let name = thread.as_ref().and_then(|t| t.name()).unwrap_or("<unnamed>");

let write = #[optimize(size)]
|err: &mut dyn crate::io::Write| {
// Use a lock to prevent mixed output in multithreading context.
// Some platforms also require it when printing a backtrace, like `SymFromAddr` on Windows.
let mut lock = backtrace::lock();
let _ = writeln!(err, "thread '{name}' panicked at {location}:\n{msg}");
thread::with_current_name(|name| {
let name = name.unwrap_or("<unnamed>");
let _ = writeln!(err, "thread '{name}' panicked at {location}:\n{msg}");
});

static FIRST_PANIC: AtomicBool = AtomicBool::new(true);

Expand Down
23 changes: 4 additions & 19 deletions library/std/src/rt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ pub use core::panicking::{panic_display, panic_fmt};
#[rustfmt::skip]
use crate::any::Any;
use crate::sync::Once;
use crate::thread::{self, Thread};
use crate::thread::{self, main_thread};
use crate::{mem, panic, sys};

// Prints to the "panic output", depending on the platform this may be:
Expand Down Expand Up @@ -102,24 +102,9 @@ unsafe fn init(argc: isize, argv: *const *const u8, sigpipe: u8) {
sys::init(argc, argv, sigpipe)
};

// Set up the current thread handle to give it the right name.
//
// When code running before main uses `ReentrantLock` (for example by
// using `println!`), the thread ID can become initialized before we
// create this handle. Since `set_current` fails when the ID of the
// 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());
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
// bad idea though, as we then cannot set the main thread name here.
//
// FIXME: detect the main thread in `thread::current` and use the
// correct name there.
rtabort!("code running before main must not use thread::current");
}
// 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
Expand Down
9 changes: 5 additions & 4 deletions library/std/src/sys/pal/unix/stack_overflow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,11 @@ mod imp {
// If the faulting address is within the guard page, then we print a
// message saying so and abort.
if start <= addr && addr < end {
rtprintpanic!(
"\nthread '{}' has overflowed its stack\n",
thread::current().name().unwrap_or("<unknown>")
);
thread::with_current_name(|name| {
let name = name.unwrap_or("<unknown>");
rtprintpanic!("\nthread '{name}' has overflowed its stack\n");
});

rtabort!("stack overflow");
} else {
// Unregister ourselves by reverting back to the default behavior.
Expand Down
8 changes: 4 additions & 4 deletions library/std/src/sys/pal/windows/stack_overflow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ unsafe extern "system" fn vectored_handler(ExceptionInfo: *mut c::EXCEPTION_POIN
let code = rec.ExceptionCode;

if code == c::EXCEPTION_STACK_OVERFLOW {
rtprintpanic!(
"\nthread '{}' has overflowed its stack\n",
thread::current().name().unwrap_or("<unknown>")
);
thread::with_current_name(|name| {
let name = name.unwrap_or("<unknown>");
rtprintpanic!("\nthread '{name}' has overflowed its stack\n");
});
}
c::EXCEPTION_CONTINUE_SEARCH
}
Expand Down
36 changes: 18 additions & 18 deletions library/std/src/thread/current.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ local_pointer! {
///
/// We store the thread ID so that it never gets destroyed during the lifetime
/// of a thread, either using `#[thread_local]` or multiple `local_pointer!`s.
mod id {
pub(super) mod id {
use super::*;

cfg_if::cfg_if! {
Expand All @@ -27,7 +27,7 @@ mod id {

pub(super) const CHEAP: bool = true;

pub(super) fn get() -> Option<ThreadId> {
pub(crate) fn get() -> Option<ThreadId> {
ID.get()
}

Expand All @@ -44,7 +44,7 @@ mod id {

pub(super) const CHEAP: bool = false;

pub(super) fn get() -> Option<ThreadId> {
pub(crate) fn get() -> Option<ThreadId> {
let id0 = ID0.get().addr() as u64;
let id16 = ID16.get().addr() as u64;
let id32 = ID32.get().addr() as u64;
Expand All @@ -67,7 +67,7 @@ mod id {

pub(super) const CHEAP: bool = false;

pub(super) fn get() -> Option<ThreadId> {
pub(crate) fn get() -> Option<ThreadId> {
let id0 = ID0.get().addr() as u64;
let id32 = ID32.get().addr() as u64;
ThreadId::from_u64((id32 << 32) + id0)
Expand All @@ -85,7 +85,7 @@ mod id {

pub(super) const CHEAP: bool = true;

pub(super) fn get() -> Option<ThreadId> {
pub(crate) fn get() -> Option<ThreadId> {
let id = ID.get().addr() as u64;
ThreadId::from_u64(id)
}
Expand All @@ -112,7 +112,7 @@ 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(crate) fn set_current(thread: Thread) -> Result<(), Thread> {
pub(super) fn set_current(thread: Thread) -> Result<(), Thread> {
if CURRENT.get() != NONE {
return Err(thread);
}
Expand Down Expand Up @@ -140,28 +140,28 @@ pub(crate) fn current_id() -> ThreadId {
// to retrieve it from the current thread handle, which will only take one
// TLS access.
if !id::CHEAP {
let current = CURRENT.get();
if current > DESTROYED {
unsafe {
let current = ManuallyDrop::new(Thread::from_raw(current));
return current.id();
}
if let Some(id) = try_with_current(|t| t.map(|t| t.id())) {
return id;
}
}

id::get_or_init()
}

/// Gets a handle to the thread that invokes it, if the handle has been initialized.
pub(crate) fn try_current() -> Option<Thread> {
/// Gets a reference to the handle of the thread that invokes it, if the handle
/// has been initialized.
pub(super) fn try_with_current<F, R>(f: F) -> R
where
F: FnOnce(Option<&Thread>) -> R,
{
let current = CURRENT.get();
if current > DESTROYED {
unsafe {
let current = ManuallyDrop::new(Thread::from_raw(current));
Some((*current).clone())
f(Some(&current))
}
} else {
None
f(None)
}
}

Expand All @@ -176,7 +176,7 @@ pub(crate) fn current_or_unnamed() -> Thread {
(*current).clone()
}
} else if current == DESTROYED {
Thread::new_unnamed(id::get_or_init())
Thread::new(id::get_or_init(), None)
} else {
init_current(current)
}
Expand Down Expand Up @@ -221,7 +221,7 @@ fn init_current(current: *mut ()) -> Thread {
CURRENT.set(BUSY);
// If the thread ID was initialized already, use it.
let id = id::get_or_init();
let thread = Thread::new_unnamed(id);
let thread = Thread::new(id, None);

// Make sure that `crate::rt::thread_cleanup` will be run, which will
// call `drop_current`.
Expand Down
Loading

0 comments on commit f13ff4b

Please sign in to comment.