From f5217792ed0dbf132174250ea81ca2e3dd7b60f1 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Tue, 4 Jan 2022 16:10:14 +0100 Subject: [PATCH] Simplify panicking mechanism of thread::scope. It now panic!()s on its own, rather than resume_unwind'ing the panic payload from the thread. Using resume_unwind skips the panic_handler, meaning that the main thread would never have a panic handler run, which can get confusing. --- library/std/src/thread/mod.rs | 13 ++++------- library/std/src/thread/scoped.rs | 40 ++++++++++++++------------------ 2 files changed, 22 insertions(+), 31 deletions(-) diff --git a/library/std/src/thread/mod.rs b/library/std/src/thread/mod.rs index 0125545a3db..88dabfa8d22 100644 --- a/library/std/src/thread/mod.rs +++ b/library/std/src/thread/mod.rs @@ -1286,16 +1286,13 @@ unsafe impl<'scope, T: Sync> Sync for Packet<'scope, T> {} impl<'scope, T> Drop for Packet<'scope, T> { fn drop(&mut self) { + // Book-keeping so the scope knows when it's done. if let Some(scope) = self.scope { // If this packet was for a thread that ran in a scope, the thread - // panicked, and nobody consumed the panic payload, we put the - // panic payload in the scope so it can re-throw it, if it didn't - // already capture any panic yet. - if let Some(Err(e)) = self.result.get_mut().take() { - scope.panic_payload.lock().unwrap().get_or_insert(e); - } - // Book-keeping so the scope knows when it's done. - scope.decrement_n_running_threads(); + // panicked, and nobody consumed the panic payload, we make sure + // the scope function will panic. + let unhandled_panic = matches!(self.result.get_mut(), Some(Err(_))); + scope.decrement_n_running_threads(unhandled_panic); } } } diff --git a/library/std/src/thread/scoped.rs b/library/std/src/thread/scoped.rs index 8f050b72a41..f9b056dcd46 100644 --- a/library/std/src/thread/scoped.rs +++ b/library/std/src/thread/scoped.rs @@ -1,11 +1,10 @@ use super::{current, park, Builder, JoinInner, Result, Thread}; -use crate::any::Any; use crate::fmt; use crate::io; use crate::marker::PhantomData; use crate::panic::{catch_unwind, resume_unwind, AssertUnwindSafe}; -use crate::sync::atomic::{AtomicUsize, Ordering}; -use crate::sync::{Arc, Mutex}; +use crate::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; +use crate::sync::Arc; /// A scope to spawn scoped threads in. /// @@ -22,8 +21,8 @@ pub struct ScopedJoinHandle<'scope, T>(JoinInner<'scope, T>); pub(super) struct ScopeData { n_running_threads: AtomicUsize, + a_thread_panicked: AtomicBool, main_thread: Thread, - pub(super) panic_payload: Mutex>>, } impl ScopeData { @@ -32,11 +31,14 @@ impl ScopeData { // chance it overflows to 0, which would result in unsoundness. if self.n_running_threads.fetch_add(1, Ordering::Relaxed) == usize::MAX / 2 { // This can only reasonably happen by mem::forget()'ing many many ScopedJoinHandles. - self.decrement_n_running_threads(); + self.decrement_n_running_threads(false); panic!("too many running threads in thread scope"); } } - pub(super) fn decrement_n_running_threads(&self) { + pub(super) fn decrement_n_running_threads(&self, panic: bool) { + if panic { + self.a_thread_panicked.store(true, Ordering::Relaxed); + } if self.n_running_threads.fetch_sub(1, Ordering::Release) == 1 { self.main_thread.unpark(); } @@ -89,15 +91,16 @@ impl ScopeData { /// a.push(4); /// assert_eq!(x, a.len()); /// ``` +#[track_caller] pub fn scope<'env, F, T>(f: F) -> T where F: FnOnce(&Scope<'env>) -> T, { - let mut scope = Scope { + let scope = Scope { data: ScopeData { n_running_threads: AtomicUsize::new(0), main_thread: current(), - panic_payload: Mutex::new(None), + a_thread_panicked: AtomicBool::new(false), }, env: PhantomData, }; @@ -110,21 +113,11 @@ where park(); } - // Throw any panic from `f` or from any panicked thread, or the return value of `f` otherwise. + // Throw any panic from `f`, or the return value of `f` if no thread panicked. match result { - Err(e) => { - // `f` itself panicked. - resume_unwind(e); - } - Ok(result) => { - if let Some(panic_payload) = scope.data.panic_payload.get_mut().unwrap().take() { - // A thread panicked. - resume_unwind(panic_payload); - } else { - // Nothing panicked. - result - } - } + Err(e) => resume_unwind(e), + Ok(_) if scope.data.a_thread_panicked.load(Ordering::Relaxed) => panic!("a thread panicked"), + Ok(result) => result, } } @@ -293,7 +286,8 @@ impl<'env> fmt::Debug for Scope<'env> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("Scope") .field("n_running_threads", &self.data.n_running_threads.load(Ordering::Relaxed)) - .field("panic_payload", &self.data.panic_payload) + .field("a_thread_panicked", &self.data.a_thread_panicked) + .field("main_thread", &self.data.main_thread) .finish_non_exhaustive() } }