Skip to content

Commit 7a481ff

Browse files
committed
Properly abort when thread result panics on drop.
1 parent 5226395 commit 7a481ff

File tree

2 files changed

+26
-24
lines changed

2 files changed

+26
-24
lines changed

library/std/src/lib.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,11 @@ extern crate std as realstd;
363363
#[macro_use]
364364
mod macros;
365365

366+
// The runtime entry point and a few unstable public functions used by the
367+
// compiler
368+
#[macro_use]
369+
pub mod rt;
370+
366371
// The Rust prelude
367372
pub mod prelude;
368373

@@ -547,11 +552,6 @@ pub mod arch {
547552
#[stable(feature = "simd_x86", since = "1.27.0")]
548553
pub use std_detect::is_x86_feature_detected;
549554

550-
// The runtime entry point and a few unstable public functions used by the
551-
// compiler
552-
#[macro_use]
553-
pub mod rt;
554-
555555
// Platform-abstraction modules
556556
mod sys;
557557
mod sys_common;

library/std/src/thread/mod.rs

+21-19
Original file line numberDiff line numberDiff line change
@@ -1287,29 +1287,31 @@ unsafe impl<'scope, T: Sync> Sync for Packet<'scope, T> {}
12871287

12881288
impl<'scope, T> Drop for Packet<'scope, T> {
12891289
fn drop(&mut self) {
1290+
// If this packet was for a thread that ran in a scope, the thread
1291+
// panicked, and nobody consumed the panic payload, we make sure
1292+
// the scope function will panic.
1293+
let unhandled_panic = matches!(self.result.get_mut(), Some(Err(_)));
1294+
// Drop the result without causing unwinding.
1295+
// This is only relevant for threads that aren't join()ed, as
1296+
// join() will take the `result` and set it to None, such that
1297+
// there is nothing left to drop here.
1298+
// If this panics, we should handle that, because we're outside the
1299+
// outermost `catch_unwind` of our thread.
1300+
// We just abort in that case, since there's nothing else we can do.
1301+
// (And even if we tried to handle it somehow, we'd also need to handle
1302+
// the case where the panic payload we get out of it also panics on
1303+
// drop, and so on. See issue #86027.)
1304+
if let Err(_) = panic::catch_unwind(panic::AssertUnwindSafe(|| {
1305+
*self.result.get_mut() = None;
1306+
})) {
1307+
rtabort!("thread result panicked on drop");
1308+
}
12901309
// Book-keeping so the scope knows when it's done.
12911310
if let Some(scope) = self.scope {
1292-
// If this packet was for a thread that ran in a scope, the thread
1293-
// panicked, and nobody consumed the panic payload, we make sure
1294-
// the scope function will panic.
1295-
let unhandled_panic = matches!(self.result.get_mut(), Some(Err(_)));
1296-
// Drop the result before decrementing the number of running
1297-
// threads, because the Drop implementation might still use things
1298-
// it borrowed from 'scope.
1299-
// This is only relevant for threads that aren't join()ed, as
1300-
// join() will take the `result` and set it to None, such that
1301-
// there is nothing left to drop here.
1302-
// If this drop panics, that just results in an abort, because
1303-
// we're outside of the outermost `catch_unwind` of our thread.
1304-
// The same happens for detached non-scoped threads when dropping
1305-
// their ignored return value (or panic payload) panics, so
1306-
// there's no need to try to do anything better.
1307-
// (And even if we tried to handle it, we'd also need to handle
1308-
// the case where the panic payload we get out of it also panics
1309-
// on drop, and so on. See issue #86027.)
1310-
*self.result.get_mut() = None;
13111311
// Now that there will be no more user code running on this thread
13121312
// that can use 'scope, mark the thread as 'finished'.
1313+
// It's important we only do this after the `result` has been dropped,
1314+
// since dropping it might still use things it borrowed from 'scope.
13131315
scope.decrement_num_running_threads(unhandled_panic);
13141316
}
13151317
}

0 commit comments

Comments
 (0)