From 7ac4c04731ecb6eedd63a1f899a0c6207679cbf9 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Wed, 22 May 2024 14:10:52 +0200 Subject: [PATCH 1/9] Add std::thread::add_spawn_hook. --- library/std/src/thread/mod.rs | 11 ++++ library/std/src/thread/spawnhook.rs | 92 +++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+) create mode 100644 library/std/src/thread/spawnhook.rs diff --git a/library/std/src/thread/mod.rs b/library/std/src/thread/mod.rs index 227ee9d64f375..ee8d688398d2c 100644 --- a/library/std/src/thread/mod.rs +++ b/library/std/src/thread/mod.rs @@ -188,6 +188,11 @@ mod current; pub use current::current; pub(crate) use current::{current_id, drop_current, set_current, try_current}; +mod spawnhook; + +#[unstable(feature = "thread_spawn_hook", issue = "none")] +pub use spawnhook::add_spawn_hook; + //////////////////////////////////////////////////////////////////////////////// // Thread-local storage //////////////////////////////////////////////////////////////////////////////// @@ -485,6 +490,9 @@ impl Builder { Some(name) => Thread::new(id, name.into()), None => Thread::new_unnamed(id), }; + + let hooks = spawnhook::run_spawn_hooks(&my_thread)?; + let their_thread = my_thread.clone(); let my_packet: Arc> = Arc::new(Packet { @@ -535,6 +543,9 @@ impl Builder { } crate::io::set_output_capture(output_capture); + for hook in hooks { + hook(); + } let f = f.into_inner(); let try_result = panic::catch_unwind(panic::AssertUnwindSafe(|| { diff --git a/library/std/src/thread/spawnhook.rs b/library/std/src/thread/spawnhook.rs new file mode 100644 index 0000000000000..c64aea4262b8c --- /dev/null +++ b/library/std/src/thread/spawnhook.rs @@ -0,0 +1,92 @@ +use crate::io; +use crate::sync::RwLock; +use crate::thread::Thread; + +static SPAWN_HOOKS: RwLock< + Vec<&'static (dyn Fn(&Thread) -> io::Result> + Sync)>, +> = RwLock::new(Vec::new()); + +/// Registers a function to run for every new thread spawned. +/// +/// The hook is executed in the parent thread, and returns a function +/// that will be executed in the new thread. +/// +/// The hook is called with the `Thread` handle for the new thread. +/// +/// If the hook returns an `Err`, thread spawning is aborted. In that case, the +/// function used to spawn the thread (e.g. `std::thread::spawn`) will return +/// the error returned by the hook. +/// +/// Hooks can only be added, not removed. +/// +/// The hooks will run in order, starting with the most recently added. +/// +/// # Usage +/// +/// ``` +/// #![feature(thread_spawn_hook)] +/// +/// std::thread::add_spawn_hook(|_| { +/// ..; // This will run in the parent (spawning) thread. +/// Ok(move || { +/// ..; // This will run it the child (spawned) thread. +/// }) +/// }); +/// ``` +/// +/// # Example +/// +/// A spawn hook can be used to initialize thread locals from the parent thread: +/// +/// ``` +/// #![feature(thread_spawn_hook)] +/// +/// use std::cell::Cell; +/// +/// thread_local! { +/// static X: Cell = Cell::new(0); +/// } +/// +/// std::thread::add_spawn_hook(|_| { +/// // Get the value of X in the spawning thread. +/// let value = X.get(); +/// // Set the value of X in the newly spawned thread. +/// Ok(move || { +/// X.set(value); +/// }) +/// }); +/// +/// X.set(123); +/// +/// std::thread::spawn(|| { +/// assert_eq!(X.get(), 123); +/// }).join().unwrap(); +/// ``` +#[unstable(feature = "thread_spawn_hook", issue = "none")] +pub fn add_spawn_hook(hook: F) +where + F: 'static + Sync + Fn(&Thread) -> io::Result, + G: 'static + Send + FnOnce(), +{ + SPAWN_HOOKS.write().unwrap_or_else(|e| e.into_inner()).push(Box::leak(Box::new( + move |thread: &Thread| -> io::Result<_> { + let f: Box = Box::new(hook(thread)?); + Ok(f) + }, + ))); +} + +/// Runs all the spawn hooks. +/// +/// Called on the parent thread. +/// +/// Returns the functions to be called on the newly spawned thread. +pub(super) fn run_spawn_hooks(thread: &Thread) -> io::Result>> { + SPAWN_HOOKS + .read() + .unwrap_or_else(|e| e.into_inner()) + .iter() + .rev() + .map(|hook| hook(thread)) + .collect() +} From ef9055f3eef409e99e1e786f6a90a1684fde810d Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Wed, 22 May 2024 14:16:33 +0200 Subject: [PATCH 2/9] Use add_spawn_hook for libtest's output capturing. --- library/std/src/thread/mod.rs | 4 ---- library/test/src/lib.rs | 11 +++++++++++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/library/std/src/thread/mod.rs b/library/std/src/thread/mod.rs index ee8d688398d2c..c6b1e0de84cf5 100644 --- a/library/std/src/thread/mod.rs +++ b/library/std/src/thread/mod.rs @@ -502,9 +502,6 @@ impl Builder { }); let their_packet = my_packet.clone(); - let output_capture = crate::io::set_output_capture(None); - crate::io::set_output_capture(output_capture.clone()); - // Pass `f` in `MaybeUninit` because actually that closure might *run longer than the lifetime of `F`*. // See for more details. // To prevent leaks we use a wrapper that drops its contents. @@ -542,7 +539,6 @@ impl Builder { imp::Thread::set_name(name); } - crate::io::set_output_capture(output_capture); for hook in hooks { hook(); } diff --git a/library/test/src/lib.rs b/library/test/src/lib.rs index 30ccfe2af8dbd..00a0b55c6a9f9 100644 --- a/library/test/src/lib.rs +++ b/library/test/src/lib.rs @@ -24,6 +24,7 @@ #![feature(process_exitcode_internals)] #![feature(panic_can_unwind)] #![feature(test)] +#![feature(thread_spawn_hook)] #![allow(internal_features)] #![warn(rustdoc::unescaped_backticks)] @@ -134,6 +135,16 @@ pub fn test_main(args: &[String], tests: Vec, options: Option Date: Thu, 24 Oct 2024 10:52:47 +0200 Subject: [PATCH 3/9] Update thread spawn hooks. 1. Make the effect thread local. 2. Don't return a io::Result from hooks. --- library/std/src/thread/mod.rs | 7 +- library/std/src/thread/spawnhook.rs | 112 ++++++++++++++++++++-------- library/test/src/lib.rs | 4 +- 3 files changed, 86 insertions(+), 37 deletions(-) diff --git a/library/std/src/thread/mod.rs b/library/std/src/thread/mod.rs index c6b1e0de84cf5..79621e7fa6bd4 100644 --- a/library/std/src/thread/mod.rs +++ b/library/std/src/thread/mod.rs @@ -491,7 +491,7 @@ impl Builder { None => Thread::new_unnamed(id), }; - let hooks = spawnhook::run_spawn_hooks(&my_thread)?; + let hooks = spawnhook::run_spawn_hooks(&my_thread); let their_thread = my_thread.clone(); @@ -539,12 +539,9 @@ impl Builder { imp::Thread::set_name(name); } - for hook in hooks { - hook(); - } - let f = f.into_inner(); let try_result = panic::catch_unwind(panic::AssertUnwindSafe(|| { + crate::sys::backtrace::__rust_begin_short_backtrace(|| hooks.run()); crate::sys::backtrace::__rust_begin_short_backtrace(f) })); // SAFETY: `their_packet` as been built just above and moved by the diff --git a/library/std/src/thread/spawnhook.rs b/library/std/src/thread/spawnhook.rs index c64aea4262b8c..9c4e9eb6aa13a 100644 --- a/library/std/src/thread/spawnhook.rs +++ b/library/std/src/thread/spawnhook.rs @@ -1,21 +1,43 @@ -use crate::io; -use crate::sync::RwLock; +use crate::cell::Cell; +use crate::sync::Arc; use crate::thread::Thread; -static SPAWN_HOOKS: RwLock< - Vec<&'static (dyn Fn(&Thread) -> io::Result> + Sync)>, -> = RwLock::new(Vec::new()); +// A thread local linked list of spawn hooks. +crate::thread_local! { + static SPAWN_HOOKS: Cell = const { Cell::new(SpawnHooks { first: None }) }; +} + +#[derive(Default, Clone)] +struct SpawnHooks { + first: Option>, +} + +// Manually implement drop to prevent deep recursion when dropping linked Arc list. +impl Drop for SpawnHooks { + fn drop(&mut self) { + let mut next = self.first.take(); + while let Some(SpawnHook { hook, next: n }) = next.and_then(|n| Arc::into_inner(n)) { + drop(hook); + next = n; + } + } +} + +struct SpawnHook { + hook: Box Box>, + next: Option>, +} -/// Registers a function to run for every new thread spawned. +/// Registers a function to run for every newly thread spawned. /// /// The hook is executed in the parent thread, and returns a function /// that will be executed in the new thread. /// /// The hook is called with the `Thread` handle for the new thread. /// -/// If the hook returns an `Err`, thread spawning is aborted. In that case, the -/// function used to spawn the thread (e.g. `std::thread::spawn`) will return -/// the error returned by the hook. +/// The hook will only be added for the current thread and is inherited by the threads it spawns. +/// In other words, adding a hook has no effect on already running threads (other than the current +/// thread) and the threads they might spawn in the future. /// /// Hooks can only be added, not removed. /// @@ -28,15 +50,15 @@ static SPAWN_HOOKS: RwLock< /// /// std::thread::add_spawn_hook(|_| { /// ..; // This will run in the parent (spawning) thread. -/// Ok(move || { +/// move || { /// ..; // This will run it the child (spawned) thread. -/// }) +/// } /// }); /// ``` /// /// # Example /// -/// A spawn hook can be used to initialize thread locals from the parent thread: +/// A spawn hook can be used to "inherit" a thread local from the parent thread: /// /// ``` /// #![feature(thread_spawn_hook)] @@ -47,13 +69,12 @@ static SPAWN_HOOKS: RwLock< /// static X: Cell = Cell::new(0); /// } /// +/// // This needs to be done once in the main thread before spawning any threads. /// std::thread::add_spawn_hook(|_| { /// // Get the value of X in the spawning thread. /// let value = X.get(); /// // Set the value of X in the newly spawned thread. -/// Ok(move || { -/// X.set(value); -/// }) +/// move || X.set(value) /// }); /// /// X.set(123); @@ -65,15 +86,17 @@ static SPAWN_HOOKS: RwLock< #[unstable(feature = "thread_spawn_hook", issue = "none")] pub fn add_spawn_hook(hook: F) where - F: 'static + Sync + Fn(&Thread) -> io::Result, + F: 'static + Sync + Fn(&Thread) -> G, G: 'static + Send + FnOnce(), { - SPAWN_HOOKS.write().unwrap_or_else(|e| e.into_inner()).push(Box::leak(Box::new( - move |thread: &Thread| -> io::Result<_> { - let f: Box = Box::new(hook(thread)?); - Ok(f) - }, - ))); + SPAWN_HOOKS.with(|h| { + let mut hooks = h.take(); + hooks.first = Some(Arc::new(SpawnHook { + hook: Box::new(move |thread| Box::new(hook(thread))), + next: hooks.first.take(), + })); + h.set(hooks); + }); } /// Runs all the spawn hooks. @@ -81,12 +104,41 @@ where /// Called on the parent thread. /// /// Returns the functions to be called on the newly spawned thread. -pub(super) fn run_spawn_hooks(thread: &Thread) -> io::Result>> { - SPAWN_HOOKS - .read() - .unwrap_or_else(|e| e.into_inner()) - .iter() - .rev() - .map(|hook| hook(thread)) - .collect() +pub(super) fn run_spawn_hooks(thread: &Thread) -> SpawnHookResults { + // Get a snapshot of the spawn hooks. + // (Increments the refcount to the first node.) + let hooks = SPAWN_HOOKS.with(|hooks| { + let snapshot = hooks.take(); + hooks.set(snapshot.clone()); + snapshot + }); + // Iterate over the hooks, run them, and collect the results in a vector. + let mut next: &Option> = &hooks.first; + let mut to_run = Vec::new(); + while let Some(hook) = next { + to_run.push((hook.hook)(thread)); + next = &hook.next; + } + // Pass on the snapshot of the hooks and the results to the new thread, + // which will then run SpawnHookResults::run(). + SpawnHookResults { hooks, to_run } +} + +/// The results of running the spawn hooks. +/// +/// This struct is sent to the new thread. +/// It contains the inherited hooks and the closures to be run. +pub(super) struct SpawnHookResults { + hooks: SpawnHooks, + to_run: Vec>, +} + +impl SpawnHookResults { + // This is run on the newly spawned thread, directly at the start. + pub(super) fn run(self) { + SPAWN_HOOKS.set(self.hooks); + for run in self.to_run { + run(); + } + } } diff --git a/library/test/src/lib.rs b/library/test/src/lib.rs index 00a0b55c6a9f9..47407df909bdf 100644 --- a/library/test/src/lib.rs +++ b/library/test/src/lib.rs @@ -141,9 +141,9 @@ pub fn test_main(args: &[String], tests: Vec, options: Option Date: Thu, 24 Oct 2024 11:19:02 +0200 Subject: [PATCH 4/9] Add thread Builder::no_hooks(). --- library/std/src/thread/mod.rs | 22 +++++++++++++++++++--- library/std/src/thread/spawnhook.rs | 9 +++++---- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/library/std/src/thread/mod.rs b/library/std/src/thread/mod.rs index 79621e7fa6bd4..4a4c2c4b96020 100644 --- a/library/std/src/thread/mod.rs +++ b/library/std/src/thread/mod.rs @@ -264,6 +264,8 @@ pub struct Builder { name: Option, // The size of the stack for the spawned thread in bytes stack_size: Option, + // Skip running and inheriting the thread spawn hooks + no_hooks: bool, } impl Builder { @@ -287,7 +289,7 @@ impl Builder { /// ``` #[stable(feature = "rust1", since = "1.0.0")] pub fn new() -> Builder { - Builder { name: None, stack_size: None } + Builder { name: None, stack_size: None, no_hooks: false } } /// Names the thread-to-be. Currently the name is used for identification @@ -343,6 +345,16 @@ impl Builder { self } + /// Disables running and inheriting [spawn hooks](add_spawn_hook). + /// + /// Use this if the parent thread is in no way relevant for the child thread. + /// For example, when lazily spawning threads for a thread pool. + #[unstable(feature = "thread_spawn_hook", issue = "none")] + pub fn no_hooks(mut self) -> Builder { + self.no_hooks = true; + self + } + /// Spawns a new thread by taking ownership of the `Builder`, and returns an /// [`io::Result`] to its [`JoinHandle`]. /// @@ -465,7 +477,7 @@ impl Builder { F: Send, T: Send, { - let Builder { name, stack_size } = self; + let Builder { name, stack_size, no_hooks } = self; let stack_size = stack_size.unwrap_or_else(|| { static MIN: AtomicUsize = AtomicUsize::new(0); @@ -491,7 +503,11 @@ impl Builder { None => Thread::new_unnamed(id), }; - let hooks = spawnhook::run_spawn_hooks(&my_thread); + let hooks = if no_hooks { + spawnhook::ChildSpawnHooks::default() + } else { + spawnhook::run_spawn_hooks(&my_thread) + }; let their_thread = my_thread.clone(); diff --git a/library/std/src/thread/spawnhook.rs b/library/std/src/thread/spawnhook.rs index 9c4e9eb6aa13a..b0732ae69c302 100644 --- a/library/std/src/thread/spawnhook.rs +++ b/library/std/src/thread/spawnhook.rs @@ -104,7 +104,7 @@ where /// Called on the parent thread. /// /// Returns the functions to be called on the newly spawned thread. -pub(super) fn run_spawn_hooks(thread: &Thread) -> SpawnHookResults { +pub(super) fn run_spawn_hooks(thread: &Thread) -> ChildSpawnHooks { // Get a snapshot of the spawn hooks. // (Increments the refcount to the first node.) let hooks = SPAWN_HOOKS.with(|hooks| { @@ -121,19 +121,20 @@ pub(super) fn run_spawn_hooks(thread: &Thread) -> SpawnHookResults { } // Pass on the snapshot of the hooks and the results to the new thread, // which will then run SpawnHookResults::run(). - SpawnHookResults { hooks, to_run } + ChildSpawnHooks { hooks, to_run } } /// The results of running the spawn hooks. /// /// This struct is sent to the new thread. /// It contains the inherited hooks and the closures to be run. -pub(super) struct SpawnHookResults { +#[derive(Default)] +pub(super) struct ChildSpawnHooks { hooks: SpawnHooks, to_run: Vec>, } -impl SpawnHookResults { +impl ChildSpawnHooks { // This is run on the newly spawned thread, directly at the start. pub(super) fn run(self) { SPAWN_HOOKS.set(self.hooks); From 5a80b48fe17f8ea772125dbc5220a134b1b15c68 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 24 Oct 2024 11:37:35 +0200 Subject: [PATCH 5/9] Use Send + Sync for spawn hooks. --- library/std/src/thread/spawnhook.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/std/src/thread/spawnhook.rs b/library/std/src/thread/spawnhook.rs index b0732ae69c302..f9a4a2e0da7da 100644 --- a/library/std/src/thread/spawnhook.rs +++ b/library/std/src/thread/spawnhook.rs @@ -24,7 +24,7 @@ impl Drop for SpawnHooks { } struct SpawnHook { - hook: Box Box>, + hook: Box Box>, next: Option>, } @@ -86,7 +86,7 @@ struct SpawnHook { #[unstable(feature = "thread_spawn_hook", issue = "none")] pub fn add_spawn_hook(hook: F) where - F: 'static + Sync + Fn(&Thread) -> G, + F: 'static + Send + Sync + Fn(&Thread) -> G, G: 'static + Send + FnOnce(), { SPAWN_HOOKS.with(|h| { From 24fec0d896ea160ce98113c9600d25f5db4d9827 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Tue, 12 Nov 2024 17:18:15 +0100 Subject: [PATCH 6/9] Add tracking issue. --- library/std/src/thread/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/std/src/thread/mod.rs b/library/std/src/thread/mod.rs index 4a4c2c4b96020..2c2cc58a9dda7 100644 --- a/library/std/src/thread/mod.rs +++ b/library/std/src/thread/mod.rs @@ -190,7 +190,7 @@ pub(crate) use current::{current_id, drop_current, set_current, try_current}; mod spawnhook; -#[unstable(feature = "thread_spawn_hook", issue = "none")] +#[unstable(feature = "thread_spawn_hook", issue = "132951")] pub use spawnhook::add_spawn_hook; //////////////////////////////////////////////////////////////////////////////// @@ -349,7 +349,7 @@ impl Builder { /// /// Use this if the parent thread is in no way relevant for the child thread. /// For example, when lazily spawning threads for a thread pool. - #[unstable(feature = "thread_spawn_hook", issue = "none")] + #[unstable(feature = "thread_spawn_hook", issue = "132951")] pub fn no_hooks(mut self) -> Builder { self.no_hooks = true; self From 38b9a448c9a00ca25c6fca352273d819cf92dacb Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Tue, 19 Nov 2024 15:19:13 +0100 Subject: [PATCH 7/9] Fix tracking issue. --- library/std/src/thread/spawnhook.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/thread/spawnhook.rs b/library/std/src/thread/spawnhook.rs index f9a4a2e0da7da..1513a5036cba1 100644 --- a/library/std/src/thread/spawnhook.rs +++ b/library/std/src/thread/spawnhook.rs @@ -83,7 +83,7 @@ struct SpawnHook { /// assert_eq!(X.get(), 123); /// }).join().unwrap(); /// ``` -#[unstable(feature = "thread_spawn_hook", issue = "none")] +#[unstable(feature = "thread_spawn_hook", issue = "132951")] pub fn add_spawn_hook(hook: F) where F: 'static + Send + Sync + Fn(&Thread) -> G, From b96f023dbd044e70eddd208cd21a295f62e5b28b Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Tue, 19 Nov 2024 17:50:42 +0000 Subject: [PATCH 8/9] Address review comments. Co-authored-by: waffle --- library/std/src/thread/spawnhook.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/library/std/src/thread/spawnhook.rs b/library/std/src/thread/spawnhook.rs index 1513a5036cba1..b979db6bd6059 100644 --- a/library/std/src/thread/spawnhook.rs +++ b/library/std/src/thread/spawnhook.rs @@ -1,4 +1,5 @@ use crate::cell::Cell; +use crate::iter; use crate::sync::Arc; use crate::thread::Thread; @@ -91,9 +92,10 @@ where { SPAWN_HOOKS.with(|h| { let mut hooks = h.take(); + let next = hooks.first.take(); hooks.first = Some(Arc::new(SpawnHook { hook: Box::new(move |thread| Box::new(hook(thread))), - next: hooks.first.take(), + next, })); h.set(hooks); }); @@ -113,12 +115,9 @@ pub(super) fn run_spawn_hooks(thread: &Thread) -> ChildSpawnHooks { snapshot }); // Iterate over the hooks, run them, and collect the results in a vector. - let mut next: &Option> = &hooks.first; - let mut to_run = Vec::new(); - while let Some(hook) = next { - to_run.push((hook.hook)(thread)); - next = &hook.next; - } + let to_run: Vec<_> = iter::successors(hooks.first.as_deref(), |hook| hook.next.as_deref()) + .map(|hook| (hook.hook)(thread)) + .collect(); // Pass on the snapshot of the hooks and the results to the new thread, // which will then run SpawnHookResults::run(). ChildSpawnHooks { hooks, to_run } From 691796be03d77951982a86f4913994791ed4fb61 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Tue, 19 Nov 2024 18:53:39 +0100 Subject: [PATCH 9/9] Update doc comments for spawn hook. --- library/std/src/thread/spawnhook.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/library/std/src/thread/spawnhook.rs b/library/std/src/thread/spawnhook.rs index b979db6bd6059..99b5ad9cb9fe5 100644 --- a/library/std/src/thread/spawnhook.rs +++ b/library/std/src/thread/spawnhook.rs @@ -3,8 +3,12 @@ use crate::iter; use crate::sync::Arc; use crate::thread::Thread; -// A thread local linked list of spawn hooks. crate::thread_local! { + /// A thread local linked list of spawn hooks. + /// + /// It is a linked list of Arcs, such that it can very cheaply be inhereted by spawned threads. + /// + /// (That technically makes it a set of linked lists with shared tails, so a linked tree.) static SPAWN_HOOKS: Cell = const { Cell::new(SpawnHooks { first: None }) }; } @@ -42,7 +46,7 @@ struct SpawnHook { /// /// Hooks can only be added, not removed. /// -/// The hooks will run in order, starting with the most recently added. +/// The hooks will run in reverse order, starting with the most recently added. /// /// # Usage ///