From db1f0d045887e8046dd542e119b27773991039b6 Mon Sep 17 00:00:00 2001 From: Mahmoud Mazouz Date: Sun, 23 Feb 2025 12:22:52 +0100 Subject: [PATCH 1/3] Return error on unexpected termination in `Thread::join`. There is a time window during which the OS can terminate a thread before stdlib can retreive its `Packet`. Currently the `Thread::join` panics with no message in such an event, which makes debugging difficult; fixes #124466. --- library/std/src/thread/mod.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/library/std/src/thread/mod.rs b/library/std/src/thread/mod.rs index 59b395336f2e3..f5101a66ce192 100644 --- a/library/std/src/thread/mod.rs +++ b/library/std/src/thread/mod.rs @@ -1739,7 +1739,11 @@ struct JoinInner<'scope, T> { impl<'scope, T> JoinInner<'scope, T> { fn join(mut self) -> Result { self.native.join(); - Arc::get_mut(&mut self.packet).unwrap().result.get_mut().take().unwrap() + if let Some(packet) = Arc::get_mut(&mut self.packet) { + packet.result.get_mut().take().unwrap() + } else { + Err(Box::new("thread terminated unexpectedly (e.g. due to OS intervention)")) + } } } From 1ccdc06136ccb134f30d6604e89a659eae62b032 Mon Sep 17 00:00:00 2001 From: Mahmoud Mazouz Date: Mon, 24 Feb 2025 09:50:46 +0100 Subject: [PATCH 2/3] Remove speculation on cause of error Co-authored-by: Jubilee --- library/std/src/thread/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/thread/mod.rs b/library/std/src/thread/mod.rs index f5101a66ce192..d9e28acdcdac1 100644 --- a/library/std/src/thread/mod.rs +++ b/library/std/src/thread/mod.rs @@ -1742,7 +1742,7 @@ impl<'scope, T> JoinInner<'scope, T> { if let Some(packet) = Arc::get_mut(&mut self.packet) { packet.result.get_mut().take().unwrap() } else { - Err(Box::new("thread terminated unexpectedly (e.g. due to OS intervention)")) + Err(Box::new("thread terminated unexpectedly")) } } } From 7058f62d0a57d94eb25047741460656f1b14e566 Mon Sep 17 00:00:00 2001 From: Mahmoud Mazouz Date: Tue, 25 Feb 2025 13:09:09 +0100 Subject: [PATCH 3/3] Use `.expect(..)` instead --- library/std/src/thread/mod.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/library/std/src/thread/mod.rs b/library/std/src/thread/mod.rs index d9e28acdcdac1..3f3ba02361cc8 100644 --- a/library/std/src/thread/mod.rs +++ b/library/std/src/thread/mod.rs @@ -1739,11 +1739,16 @@ struct JoinInner<'scope, T> { impl<'scope, T> JoinInner<'scope, T> { fn join(mut self) -> Result { self.native.join(); - if let Some(packet) = Arc::get_mut(&mut self.packet) { - packet.result.get_mut().take().unwrap() - } else { - Err(Box::new("thread terminated unexpectedly")) - } + Arc::get_mut(&mut self.packet) + // FIXME(fuzzypixelz): returning an error instead of panicking here + // would require updating the documentation of + // `std::thread::Result`; currently we can return `Err` if and only + // if the thread had panicked. + .expect("threads should not terminate unexpectedly") + .result + .get_mut() + .take() + .unwrap() } }