Rollup merge of #102589 - RalfJung:scoped-threads-dangling, r=m-ou-se
scoped threads: pass closure through MaybeUninit to avoid invalid dangling references The `main` function defined here looks roughly like this, if it were written as a more explicit stand-alone function: ```rust // Not showing all the `'lifetime` tracking, the point is that // this closure might live shorter than `thread`. fn thread(control: ..., closure: impl FnOnce() + 'lifetime) { closure(); control.signal_done(); // A lot of time can pass here. } ``` Note that `thread` continues to run even after `signal_done`! Now consider what happens if the `closure` captures a reference of lifetime `'lifetime`: - The type of `closure` is a struct (the implicit unnameable closure type) with a `&'lifetime mut T` field. References passed to a function are marked with `dereferenceable`, which is LLVM speak for *this reference will remain live for the entire duration of this function*. - The closure runs, `signal_done` runs. Then -- potentially -- this thread gets scheduled away and the main thread runs, seeing the signal and returning to the user. Now `'lifetime` ends and the memory the reference points to might be deallocated. - Now we have UB! The reference that as passed to `thread` with the promise of remaining live for the entire duration of the function, actually got deallocated while the function still runs. Oops. Long-term I think we should be able to use `ManuallyDrop` to fix this without `unsafe`, or maybe a new `MaybeDangling` type. I am working on an RFC for that. But in the mean time it'd be nice to fix this so that Miri with `-Zmiri-retag-fields` (which is needed for "full enforcement" of all the LLVM flags we generate) stops erroring on scoped threads. Fixes https://github.com/rust-lang/rust/issues/101983 r? `@m-ou-se`
This commit is contained in:
commit
919d6bf446
1 changed files with 33 additions and 0 deletions
|
@ -499,6 +499,31 @@ 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);
|
||||
|
@ -506,6 +531,8 @@ impl Builder {
|
|||
|
||||
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.
|
||||
|
@ -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 {
|
||||
|
|
Loading…
Add table
Reference in a new issue