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.
This commit is contained in:
Mara Bos 2022-01-04 16:10:14 +01:00
parent da33da161b
commit f5217792ed
2 changed files with 22 additions and 31 deletions

View File

@ -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);
}
}
}

View File

@ -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 Scope<'env> {
pub(super) struct ScopeData {
n_running_threads: AtomicUsize,
a_thread_panicked: AtomicBool,
main_thread: Thread,
pub(super) panic_payload: Mutex<Option<Box<dyn Any + Send>>>,
}
impl ScopeData {
@ -32,11 +31,14 @@ pub(super) fn increment_n_running_threads(&self) {
// 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 @@ pub(super) fn decrement_n_running_threads(&self) {
/// 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 @@ pub fn scope<'env, F, T>(f: F) -> T
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()
}
}