Skip to content

scoped threads: pass closure through MaybeUninit to avoid invalid dangling references #102589

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 11, 2022
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions library/std/src/thread/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -499,13 +499,40 @@ impl Builder {
let output_capture = crate::io::set_output_capture(None);
crate::io::set_output_capture(output_capture.clone());

// Pass `f` in `MaybeUninit` because actually that closure might *run longer than the lifetime of `F`*.
// See <https://github.com/rust-lang/rust/issues/101983> for more details.
// To prevent leaks we use a wrapper that drops its contents.
#[repr(transparent)]
struct MaybeDangling<T>(mem::MaybeUninit<T>);
impl<T> MaybeDangling<T> {
fn new(x: T) -> Self {
MaybeDangling(mem::MaybeUninit::new(x))
}
fn into_inner(self) -> T {
// SAFETY: we are always initiailized.
let ret = unsafe { self.0.assume_init_read() };
// Make sure we don't drop.
mem::forget(self);
ret
}
}
impl<T> Drop for MaybeDangling<T> {
fn drop(&mut self) {
// SAFETY: we are always initiailized.
unsafe { self.0.assume_init_drop() };
}
}

let f = MaybeDangling::new(f);
let main = move || {
if let Some(name) = their_thread.cname() {
imp::Thread::set_name(name);
}

crate::io::set_output_capture(output_capture);

// SAFETY: we constructed `f` initialized.
let f = f.into_inner();
// SAFETY: the stack guard passed is the one for the current thread.
// This means the current thread's stack and the new thread's stack
// are properly set and protected from each other.
Expand All @@ -518,6 +545,12 @@ impl Builder {
// same `JoinInner` as this closure meaning the mutation will be
// safe (not modify it and affect a value far away).
unsafe { *their_packet.result.get() = Some(try_result) };
// Here `their_packet` gets dropped, and if this is the last `Arc` for that packet that
// will call `decrement_num_running_threads` and therefore signal that this thread is
// done.
drop(their_packet);
// Here, the lifetime `'a` and even `'scope` can end. `main` keeps running for a bit
// after that before returning itself.
};

if let Some(scope_data) = &my_packet.scope {
Expand Down