From ce472c9f9fccf05cba4529a913532af9413d5a1d Mon Sep 17 00:00:00 2001 From: Max Dexheimer Date: Sun, 26 Jul 2026 11:41:51 +0200 Subject: [PATCH 1/2] Ensure noone can observe the thread spawn hooks being taken --- library/std/src/thread/spawnhook.rs | 14 +++-- .../std/add-spawn-hook-reentrancy-159923.rs | 61 +++++++++++++++++++ 2 files changed, 69 insertions(+), 6 deletions(-) create mode 100644 tests/ui/std/add-spawn-hook-reentrancy-159923.rs diff --git a/library/std/src/thread/spawnhook.rs b/library/std/src/thread/spawnhook.rs index bb36fb687b4af..ec771ef48dede 100644 --- a/library/std/src/thread/spawnhook.rs +++ b/library/std/src/thread/spawnhook.rs @@ -1,7 +1,7 @@ use super::thread::Thread; use crate::cell::Cell; use crate::iter; -use crate::sync::Arc; +use crate::sync::{Arc, UniqueArc}; crate::thread_local! { /// A thread local linked list of spawn hooks. @@ -95,12 +95,14 @@ where G: 'static + Send + FnOnce(), { SPAWN_HOOKS.with(|h| { - let mut hooks = h.take(); - let next = hooks.first.take(); - hooks.first = Some(Arc::new(SpawnHook { + // Perform all fallible operations before taking the current hooks (see #159923) + let mut new_first = UniqueArc::new(SpawnHook { hook: Box::new(move |thread| Box::new(hook(thread))), - next, - })); + next: None, + }); + let mut hooks = h.take(); + new_first.next = hooks.first.take(); + hooks.first = Some(UniqueArc::into_arc(new_first)); h.set(hooks); }); } diff --git a/tests/ui/std/add-spawn-hook-reentrancy-159923.rs b/tests/ui/std/add-spawn-hook-reentrancy-159923.rs new file mode 100644 index 0000000000000..a3714bfda9501 --- /dev/null +++ b/tests/ui/std/add-spawn-hook-reentrancy-159923.rs @@ -0,0 +1,61 @@ +// https://github.com/rust-lang/rust/issues/159923 +//@ edition:2024 +//@ run-pass +//@ needs-threads + +#![feature(alloc_error_hook, thread_spawn_hook)] + +use std::{ + alloc::{GlobalAlloc, Layout, System, set_alloc_error_hook}, + sync::atomic::{AtomicBool, AtomicU32, Ordering}, + thread, +}; + +struct FailingGlobalAlloc; +unsafe impl GlobalAlloc for FailingGlobalAlloc { + unsafe fn alloc(&self, layout: Layout) -> *mut u8 { + if FAIL_NEXT_ALLOC.swap(false, Ordering::Relaxed) { + return std::ptr::null_mut(); + } + unsafe { System.alloc(layout) } + } + unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) { + unsafe { System.dealloc(ptr, layout) } + } +} +#[global_allocator] +static ALLOC: FailingGlobalAlloc = FailingGlobalAlloc; + +static FAIL_NEXT_ALLOC: AtomicBool = AtomicBool::new(false); + +fn spawn_and_join() { + thread::scope(|scope| { + scope.spawn(|| {}); + }); +} + +fn main() { + static COUNT: AtomicU32 = AtomicU32::new(0); + + thread::add_spawn_hook(|_| || _ = COUNT.fetch_add(1, Ordering::Relaxed)); + thread::add_spawn_hook(|_| || _ = COUNT.fetch_add(1, Ordering::Relaxed)); + + spawn_and_join(); + spawn_and_join(); + assert_eq!(COUNT.swap(0, Ordering::Relaxed), 4); + + set_alloc_error_hook(|_| { + spawn_and_join(); + assert_eq!(COUNT.swap(0, Ordering::Relaxed), 2); + panic!() + }); + + FAIL_NEXT_ALLOC.store(true, Ordering::Relaxed); + std::panic::catch_unwind(|| { + std::thread::add_spawn_hook(|_| || {}); + }) + .unwrap_err(); + + spawn_and_join(); + assert_eq!(COUNT.swap(0, Ordering::Relaxed), 2); +} From f80e5c1e9b38c1db7b98f1ee2963a684f71e6966 Mon Sep 17 00:00:00 2001 From: Max Dexheimer Date: Sun, 26 Jul 2026 11:45:36 +0200 Subject: [PATCH 2/2] Explicitly document that spawn hooks are not guaranteed to run --- 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 ec771ef48dede..92fb586d39dcf 100644 --- a/library/std/src/thread/spawnhook.rs +++ b/library/std/src/thread/spawnhook.rs @@ -44,10 +44,12 @@ struct SpawnHook { /// 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. -/// /// The hooks will run in reverse order, starting with the most recently added. /// +/// Hooks can only be added, not removed. +/// Note that this does *not* mean that they are guaranteed to run. See [`Builder::no_hooks`]. +/// You cannot rely on a hook running for soundness. +/// /// # Usage /// /// ``` @@ -88,6 +90,8 @@ struct SpawnHook { /// assert_eq!(X.get(), 123); /// }).join().unwrap(); /// ``` +/// +/// [`Builder::no_hooks`]: crate::thread::Builder::no_hooks #[unstable(feature = "thread_spawn_hook", issue = "132951")] pub fn add_spawn_hook(hook: F) where