Skip to content

Commit f521779

Browse files
committed
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.
1 parent da33da1 commit f521779

File tree

2 files changed

+22
-31
lines changed

2 files changed

+22
-31
lines changed

library/std/src/thread/mod.rs

+5-8
Original file line numberDiff line numberDiff line change
@@ -1286,16 +1286,13 @@ unsafe impl<'scope, T: Sync> Sync for Packet<'scope, T> {}
12861286

12871287
impl<'scope, T> Drop for Packet<'scope, T> {
12881288
fn drop(&mut self) {
1289+
// Book-keeping so the scope knows when it's done.
12891290
if let Some(scope) = self.scope {
12901291
// If this packet was for a thread that ran in a scope, the thread
1291-
// panicked, and nobody consumed the panic payload, we put the
1292-
// panic payload in the scope so it can re-throw it, if it didn't
1293-
// already capture any panic yet.
1294-
if let Some(Err(e)) = self.result.get_mut().take() {
1295-
scope.panic_payload.lock().unwrap().get_or_insert(e);
1296-
}
1297-
// Book-keeping so the scope knows when it's done.
1298-
scope.decrement_n_running_threads();
1292+
// panicked, and nobody consumed the panic payload, we make sure
1293+
// the scope function will panic.
1294+
let unhandled_panic = matches!(self.result.get_mut(), Some(Err(_)));
1295+
scope.decrement_n_running_threads(unhandled_panic);
12991296
}
13001297
}
13011298
}

library/std/src/thread/scoped.rs

+17-23
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
use super::{current, park, Builder, JoinInner, Result, Thread};
2-
use crate::any::Any;
32
use crate::fmt;
43
use crate::io;
54
use crate::marker::PhantomData;
65
use crate::panic::{catch_unwind, resume_unwind, AssertUnwindSafe};
7-
use crate::sync::atomic::{AtomicUsize, Ordering};
8-
use crate::sync::{Arc, Mutex};
6+
use crate::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
7+
use crate::sync::Arc;
98

109
/// A scope to spawn scoped threads in.
1110
///
@@ -22,8 +21,8 @@ pub struct ScopedJoinHandle<'scope, T>(JoinInner<'scope, T>);
2221

2322
pub(super) struct ScopeData {
2423
n_running_threads: AtomicUsize,
24+
a_thread_panicked: AtomicBool,
2525
main_thread: Thread,
26-
pub(super) panic_payload: Mutex<Option<Box<dyn Any + Send>>>,
2726
}
2827

2928
impl ScopeData {
@@ -32,11 +31,14 @@ impl ScopeData {
3231
// chance it overflows to 0, which would result in unsoundness.
3332
if self.n_running_threads.fetch_add(1, Ordering::Relaxed) == usize::MAX / 2 {
3433
// This can only reasonably happen by mem::forget()'ing many many ScopedJoinHandles.
35-
self.decrement_n_running_threads();
34+
self.decrement_n_running_threads(false);
3635
panic!("too many running threads in thread scope");
3736
}
3837
}
39-
pub(super) fn decrement_n_running_threads(&self) {
38+
pub(super) fn decrement_n_running_threads(&self, panic: bool) {
39+
if panic {
40+
self.a_thread_panicked.store(true, Ordering::Relaxed);
41+
}
4042
if self.n_running_threads.fetch_sub(1, Ordering::Release) == 1 {
4143
self.main_thread.unpark();
4244
}
@@ -89,15 +91,16 @@ impl ScopeData {
8991
/// a.push(4);
9092
/// assert_eq!(x, a.len());
9193
/// ```
94+
#[track_caller]
9295
pub fn scope<'env, F, T>(f: F) -> T
9396
where
9497
F: FnOnce(&Scope<'env>) -> T,
9598
{
96-
let mut scope = Scope {
99+
let scope = Scope {
97100
data: ScopeData {
98101
n_running_threads: AtomicUsize::new(0),
99102
main_thread: current(),
100-
panic_payload: Mutex::new(None),
103+
a_thread_panicked: AtomicBool::new(false),
101104
},
102105
env: PhantomData,
103106
};
@@ -110,21 +113,11 @@ where
110113
park();
111114
}
112115

113-
// Throw any panic from `f` or from any panicked thread, or the return value of `f` otherwise.
116+
// Throw any panic from `f`, or the return value of `f` if no thread panicked.
114117
match result {
115-
Err(e) => {
116-
// `f` itself panicked.
117-
resume_unwind(e);
118-
}
119-
Ok(result) => {
120-
if let Some(panic_payload) = scope.data.panic_payload.get_mut().unwrap().take() {
121-
// A thread panicked.
122-
resume_unwind(panic_payload);
123-
} else {
124-
// Nothing panicked.
125-
result
126-
}
127-
}
118+
Err(e) => resume_unwind(e),
119+
Ok(_) if scope.data.a_thread_panicked.load(Ordering::Relaxed) => panic!("a thread panicked"),
120+
Ok(result) => result,
128121
}
129122
}
130123

@@ -293,7 +286,8 @@ impl<'env> fmt::Debug for Scope<'env> {
293286
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
294287
f.debug_struct("Scope")
295288
.field("n_running_threads", &self.data.n_running_threads.load(Ordering::Relaxed))
296-
.field("panic_payload", &self.data.panic_payload)
289+
.field("a_thread_panicked", &self.data.a_thread_panicked)
290+
.field("main_thread", &self.data.main_thread)
297291
.finish_non_exhaustive()
298292
}
299293
}

0 commit comments

Comments
 (0)